diff --git a/base/reflection.jl b/base/reflection.jl index babd0fd577150..a8ed7cbfe3bb6 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -176,11 +176,13 @@ code_lowered(f, t::ANY=Tuple) = map(m -> (m::Method).lambda_template, methods(f, function _methods(f::ANY,t::ANY,lim) ft = isa(f,Type) ? Type{f} : typeof(f) - if isa(t,Type) - return _methods_by_ftype(Tuple{ft, t.parameters...}, lim) - else - return _methods_by_ftype(Tuple{ft, t...}, lim) - end + tt = isa(t,Type) ? Tuple{ft, t.parameters...} : Tuple{ft, t...} + return _methods_by_ftype(tt, lim) +end +function methods_including_ambiguous(f::ANY, t::ANY) + ft = isa(f,Type) ? Type{f} : typeof(f) + tt = isa(t,Type) ? Tuple{ft, t.parameters...} : Tuple{ft, t...} + return ccall(:jl_matching_methods, Any, (Any,Cint,Cint), tt, -1, 1) end function _methods_by_ftype(t::ANY, lim) tp = t.parameters @@ -194,11 +196,11 @@ function _methods_by_ftype(t::ANY, lim) return _methods(Any[tp...], length(tp), lim, []) end # TODO: the following can return incorrect answers that the above branch would have corrected - return ccall(:jl_matching_methods, Any, (Any,Int32), t, lim) + return ccall(:jl_matching_methods, Any, (Any,Cint,Cint), t, lim, 0) end function _methods(t::Array,i,lim::Integer,matching::Array{Any,1}) if i == 0 - new = ccall(:jl_matching_methods, Any, (Any,Int32), Tuple{t...}, lim) + new = ccall(:jl_matching_methods, Any, (Any,Cint,Cint), Tuple{t...}, lim, 0) new === false && return false append!(matching, new::Array{Any,1}) else @@ -437,8 +439,13 @@ end function isambiguous(m1::Method, m2::Method) ti = typeintersect(m1.sig, m2.sig) - ml = _methods_by_ftype(ti, 1) - ml == false && return true - m = ml[1][3] - !(m.sig <: ti) + ti === Bottom && return false + ml = _methods_by_ftype(ti, -1) + isempty(ml) && return true + for m in ml + if ti <: m[3].sig + return false + end + end + return true end diff --git a/base/replutil.jl b/base/replutil.jl index bb0c44c55f6da..4a4cc101d244e 100644 --- a/base/replutil.jl +++ b/base/replutil.jl @@ -167,7 +167,7 @@ function showerror(io::IO, ex::MethodError) is_arg_types = isa(ex.args, DataType) arg_types = is_arg_types ? ex.args : typesof(ex.args...) f = ex.f - meth = methods(f, arg_types) + meth = methods_including_ambiguous(f, arg_types) if length(meth) > 1 return showerror_ambiguous(io, meth, f, arg_types) end diff --git a/src/gf.c b/src/gf.c index 9fd2e8a7efc17..8202ae907fdb1 100644 --- a/src/gf.c +++ b/src/gf.c @@ -356,7 +356,7 @@ static jl_tupletype_t *join_tsig(jl_tupletype_t *tt, jl_tupletype_t *sig) } static jl_value_t *ml_matches(union jl_typemap_t ml, int offs, - jl_tupletype_t *type, int lim); + jl_tupletype_t *type, int lim, int include_ambiguous); static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *cache, jl_value_t *parent, jl_tupletype_t *type, jl_tupletype_t *origtype, @@ -560,7 +560,7 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *ca temp2 = (jl_value_t*)type; } if (need_guard_entries) { - temp = ml_matches(mt->defs, 0, type, -1); // TODO: use MAX_UNSPECIALIZED_CONFLICTS? + temp = ml_matches(mt->defs, 0, type, -1, 0); // TODO: use MAX_UNSPECIALIZED_CONFLICTS? int guards = 0; if (temp == jl_false) { cache_with_orig = 1; @@ -749,6 +749,7 @@ static int check_ambiguous_visitor(jl_typemap_entry_t *oldentry, struct typemap_ struct ambiguous_matches_env *closure = container_of(closure0, struct ambiguous_matches_env, match); if (oldentry == closure->newentry) return 0; // finished once it finds the method that was just inserted + // TODO: instead, maybe stop once we hit something newentry is definitely more specific than union jl_typemap_t map = closure->defs; jl_tupletype_t *type = (jl_tupletype_t*)closure->match.type; @@ -1006,7 +1007,7 @@ jl_lambda_info_t *jl_method_lookup(jl_methtable_t *mt, jl_value_t **args, size_t return sf; } -JL_DLLEXPORT jl_value_t *jl_matching_methods(jl_tupletype_t *types, int lim); +JL_DLLEXPORT jl_value_t *jl_matching_methods(jl_tupletype_t *types, int lim, int include_ambiguous); // compile-time method lookup jl_lambda_info_t *jl_get_specialization1(jl_tupletype_t *types) @@ -1025,7 +1026,7 @@ jl_lambda_info_t *jl_get_specialization1(jl_tupletype_t *types) // might match. also be conservative with tuples rather than trying // to analyze them in detail. if (ti == (jl_value_t*)jl_datatype_type || jl_is_tuple_type(ti)) { - jl_value_t *matches = jl_matching_methods(types, 1); + jl_value_t *matches = jl_matching_methods(types, 1, 0); if (matches == jl_false) return NULL; break; @@ -1717,6 +1718,7 @@ struct ml_matches_env { jl_value_t *t; // results: array of svec(argtypes, params, Method) jl_svec_t *matc; // current working svec int lim; + int include_ambiguous; // whether ambiguous matches should be included }; static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersection_env *closure0) { @@ -1732,18 +1734,11 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio assert(meth); int skip = 0; size_t len = jl_array_len(closure->t); - // skip over any previously-added or less specific methods - // (this method might have been added previously through meth->ambig) - for (i = 0; i < len; i++) { - jl_method_t *priormeth = (jl_method_t*)jl_svecref(jl_cellref(closure->t, i), 2); - if (priormeth == meth) { - skip = 1; - break; - } - if (closure->lim >= 0) { - // we can skip this match if the types are already covered - // by a prior (more specific) match. but only do this in - // the "limited" mode used by type inference. + if (closure->lim >= 0) { + // we can skip this match if the types are already covered + // by a prior (more specific) match. but only do this in + // the "limited" mode used by type inference. + for (i = 0; i < len; i++) { jl_value_t *prior_ti = jl_svecref(jl_cellref(closure->t, i), 0); // in issue #13007 we incorrectly set skip=1 here, due to // Type{_<:T} ∩ (UnionAll S Type{T{S}}) = Type{T{S}} @@ -1768,35 +1763,6 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio */ } if (!skip) { - if (closure->lim >= 0 && len >= closure->lim) { - closure->t = (jl_value_t*)jl_false; - return 0; // terminate search - } - closure->matc = jl_svec(3, closure->match.ti, closure->match.env, meth); - if (len == 0) { - closure->t = (jl_value_t*)jl_alloc_cell_1d(1); - jl_cellset(closure->t, 0, (jl_value_t*)closure->matc); - } - else { - jl_cell_1d_push((jl_array_t*)closure->t, (jl_value_t*)closure->matc); - } - // Add ambiguous calls - if (meth->ambig != jl_nothing) { - jl_svec_t *env = NULL; - JL_GC_PUSH1(&env); - for (size_t j = 0; j < jl_array_len(meth->ambig); j++) { - jl_method_t *mambig = (jl_method_t*)jl_cellref(meth->ambig, j); - env = jl_emptysvec; - jl_value_t *mti = jl_type_intersection_matching((jl_value_t*)mambig->sig, - (jl_value_t*)closure->match.type, - &env, jl_emptysvec); - if (mti != (jl_value_t*)jl_bottom_type) { - jl_cell_1d_push((jl_array_t*)closure->t, - (jl_value_t*)jl_svec(3, mti, env, mambig)); - } - } - JL_GC_POP(); - } /* Check whether all static parameters matched. If not, then we have an argument type like Vector{T{Int,_}}, and a signature like @@ -1820,13 +1786,65 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio break; } } + int done = 0, return_this_match = 1; // (type ∩ ml->sig == type) ⇒ (type ⊆ ml->sig) // NOTE: jl_subtype check added in case the intersection is // over-approximated. if (matched_all_typevars && jl_types_equal(closure->match.ti, closure->match.type) && jl_subtype(closure->match.type, (jl_value_t*)ml->sig, 0)) { - return 0; // terminate visiting method list + done = 1; // terminate visiting method list + // here we have reached a definition that fully covers the arguments. + // however, if there are ambiguities this method might not actually + // match, so we shouldn't add it to the results. + if (meth->ambig != jl_nothing) { + jl_svec_t *env = NULL; + JL_GC_PUSH1(&env); + for (size_t j = 0; j < jl_array_len(meth->ambig); j++) { + jl_method_t *mambig = (jl_method_t*)jl_cellref(meth->ambig, j); + env = jl_emptysvec; + jl_value_t *mti = jl_type_intersection_matching((jl_value_t*)closure->match.type, + (jl_value_t*)mambig->sig, + &env, mambig->tvars); + if (mti != (jl_value_t*)jl_bottom_type) { + if (closure->include_ambiguous) { + int k; + for(k=0; k < len; k++) { + if ((jl_value_t*)mambig == jl_svecref(jl_cellref(closure->t, k), 2)) + break; + } + if (k >= len) { + if (len == 0) { + closure->t = (jl_value_t*)jl_alloc_cell_1d(0); + } + jl_cell_1d_push((jl_array_t*)closure->t, + (jl_value_t*)jl_svec(3, mti, env, mambig)); + len++; + } + } + else { + return_this_match = 0; + break; + } + } + } + JL_GC_POP(); + } + } + if (return_this_match) { + if (closure->lim >= 0 && len >= closure->lim) { + closure->t = (jl_value_t*)jl_false; + return 0; // terminate search + } + closure->matc = jl_svec(3, closure->match.ti, closure->match.env, meth); + if (len == 0) { + closure->t = (jl_value_t*)jl_alloc_cell_1d(1); + jl_cellset(closure->t, 0, (jl_value_t*)closure->matc); + } + else { + jl_cell_1d_push((jl_array_t*)closure->t, (jl_value_t*)closure->matc); + } } + if (done) return 0; } return 1; } @@ -1837,7 +1855,7 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio // Returns a match as an array of svec(argtypes, static_params, Method). // See below for the meaning of lim. static jl_value_t *ml_matches(union jl_typemap_t defs, int offs, - jl_tupletype_t *type, int lim) + jl_tupletype_t *type, int lim, int include_ambiguous) { size_t l = jl_svec_len(type->parameters); jl_value_t *va = NULL; @@ -1857,6 +1875,7 @@ static jl_value_t *ml_matches(union jl_typemap_t defs, int offs, env.t = jl_an_empty_cell; env.matc = NULL; env.lim = lim; + env.include_ambiguous = include_ambiguous; JL_GC_PUSH4(&env.t, &env.matc, &env.match.env, &env.match.ti); jl_typemap_intersection_visitor(defs, offs, &env.match); JL_GC_POP(); @@ -1870,7 +1889,7 @@ static jl_value_t *ml_matches(union jl_typemap_t defs, int offs, // // lim is the max # of methods to return. if there are more, returns jl_false. // -1 for no limit. -JL_DLLEXPORT jl_value_t *jl_matching_methods(jl_tupletype_t *types, int lim) +JL_DLLEXPORT jl_value_t *jl_matching_methods(jl_tupletype_t *types, int lim, int include_ambiguous) { assert(jl_nparams(types) > 0); if (jl_tparam0(types) == jl_bottom_type) @@ -1879,7 +1898,7 @@ JL_DLLEXPORT jl_value_t *jl_matching_methods(jl_tupletype_t *types, int lim) jl_methtable_t *mt = ((jl_datatype_t*)jl_tparam0(types))->name->mt; if (mt == NULL) return (jl_value_t*)jl_alloc_cell_1d(0); - return ml_matches(mt->defs, 0, types, lim); + return ml_matches(mt->defs, 0, types, lim, include_ambiguous); } #ifdef __cplusplus diff --git a/test/ambiguous.jl b/test/ambiguous.jl index 87d6aa2670306..12052b290bbbd 100644 --- a/test/ambiguous.jl +++ b/test/ambiguous.jl @@ -24,7 +24,8 @@ for m in mt end @test length(methods(ambig, (Int, Int))) == 1 -@test length(methods(ambig, (UInt8, Int))) == 2 +@test length(methods(ambig, (UInt8, Int))) == 0 +@test length(Base.methods_including_ambiguous(ambig, (UInt8, Int))) == 2 @test ambig("hi", "there") == 1 @test ambig(3.1, 3.2) == 5 @@ -49,8 +50,6 @@ code_native(io, ambig, (Int, Int)) # Test that ambiguous cases fail appropriately @test precompile(ambig, (UInt8, Int)) == false cfunction(ambig, Int, (UInt8, Int)) # test for a crash (doesn't throw an error) -@test length(code_lowered(ambig, (UInt8, Int))) == 2 -@test length(code_typed(ambig, (UInt8, Int))) == 2 @test_throws ErrorException code_llvm(io, ambig, (UInt8, Int)) @test_throws ErrorException code_native(io, ambig, (UInt8, Int)) @@ -110,4 +109,36 @@ ambs = detect_ambiguities(Ambig4) # Test that Core and Base are free of ambiguities @test isempty(detect_ambiguities(Core, Base; imported=true)) +amb_1(::Int8, ::Int) = 1 +amb_1(::Integer, x) = 2 +amb_1(x, ::Int) = 3 +# if there is an ambiguity with some methods and not others, `methods` +# should return just the non-ambiguous ones, i.e. the ones that could actually +# be called. +@test length(methods(amb_1, Tuple{Integer, Int})) == 1 + +amb_2(::Int, y) = 1 +amb_2(x, ::Int) = 2 +amb_2(::Int8, y) = 3 +@test length(methods(amb_2)) == 3 # make sure no duplicates + +amb_3(::Int8, ::Int8) = 1 +amb_3(::Int16, ::Int16) = 2 +amb_3(::Integer, ::Integer) = 3 +amb_3(::Integer, x) = 4 +amb_3(x, ::Integer) = 5 +# ambiguous definitions exist, but are covered by multiple more specific definitions +let ms = methods(amb_3).ms + @test !Base.isambiguous(ms[4], ms[5]) +end + +amb_4(::Int8, ::Int8) = 1 +amb_4(::Int16, ::Int16) = 2 +amb_4(::Integer, x) = 4 +amb_4(x, ::Integer) = 5 +# as above, but without sufficient definition coverage +let ms = methods(amb_4).ms + @test Base.isambiguous(ms[3], ms[4]) +end + nothing