julep: replace InexactError with something more informative

[timholy]

Trying to convert -1 to a UInt justifiably throws an error, but the message isn't very informative:

julia> convert(UInt, -1)
ERROR: InexactError()
 in convert(::Type{UInt64}, ::Int64) at ./int.jl:226

Now that we have good backtraces, it's usually reasonably straightforward to figure out what went wrong, but I don't think anyone would say this is bending over backwards to be friendly to newbies.

Historically, there was a good reason for the use of InexactError, which can be seen from the following alternative implementation of convert:

@inline function iconvert1{T<:Integer}(::Type{T}, x::Integer)
    typemin(T) <= x <= typemax(T) || error("cannot convert $x to $T, need $(typemin(T)) <= $x <= $(typemax(T))")
    x % T  # converts without checking
end

This is obviously a much more friendly message. The rub is appears to be the LLVM:

 julia> @code_llvm convert(UInt, -1)

define i64 @jlsys_convert_41836(%jl_value_t*, i64) #0 {
top:
  %2 = icmp sgt i64 %1, -1
  br i1 %2, label %pass, label %fail

fail:                                             ; preds = %top
  call void @jl_throw(%jl_value_t* inttoptr (i64 139778617325664 to %jl_value_t*))
  unreachable

pass:                                             ; preds = %top
  ret i64 %1
}

but

julia> @code_llvm iconvert1(UInt, -1)

define i64 @julia_iconvert1_70982(%jl_value_t*, i64) #0 {
top:
  %ptls_i8 = call i8* asm "movq %fs:0, $0;\0Aaddq $$-2672, $0", "=r,~{dirflag},~{fpsr},~{flags}"() #4
  %ptls = bitcast i8* %ptls_i8 to %jl_value_t***
  %2 = alloca [15 x %jl_value_t*], align 8
  %.sub = getelementptr inbounds [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 0
  %3 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 5
  %4 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 2
  %5 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 4
  %6 = bitcast %jl_value_t** %3 to i8*
  call void @llvm.memset.p0i8.i32(i8* %6, i8 0, i32 80, i32 8, i1 false)
  %7 = bitcast [15 x %jl_value_t*]* %2 to i64*
  %8 = bitcast %jl_value_t** %4 to i8*
  call void @llvm.memset.p0i8.i64(i8* %8, i8 0, i64 16, i32 8, i1 false)
  store i64 26, i64* %7, align 8
  %9 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 1
  %10 = bitcast i8* %ptls_i8 to i64*
  %11 = load i64, i64* %10, align 8
  %12 = bitcast %jl_value_t** %9 to i64*
  store i64 %11, i64* %12, align 8
  store %jl_value_t** %.sub, %jl_value_t*** %ptls, align 8
  store %jl_value_t* null, %jl_value_t** %5, align 8
  %13 = icmp slt i64 %1, 0
  br i1 %13, label %pass.7, label %if4

if4:                                              ; preds = %top
  %14 = load i64, i64* %12, align 8
  store i64 %14, i64* %10, align 8
  ret i64 %1

pass.7:                                           ; preds = %top
  %15 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 3
  %16 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 6
  %17 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 7
  %18 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 8
  %19 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 9
  %20 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 10
  %21 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 11
  %22 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 12
  %23 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 13
  %24 = getelementptr [15 x %jl_value_t*], [15 x %jl_value_t*]* %2, i64 0, i64 14
  store %jl_value_t* inttoptr (i64 139778710250096 to %jl_value_t*), %jl_value_t** %3, align 8
  %25 = call %jl_value_t* @jl_box_int64(i64 signext %1)
  store %jl_value_t* %25, %jl_value_t** %16, align 8
  store %jl_value_t* inttoptr (i64 139778710250112 to %jl_value_t*), %jl_value_t** %17, align 8
  store %jl_value_t* inttoptr (i64 139778617303408 to %jl_value_t*), %jl_value_t** %18, align 8
  store %jl_value_t* inttoptr (i64 139778710250128 to %jl_value_t*), %jl_value_t** %19, align 8
  store %jl_value_t* inttoptr (i64 139778708168720 to %jl_value_t*), %jl_value_t** %20, align 8
  store %jl_value_t* inttoptr (i64 139778710250144 to %jl_value_t*), %jl_value_t** %21, align 8
  %26 = call %jl_value_t* @jl_box_int64(i64 signext %1)
  store %jl_value_t* %26, %jl_value_t** %22, align 8
  store %jl_value_t* inttoptr (i64 139778710250160 to %jl_value_t*), %jl_value_t** %23, align 8
  store %jl_value_t* inttoptr (i64 139778620813600 to %jl_value_t*), %jl_value_t** %24, align 8
  %27 = call %jl_value_t* @jlsys_string_42026(%jl_value_t* inttoptr (i64 139778619782296 to %jl_value_t*), %jl_value_t** %3, i32 10)
  store %jl_value_t* %27, %jl_value_t** %4, align 8
  %28 = call %jl_value_t* @jl_gc_pool_alloc(i8* %ptls_i8, i32 1432, i32 16)
  %29 = getelementptr inbounds %jl_value_t, %jl_value_t* %28, i64 -1, i32 0
  store %jl_value_t* inttoptr (i64 139778620410704 to %jl_value_t*), %jl_value_t** %29, align 8
  store %jl_value_t* %28, %jl_value_t** %15, align 8
  store %jl_value_t* %27, %jl_value_t** %5, align 8
  %30 = getelementptr inbounds %jl_value_t, %jl_value_t* %28, i64 0, i32 0
  store %jl_value_t* %27, %jl_value_t** %30, align 8
  call void @jl_throw(%jl_value_t* %28)
  unreachable
}

Surprisingly this doesn't seem to be as bad as it looks (see below), but at least on older versions of LLVM I suspect this was a performance disaster.

However, now that we have @noinline the following alternative implementation is available:

@inline function iconvert2{T<:Integer}(::Type{T}, x::Integer)
    typemin(T) <= x <= typemax(T) || throw_converterror(T, x)
    x % T
end
@noinline function throw_converterror{T}(::Type{T}, x)
    error("cannot convert $x to $T, need $(typemin(T)) <= $x <= $(typemax(T))")
end

for which the LLVM is essentially identical to convert:

julia> @code_llvm iconvert2(UInt, -1)

define i64 @julia_iconvert2_71005(%jl_value_t*, i64) #0 {
top:
  %2 = icmp slt i64 %1, 0
  br i1 %2, label %L2, label %if4

L2:                                               ; preds = %top
  call void @julia_throw_converterror_70883(%jl_value_t* inttoptr (i64 139778617303408 to %jl_value_t*), i64 %1) #0
  call void @llvm.trap()
  unreachable

if4:                                              ; preds = %top
  ret i64 %1
}

Benchmarking with

function fbench(f, y)
    s = UInt(0)
    @inbounds for x in y
        s += f(UInt, x)
    end
    s
end

y = rand(1:20, 10^4);

indicates that convert and iconvert2 have identical performance. (What's quite surprising is that on my machine, iconvert1 performs equally well!)

Since there's no performance hit, and since the error message is so much more informative, I recommend we ditch InexactError entirely. Or better would probably be to still keep the type (so it can be checked with @test_throws) but give it a string field that explains the problem.

[simonster]

This would be great. Could also do something similar for DomainError. My only question is whether it's better to put the string in the object, or put the function and value in the object and then format the string when the error is shown, which we could conceivably do now that we can dispatch on functions.

[timholy]

That's an interesting idea, and it seems reasonable. I guess the InexactError/DomainError could have an args field which would be a tuple. That would allow it to handle the fact that some (e.g., sqrt) only take 1 argument but others (e.g., convert) take 2.

Seems like there's interest. For now I'm going to label this with an "up for grabs" tag, since I have quite a long list of higher priorities. But I may get to it eventually, since I do think it would be an improvement.

[nalimilan]

Formatting the string only when printing the error sounds like a good idea, as it allows standardizing it and checking the exception details from code. Ideally, I think we should use distinct exception types for each well-defined error, instead of reusing the same type with variable-length fields. For example, InexactError could have a field holding the type and a field holding the value, and would only be used for cases which match this structure.

[TotalVerb]

A similar approach could be used for OverflowError; see #18303

[timholy]

WIP at #20005, milestone added.