Skip to content

Commit

Permalink
invoke should raise MethodError for ambiguous cases
Browse files Browse the repository at this point in the history
fix #18095
fix #22988 (by improving error message)
  • Loading branch information
vtjnash committed Sep 6, 2017
1 parent 508c9f5 commit db80d86
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 33 deletions.
21 changes: 5 additions & 16 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -884,23 +884,12 @@ function which(@nospecialize(f), @nospecialize(t))
throw(ArgumentError("argument is not a generic function"))
end
t = to_tuple_type(t)
if isleaftype(t)
ms = methods(f, t)
isempty(ms) && error("no method found for the specified argument types")
length(ms)!=1 && error("no unique matching method for the specified argument types")
return first(ms)
else
tt = signature_type(f, t)
m = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tt, typemax(UInt))
if m === nothing
error("no method found for the specified argument types")
end
meth = m.func::Method
if ccall(:jl_has_call_ambiguities, Cint, (Any, Any), tt, meth) != 0
error("method match is ambiguous for the specified argument types")
end
return meth
tt = signature_type(f, t)
m = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tt, typemax(UInt))
if m === nothing
error("no unique matching method found for the specified argument types")
end
return m.func::Method
end

"""
Expand Down
22 changes: 20 additions & 2 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,7 @@ jl_method_instance_t *jl_method_lookup_by_type(jl_methtable_t *mt, jl_tupletype_
JL_UNLOCK(&mt->writelock);
return linfo;
}
if (jl_is_leaf_type((jl_value_t*)types))
if (jl_is_leaf_type((jl_value_t*)types)) // FIXME: this is the wrong predicate
cache = 1;
jl_method_instance_t *sf = jl_mt_assoc_by_type(mt, types, cache, allow_exec, world);
if (cache) {
Expand Down Expand Up @@ -1745,9 +1745,25 @@ JL_DLLEXPORT jl_value_t *jl_get_spec_lambda(jl_tupletype_t *types, size_t world)
return li ? (jl_value_t*)li : jl_nothing;
}

// see if a call to m with computed from `types` is ambiguous
JL_DLLEXPORT int jl_is_call_ambiguous(jl_tupletype_t *types, jl_method_t *m)
{
if (m->ambig == jl_nothing)
return 0;
for (size_t i = 0; i < jl_array_len(m->ambig); i++) {
jl_method_t *mambig = (jl_method_t*)jl_array_ptr_ref(m->ambig, i);
if (jl_subtype((jl_value_t*)types, (jl_value_t*)mambig->sig))
return 1;
}
return 0;
}

// see if a call to m with a subtype of `types` might be ambiguous
// if types is from a call signature (approximated by isleaftype), this is the same as jl_is_call_ambiguous above
JL_DLLEXPORT int jl_has_call_ambiguities(jl_tupletype_t *types, jl_method_t *m)
{
if (m->ambig == jl_nothing) return 0;
if (m->ambig == jl_nothing)
return 0;
for (size_t i = 0; i < jl_array_len(m->ambig); i++) {
jl_method_t *mambig = (jl_method_t*)jl_array_ptr_ref(m->ambig, i);
if (!jl_has_empty_intersection((jl_value_t*)mambig->sig, (jl_value_t*)types))
Expand Down Expand Up @@ -1951,6 +1967,8 @@ JL_DLLEXPORT jl_value_t *jl_gf_invoke_lookup(jl_datatype_t *types, size_t world)
JL_GC_POP();
if (!entry)
return jl_nothing;
if (jl_is_call_ambiguous(types, entry->func.method))
return jl_nothing;
return (jl_value_t*)entry;
}

Expand Down
32 changes: 20 additions & 12 deletions test/ambiguous.jl
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,28 @@ end

## Other ways of accessing functions
# Test that non-ambiguous cases work
io = IOBuffer()
@test precompile(ambig, (Int, Int)) == true
cfunction(ambig, Int, Tuple{Int, Int})
@test length(code_lowered(ambig, (Int, Int))) == 1
@test length(code_typed(ambig, (Int, Int))) == 1
code_llvm(io, ambig, (Int, Int))
code_native(io, ambig, (Int, Int))
let io = IOBuffer()
@test precompile(ambig, (Int, Int)) == true
cf = cfunction(ambig, Int, Tuple{Int, Int})
@test ccall(cf, Int, (Int, Int), 1, 2) == 4
@test length(code_lowered(ambig, (Int, Int))) == 1
@test length(code_typed(ambig, (Int, Int))) == 1
code_llvm(io, ambig, (Int, Int))
code_native(io, ambig, (Int, Int))
end

# Test that ambiguous cases fail appropriately
@test precompile(ambig, (UInt8, Int)) == false
cfunction(ambig, Int, Tuple{UInt8, Int}) # test for a crash (doesn't throw an error)
@test_throws ErrorException which(ambig, (UInt8, Int))
@test_throws ErrorException code_llvm(io, ambig, (UInt8, Int))
@test_throws ErrorException code_native(io, ambig, (UInt8, Int))
let io = IOBuffer()
@test precompile(ambig, (UInt8, Int)) == false
cf = cfunction(ambig, Int, Tuple{UInt8, Int}) # test for a crash (doesn't throw an error)
@test_throws MethodError ccall(cf, Int, (UInt8, Int), 1, 2)
@test_throws(ErrorException("no unique matching method found for the specified argument types"),
which(ambig, (UInt8, Int)))
@test_throws(ErrorException("no unique matching method found for the specified argument types"),
code_llvm(io, ambig, (UInt8, Int)))
@test_throws(ErrorException("no unique matching method found for the specified argument types"),
code_native(io, ambig, (UInt8, Int)))
end

# Method overwriting doesn't destroy ambiguities
@test_throws MethodError ambig(2, 0x03)
Expand Down
13 changes: 10 additions & 3 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ end
# issue #2169
let
i2169(a::Array{T}) where {T} = typemin(T)
@test invoke(i2169, Tuple{Array} ,Int8[1]) === Int8(-128)
@test invoke(i2169, Tuple{Array}, Int8[1]) === Int8(-128)
end

# issue #2365
Expand Down Expand Up @@ -4541,16 +4541,23 @@ let x = 1
@noinline g18444(a) = (x += 1; a[])
f18444_1(a) = invoke(sin, Tuple{Int}, g18444(a))
f18444_2(a) = invoke(sin, Tuple{Integer}, g18444(a))
@test_throws ErrorException f18444_1(Ref{Any}(1.0))
@test_throws ErrorException("invoke: argument type error") f18444_1(Ref{Any}(1.0))
@test x == 2
@test_throws ErrorException f18444_2(Ref{Any}(1.0))
@test_throws ErrorException("invoke: argument type error") f18444_2(Ref{Any}(1.0))
@test x == 3
@test f18444_1(Ref{Any}(1)) === sin(1)
@test x == 4
@test f18444_2(Ref{Any}(1)) === sin(1)
@test x == 5
end

f18095(::Int, ::Number) = 0x21
f18095(::Number, ::Int) = 0x12
@test_throws MethodError f18095(1, 2)
@test_throws MethodError invoke(f18095, Tuple{Int, Int}, 1, 2)
@test_throws MethodError invoke(f18095, Tuple{Int, Any}, 1, 2)
@test invoke(f18095, Tuple{Int, Real}, 1, 2) === 0x21

# issue #10981, long argument lists
let a = fill(["sdf"], 2*10^6), temp_vcat(x...) = vcat(x...)
# we introduce a new function `temp_vcat` to make sure there is no existing
Expand Down

0 comments on commit db80d86

Please sign in to comment.