diff --git a/base/reflection.jl b/base/reflection.jl index 031426531b9e1..480f35091d958 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -1363,6 +1363,8 @@ Determine whether two methods `m1` and `m2` may be ambiguous for some call signature. This test is performed in the context of other methods of the same function; in isolation, `m1` and `m2` might be ambiguous, but if a third method resolving the ambiguity has been defined, this returns `false`. +Alternatively, in isolation `m1` and `m2` might be ordered, but if a third +method cannot be sorted with them, they may cause an ambiguity together. For parametric types, the `ambiguous_bottom` keyword argument controls whether `Union{}` counts as an ambiguous intersection of type parameters – when `true`, @@ -1389,27 +1391,87 @@ false ``` """ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false) - # TODO: eagerly returning `morespecific` is wrong, and fails to consider - # the possibility of an ambiguity caused by a third method: - # see the precise algorithm in ml_matches for a more correct computation - if m1 === m2 || morespecific(m1.sig, m2.sig) || morespecific(m2.sig, m1.sig) - return false - end + m1 === m2 && return false ti = typeintersect(m1.sig, m2.sig) - (ti <: m1.sig && ti <: m2.sig) || return false # XXX: completely wrong, obviously ti === Bottom && return false - if !ambiguous_bottom - has_bottom_parameter(ti) && return false - end - matches = _methods_by_ftype(ti, -1, typemax(UInt)) - for match in matches - m = match.method - m === m1 && continue - m === m2 && continue - if ti <: m.sig && morespecific(m.sig, m1.sig) && morespecific(m.sig, m2.sig) + function inner(ti) + ti === Bottom && return false + if !ambiguous_bottom + has_bottom_parameter(ti) && return false + end + min = UInt[typemin(UInt)] + max = UInt[typemax(UInt)] + has_ambig = Int32[0] + ms = _methods_by_ftype(ti, -1, typemax(UInt), true, min, max, has_ambig) + has_ambig[] == 0 && return false + if !ambiguous_bottom + filter!(ms) do m + return !has_bottom_parameter(m.spec_types) + end + end + # if ml-matches reported the existence of an ambiguity over their + # intersection, see if both m1 and m2 may be involved in it + have_m1 = have_m2 = false + for match in ms + m = match.method + m === m1 && (have_m1 = true) + m === m2 && (have_m2 = true) + end + if !have_m1 || !have_m2 + # ml-matches did not need both methods to expose the reported ambiguity + return false + end + if !ambiguous_bottom + # since we're intentionally ignoring certain ambiguities (via the + # filter call above), see if we can now declare the intersection fully + # covered even though it is partially ambiguous over Union{} as a type + # parameter somewhere + minmax = nothing + for match in ms + m = match.method + match.fully_covers || continue + if minmax === nothing || morespecific(m.sig, minmax.sig) + minmax = m + end + end + if minmax === nothing + return true + end + for match in ms + m = match.method + m === minmax && continue + if match.fully_covers + if !morespecific(minmax.sig, m.sig) + return true + end + else + if morespecific(m.sig, minmax.sig) + return true + end + end + end return false end + return true + end + if !(ti <: m1.sig && ti <: m2.sig) + # When type-intersection fails, it's often also not commutative. Thus + # checking the reverse may allow detecting ambiguity solutions + # correctly in more cases (and faster). + ti2 = typeintersect(m2.sig, m1.sig) + if ti2 <: m1.sig && ti2 <: m2.sig + ti = ti2 + elseif ti != ti2 + # TODO: this would be the correct way to handle this case, but + # people complained so we don't do it + # inner(ti2) || return false + return false # report that the type system failed to decide if it was ambiguous by saying they definitely aren't + else + return false # report that the type system failed to decide if it was ambiguous by saying they definitely aren't + end end + inner(ti) || return false + # otherwise type-intersection reported an ambiguity we couldn't solve return true end diff --git a/src/clangsa/GCChecker.cpp b/src/clangsa/GCChecker.cpp index 05974a0358e28..be564a80215e9 100644 --- a/src/clangsa/GCChecker.cpp +++ b/src/clangsa/GCChecker.cpp @@ -1340,7 +1340,8 @@ bool GCChecker::evalCall(const CallExpr *CE, return true; } else if (name == "JL_GC_PUSH1" || name == "JL_GC_PUSH2" || name == "JL_GC_PUSH3" || name == "JL_GC_PUSH4" || - name == "JL_GC_PUSH5" || name == "JL_GC_PUSH6") { + name == "JL_GC_PUSH5" || name == "JL_GC_PUSH6" || + name == "JL_GC_PUSH7") { ProgramStateRef State = C.getState(); // Transform slots to roots, transform values to rooted unsigned NumArgs = CE->getNumArgs(); @@ -1348,8 +1349,7 @@ bool GCChecker::evalCall(const CallExpr *CE, SVal V = C.getSVal(CE->getArg(i)); auto MRV = V.getAs(); if (!MRV) { - report_error(C, - "JL_GC_PUSH with something other than a local variable"); + report_error(C, "JL_GC_PUSH with something other than a local variable"); return true; } const MemRegion *Region = MRV->getRegion(); diff --git a/src/gf.c b/src/gf.c index 322222798e8de..818cb68a8f3ef 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1454,7 +1454,7 @@ static int invalidate_mt_cache(jl_typemap_entry_t *oldentry, void *closure0) } if (intersects) { if (_jl_debug_method_invalidation) { - jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)oldentry); + jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)mi); jl_value_t *loctag = jl_cstr_to_string("invalidate_mt_cache"); JL_GC_PUSH1(&loctag); jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag); @@ -1551,6 +1551,33 @@ JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *metho JL_UNLOCK(&mt->writelock); } +static int jl_type_intersection2(jl_value_t *t1, jl_value_t *t2, jl_value_t **isect, jl_value_t **isect2) +{ + *isect2 = NULL; + *isect = jl_type_intersection(t1, t2); + if (*isect == jl_bottom_type) + return 0; + // determine if type-intersection can be convinced to give a better, non-bad answer + if (!(jl_subtype(*isect, t1) && jl_subtype(*isect, t2))) { + // if the intersection was imprecise, see if we can do + // better by switching the types + *isect2 = jl_type_intersection(t2, t1); + if (*isect2 == jl_bottom_type) { + *isect = jl_bottom_type; + *isect2 = NULL; + return 0; + } + if (jl_subtype(*isect2, t1) && jl_subtype(*isect2, t2)) { + *isect = *isect2; + *isect2 = NULL; + } + else if (jl_types_equal(*isect2, *isect)) { + *isect2 = NULL; + } + } + return 1; +} + JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method, jl_tupletype_t *simpletype) { JL_TIMING(ADD_METHOD); @@ -1564,8 +1591,10 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method size_t max_world = method->primary_world - 1; jl_value_t *loctag = NULL; // debug info for invalidation jl_value_t *isect = NULL; + jl_value_t *isect2 = NULL; + jl_value_t *isect3 = NULL; jl_typemap_entry_t *newentry = NULL; - JL_GC_PUSH5(&oldvalue, &oldmi, &newentry, &loctag, &isect); + JL_GC_PUSH7(&oldvalue, &oldmi, &newentry, &loctag, &isect, &isect2, &isect3); JL_LOCK(&mt->writelock); // first find if we have an existing entry to delete struct jl_typemap_assoc search = {(jl_value_t*)type, method->primary_world, NULL, 0, ~(size_t)0}; @@ -1600,8 +1629,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method size_t ins = 0; for (i = 1; i < na; i += 2) { jl_value_t *backedgetyp = backedges[i - 1]; - isect = jl_type_intersection(backedgetyp, (jl_value_t*)type); - if (isect != jl_bottom_type) { + if (jl_type_intersection2(backedgetyp, (jl_value_t*)type, &isect, &isect2)) { // see if the intersection was actually already fully // covered by anything (method or ambiguity is okay) size_t j; @@ -1609,6 +1637,8 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method jl_method_t *m = d[j]; if (jl_subtype(isect, m->sig)) break; + if (isect2 && jl_subtype(isect2, m->sig)) + break; } if (j != n) isect = jl_bottom_type; @@ -1651,9 +1681,8 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method jl_method_instance_t *mi = jl_atomic_load_relaxed(&data[i]); if (mi == NULL) continue; - isect = jl_type_intersection(m->sig, (jl_value_t*)mi->specTypes); - isect = jl_type_intersection(type, isect); - if (isect != jl_bottom_type) { + isect3 = jl_type_intersection(m->sig, (jl_value_t*)mi->specTypes); + if (jl_type_intersection2(type, isect3, &isect, &isect2)) { if (morespec[j] == (char)morespec_unknown) morespec[j] = (char)jl_type_morespecific(m->sig, type) ? morespec_is : morespec_isnot; if (morespec[j] == (char)morespec_is) @@ -1667,7 +1696,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method size_t k; for (k = 0; k < n; k++) { jl_method_t *m2 = d[k]; - if (m == m2 || !jl_subtype(isect, m2->sig)) + if (m == m2 || !(jl_subtype(isect, m2->sig) || (isect && jl_subtype(isect, m2->sig)))) continue; if (morespec[k] == (char)morespec_unknown) morespec[k] = (char)jl_type_morespecific(m2->sig, type) ? morespec_is : morespec_isnot; @@ -2617,7 +2646,8 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, intersections, world, lim, /* .t = */ jl_an_empty_vec_any, /* .min_valid = */ *min_valid, /* .max_valid = */ *max_valid, /* .matc = */ NULL}; struct jl_typemap_assoc search = {(jl_value_t*)type, world, jl_emptysvec, 1, ~(size_t)0}; - JL_GC_PUSH5(&env.t, &env.matc, &env.match.env, &search.env, &env.match.ti); + jl_value_t *isect2 = NULL; + JL_GC_PUSH6(&env.t, &env.matc, &env.match.env, &search.env, &env.match.ti, &isect2); // check the leaf cache if this type can be in there if (((jl_datatype_t*)unw)->isdispatchtuple) { @@ -2687,84 +2717,85 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, int minmax_ambig = 0; int all_subtypes = 1; if (len > 1) { - // first try to pre-process the results to find the most specific result that fully covers the input - // (since we can do this in linear time, and the rest is O(n^2) - // - first see if this might even be profitable, given the requested output we need to compute + // first try to pre-process the results to find the most specific + // result that fully covers the input, since we can do this in linear + // time, and the rest is O(n^2) + // - first find a candidate for the best of these method results for (i = 0; i < len; i++) { jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); - if (matc->fully_covers != FULLY_COVERS) { + if (matc->fully_covers == FULLY_COVERS) { + jl_method_t *m = matc->method; + if (minmax != NULL) { + jl_method_t *minmaxm = minmax->method; + if (jl_type_morespecific((jl_value_t*)minmaxm->sig, (jl_value_t*)m->sig)) + continue; + } + minmax = matc; + } + else { all_subtypes = 0; - break; } } - if (all_subtypes || !include_ambiguous) { - // - then find a candidate for the best of these method results - // (If we have a reason to compute this. There's no point in - // finding the minmax now, if we still need to examine all - // methods for ambiguities later.) + // - then see if it dominated all of the other choices + if (minmax != NULL) { for (i = 0; i < len; i++) { jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); + if (matc == minmax) + break; if (matc->fully_covers == FULLY_COVERS) { jl_method_t *m = matc->method; - if (minmax != NULL) { - jl_method_t *minmaxm = minmax->method; - if (jl_type_morespecific((jl_value_t*)minmaxm->sig, (jl_value_t*)m->sig)) - continue; - } - minmax = matc; - } - } - // - then see if it dominated all of the other choices - if (minmax != NULL) { - for (i = 0; i < len; i++) { - jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); - if (matc == minmax) + jl_method_t *minmaxm = minmax->method; + if (!jl_type_morespecific((jl_value_t*)minmaxm->sig, (jl_value_t*)m->sig)) { + minmax_ambig = 1; + minmax = NULL; + *has_ambiguity = 1; break; - if (matc->fully_covers == FULLY_COVERS) { - jl_method_t *m = matc->method; - jl_method_t *minmaxm = minmax->method; - if (!jl_type_morespecific((jl_value_t*)minmaxm->sig, (jl_value_t*)m->sig)) { - minmax_ambig = 1; - *has_ambiguity = 1; - if (include_ambiguous) - minmax = NULL; - break; - } } } } - // - it may even dominate some choices that are not subtypes! - // move those into the subtype group, where we're filter them out shortly after - if (!all_subtypes && minmax) { - jl_method_t *minmaxm = minmax->method; - all_subtypes = 1; - for (i = 0; i < len; i++) { - jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); - if (matc->fully_covers != FULLY_COVERS) { - jl_method_t *m = matc->method; - if (jl_type_morespecific((jl_value_t*)minmaxm->sig, (jl_value_t*)m->sig)) - matc->fully_covers = SENTINEL; // put a sentinel value here for sorting - else - all_subtypes = 0; - } + } + // - it may even dominate some choices that are not subtypes! + // move those into the subtype group, where we're filter them out shortly after + // (potentially avoiding reporting these as an ambiguity, and + // potentially allowing us to hit the next fast path) + // - we could always check here if *any* FULLY_COVERS method is + // more-specific (instead of just considering minmax), but that may + // cost much extra and is less likely to help us hit a fast path + // (we will look for this later, when we compute ambig_groupid, for + // correctness) + if (!all_subtypes && minmax != NULL) { + jl_method_t *minmaxm = minmax->method; + all_subtypes = 1; + for (i = 0; i < len; i++) { + jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); + if (matc->fully_covers != FULLY_COVERS) { + jl_method_t *m = matc->method; + if (jl_type_morespecific((jl_value_t*)minmaxm->sig, (jl_value_t*)m->sig)) + matc->fully_covers = SENTINEL; // put a sentinel value here for sorting + else + all_subtypes = 0; } } - // - now we might have a fast-return here, if we see that - // we've already processed all of the possible outputs - if (all_subtypes) { - if (minmax_ambig) { - if (!include_ambiguous) { - len = 0; - env.t = jl_an_empty_vec_any; - } + } + // - now we might have a fast-return here, if we see that + // we've already processed all of the possible outputs + if (all_subtypes) { + if (minmax_ambig) { + if (!include_ambiguous) { + len = 0; + env.t = jl_an_empty_vec_any; } - else { - assert(minmax != NULL); - jl_array_ptr_set(env.t, 0, minmax); - jl_array_del_end((jl_array_t*)env.t, len - 1); - len = 1; + else if (lim == 1) { + JL_GC_POP(); + return jl_false; } } + else { + assert(minmax != NULL); + jl_array_ptr_set(env.t, 0, minmax); + jl_array_del_end((jl_array_t*)env.t, len - 1); + len = 1; + } } } if (len > 1) { @@ -2776,8 +2807,8 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, env.matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); jl_method_t *m = env.matc->method; int subt = env.matc->fully_covers != NOT_FULLY_COVERS; - if (minmax != NULL && subt) { - continue; // already the biggest + if ((minmax != NULL || (minmax_ambig && !include_ambiguous)) && subt) { + continue; // already the biggest (skip will filter others) } for (j = 0; j < i; j++) { jl_method_match_t *matc2 = (jl_method_match_t *)jl_array_ptr_ref(env.t, i - j - 1); @@ -2793,47 +2824,11 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, } jl_array_ptr_set(env.t, i - j, env.matc); } - // final step to finish sort: - // we stopped early with just having all non-subtypes before all - // subtypes, but the case on the boundary might be wrongly placed: - // check for that now - if (!minmax) { - for (i = 0; i < len; i++) { - jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); - if (matc->fully_covers == FULLY_COVERS) - break; - } - for (; i > 0; i--) { - env.matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i - 1); - jl_method_t *m = env.matc->method; - for (j = i; j < len; j++) { - jl_method_match_t *matc2 = (jl_method_match_t*)jl_array_ptr_ref(env.t, j); - jl_method_t *m2 = matc2->method; - if (matc2->fully_covers != FULLY_COVERS) - break; - if (!jl_type_morespecific((jl_value_t*)m2->sig, (jl_value_t*)m->sig)) - break; - jl_array_ptr_set(env.t, j - 1, matc2); - } - if (j == i) - break; - env.matc->fully_covers = SENTINEL; - jl_array_ptr_set(env.t, j - 1, env.matc); - } - } char *skip = (char*)alloca(len); memset(skip, 0, len); - // since we had a minmax method, now may now be able to cleanup some of our sort result - if (minmax_ambig && !include_ambiguous) { - for (i = 0; i < len; i++) { - jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); - if (matc->fully_covers != NOT_FULLY_COVERS) { - skip[i] = 1; - } - } - } - else if (minmax != NULL) { - assert(all_subtypes || !include_ambiguous); + // if we had a minmax method (any subtypes), now may now be able to + // quickly cleanup some of our sort result + if (minmax != NULL || (minmax_ambig && !include_ambiguous)) { for (i = 0; i < len; i++) { jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); if (minmax != matc && matc->fully_covers != NOT_FULLY_COVERS) { @@ -2847,7 +2842,7 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, // as we go, keep a rough count of how many methods are disjoint, which // gives us a lower bound on how many methods we will be returning // and lets us stop early if we reach our limit - int ndisjoint = 0; + int ndisjoint = minmax ? 1 : 0; for (i = 0; i < len; i++) { // can't use skip[i] here, since we still need to make sure the minmax dominates jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i); @@ -2866,10 +2861,40 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, disjoint = 0; ambig_groupid[i] = j - 1; // ambiguity covering range [i:j) } - else if (subt || subt2 || !jl_has_empty_intersection(m->sig, m2->sig)) { - if (!jl_type_morespecific((jl_value_t*)m2->sig, (jl_value_t*)m->sig)) - ambig_groupid[i] = j - 1; // ambiguity covering range [i:j) - disjoint = 0; + else { + jl_value_t *ti; + if (subt) { + ti = (jl_value_t*)matc2->spec_types; + isect2 = NULL; + } + else if (subt2) { + ti = (jl_value_t*)matc->spec_types; + isect2 = NULL; + } + else { + jl_type_intersection2((jl_value_t*)matc->spec_types, (jl_value_t*)matc2->spec_types, &env.match.ti, &isect2); + ti = env.match.ti; + } + if (ti != jl_bottom_type) { + if (!jl_type_morespecific((jl_value_t*)m2->sig, (jl_value_t*)m->sig)) { + // m and m2 are ambiguous, but let's see if we can find another method (m3) + // that dominates their intersection, and means we can ignore this + size_t k; + for (k = j; k > 0; k--) { + jl_method_match_t *matc3 = (jl_method_match_t*)jl_array_ptr_ref(env.t, k - 1); + jl_method_t *m3 = matc3->method; + if ((jl_subtype(ti, m3->sig) || (isect2 && jl_subtype(isect2, m3->sig))) + && jl_type_morespecific((jl_value_t*)m3->sig, (jl_value_t*)m->sig) + && jl_type_morespecific((jl_value_t*)m3->sig, (jl_value_t*)m2->sig)) + break; + } + if (k == 0) { + ambig_groupid[i] = j - 1; // ambiguity covering range [i:j) + } + } + disjoint = 0; + } + isect2 = NULL; } } if (disjoint && lim >= 0) { @@ -2944,20 +2969,17 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, int offs, } } // Compute whether anything could be ambiguous by seeing if any two - // methods in the result are in the same ambiguity group. - for (i = 0; i < len; i++) { - if (!skip[i]) { - uint32_t agid = ambig_groupid[i]; - for (; i < len; i++) { - if (!skip[i]) { - if (agid == ambig_groupid[i]) { - *has_ambiguity = 1; - break; - } - agid = ambig_groupid[i]; + // remaining methods in the result are in the same ambiguity group. + if (len > 0) { + uint32_t agid = ambig_groupid[0]; + for (i = 1; i < len; i++) { + if (!skip[i]) { + if (agid == ambig_groupid[i]) { + *has_ambiguity = 1; + break; } + agid = ambig_groupid[i]; } - break; } } // If we're only returning possible matches, now filter out any method diff --git a/src/julia.h b/src/julia.h index eacc76eb0e0f2..e42797cb280cf 100644 --- a/src/julia.h +++ b/src/julia.h @@ -746,7 +746,7 @@ extern void JL_GC_PUSH2(void *, void *) JL_NOTSAFEPOINT; extern void JL_GC_PUSH3(void *, void *, void *) JL_NOTSAFEPOINT; extern void JL_GC_PUSH4(void *, void *, void *, void *) JL_NOTSAFEPOINT; extern void JL_GC_PUSH5(void *, void *, void *, void *, void *) JL_NOTSAFEPOINT; -extern void JL_GC_PUSH6(void *, void *, void *, void *, void *, void *) JL_NOTSAFEPOINT; +extern void JL_GC_PUSH7(void *, void *, void *, void *, void *, void *, void *) JL_NOTSAFEPOINT; extern void _JL_GC_PUSHARGS(jl_value_t **, size_t) JL_NOTSAFEPOINT; // This is necessary, because otherwise the analyzer considers this undefined // behavior and terminates the exploration @@ -783,6 +783,11 @@ extern void JL_GC_POP() JL_NOTSAFEPOINT; void *__gc_stkf[] = {(void*)JL_GC_ENCODE_PUSH(6), jl_pgcstack, arg1, arg2, arg3, arg4, arg5, arg6}; \ jl_pgcstack = (jl_gcframe_t*)__gc_stkf; +#define JL_GC_PUSH7(arg1, arg2, arg3, arg4, arg5, arg6, arg7) \ + void *__gc_stkf[] = {(void*)JL_GC_ENCODE_PUSH(7), jl_pgcstack, arg1, arg2, arg3, arg4, arg5, arg6, arg7}; \ + jl_pgcstack = (jl_gcframe_t*)__gc_stkf; + + #define JL_GC_PUSHARGS(rts_var,n) \ rts_var = ((jl_value_t**)alloca(((n)+2)*sizeof(jl_value_t*)))+2; \ ((void**)rts_var)[-2] = (void*)JL_GC_ENCODE_PUSHARGS(n); \ diff --git a/stdlib/LinearAlgebra/test/ambiguous_exec.jl b/stdlib/LinearAlgebra/test/ambiguous_exec.jl index 6dce4926f4610..e784c43bfa000 100644 --- a/stdlib/LinearAlgebra/test/ambiguous_exec.jl +++ b/stdlib/LinearAlgebra/test/ambiguous_exec.jl @@ -1,4 +1,21 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license -using Test, LinearAlgebra -@test detect_ambiguities(LinearAlgebra; imported=true, recursive=true) == [] +using Test, LinearAlgebra, SparseArrays +let ambig = detect_ambiguities(LinearAlgebra; recursive=true) + @test isempty(ambig) + ambig = Set{Any}(((m1.sig, m2.sig) for (m1, m2) in ambig)) + expect = [] + good = true + while !isempty(ambig) + sigs = pop!(ambig) + i = findfirst(==(sigs), expect) + if i === nothing + println(stderr, "push!(expect, (", sigs[1], ", ", sigs[2], "))") + good = false + continue + end + deleteat!(expect, i) + end + @test isempty(expect) + @test good +end diff --git a/stdlib/SparseArrays/test/ambiguous_exec.jl b/stdlib/SparseArrays/test/ambiguous_exec.jl index a466f2534794a..d4f1015565168 100644 --- a/stdlib/SparseArrays/test/ambiguous_exec.jl +++ b/stdlib/SparseArrays/test/ambiguous_exec.jl @@ -1,4 +1,21 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license -using Test, SparseArrays -@test detect_ambiguities(SparseArrays; imported=true, recursive=true) == [] +using Test, LinearAlgebra, SparseArrays +let ambig = detect_ambiguities(SparseArrays; recursive=true) + @test isempty(ambig) + ambig = Set{Any}(((m1.sig, m2.sig) for (m1, m2) in ambig)) + expect = [] + good = true + while !isempty(ambig) + sigs = pop!(ambig) + i = findfirst(==(sigs), expect) + if i === nothing + println(stderr, "push!(expect, (", sigs[1], ", ", sigs[2], "))") + good = false + continue + end + deleteat!(expect, i) + end + @test isempty(expect) + @test good +end diff --git a/stdlib/Test/src/Test.jl b/stdlib/Test/src/Test.jl index 7518a8aeb1012..a590f4a2c25a1 100644 --- a/stdlib/Test/src/Test.jl +++ b/stdlib/Test/src/Test.jl @@ -1399,13 +1399,21 @@ function _inferred(ex, mod, allow = :(Union{})) end) end +function is_in_mods(m::Module, recursive::Bool, mods) + while true + m in mods && return true + recursive || return false + p = parentmodule(m) + p === m && return false + m = p + end +end + """ - detect_ambiguities(mod1, mod2...; imported=false, recursive=false, ambiguous_bottom=false) + detect_ambiguities(mod1, mod2...; recursive=false, ambiguous_bottom=false) Returns a vector of `(Method,Method)` pairs of ambiguous methods defined in the specified modules. -Use `imported=true` if you wish to also test functions that were -imported into these modules from elsewhere. Use `recursive=true` to test in all submodules. `ambiguous_bottom` controls whether ambiguities triggered only by @@ -1413,9 +1421,11 @@ Use `recursive=true` to test in all submodules. want to set this to `false`. See [`Base.isambiguous`](@ref). """ function detect_ambiguities(mods...; - imported::Bool = false, recursive::Bool = false, ambiguous_bottom::Bool = false) + @nospecialize mods + ambs = Set{Tuple{Method,Method}}() + mods = collect(mods)::Vector{Module} function sortdefs(m1::Method, m2::Method) ord12 = m1.file < m2.file if !ord12 && (m1.file == m2.file) @@ -1423,102 +1433,90 @@ function detect_ambiguities(mods...; end return ord12 ? (m1, m2) : (m2, m1) end - ambs = Set{Tuple{Method,Method}}() - for mod in mods - for n in names(mod, all = true, imported = imported) - Base.isdeprecated(mod, n) && continue - if !isdefined(mod, n) - println("Skipping ", mod, '.', n) # typically stale exports - continue - end - f = Base.unwrap_unionall(getfield(mod, n)) - if recursive && isa(f, Module) && f !== mod && parentmodule(f) === mod && nameof(f) === n - subambs = detect_ambiguities(f, - imported=imported, recursive=recursive, ambiguous_bottom=ambiguous_bottom) - union!(ambs, subambs) - elseif isa(f, DataType) && isdefined(f.name, :mt) && f.name.mt !== Symbol.name.mt - mt = Base.MethodList(f.name.mt) - for m in mt - ambig = Int32[0] - for match2 in Base._methods_by_ftype(m.sig, -1, typemax(UInt), true, UInt[typemin(UInt)], UInt[typemax(UInt)], ambig) - ambig[1] == 0 && break - m2 = match2.method - if Base.isambiguous(m, m2, ambiguous_bottom=ambiguous_bottom) - push!(ambs, sortdefs(m, m2)) - end + function examine(mt::Core.MethodTable) + for m in Base.MethodList(mt) + is_in_mods(m.module, recursive, mods) || continue + ambig = Int32[0] + ms = Base._methods_by_ftype(m.sig, -1, typemax(UInt), true, UInt[typemin(UInt)], UInt[typemax(UInt)], ambig) + ambig[1] == 0 && continue + for match2 in ms + m2 = match2.method + if !(m === m2 || Base.morespecific(m2.sig, m.sig)) + if Base.isambiguous(m, m2; ambiguous_bottom) + push!(ambs, sortdefs(m, m2)) end end end end end - function is_in_mods(m::Module) - while true - m in mods && return true - recursive || return false - p = parentmodule(m) - p === m && return false - m = p - end - end - let mt = Base.MethodList(Symbol.name.mt) - for m in mt - if is_in_mods(m.module) - ambig = Int32[0] - for match2 in Base._methods_by_ftype(m.sig, -1, typemax(UInt), true, UInt[typemin(UInt)], UInt[typemax(UInt)], ambig) - ambig[1] == 0 && break - m2 = match2.method - if Base.isambiguous(m, m2, ambiguous_bottom=ambiguous_bottom) - push!(ambs, sortdefs(m, m2)) - end - end + work = Base.loaded_modules_array() + while !isempty(work) + mod = pop!(work) + for n in names(mod, all = true) + Base.isdeprecated(mod, n) && continue + if !isdefined(mod, n) + is_in_mods(mod, recursive, mods) && println("Skipping ", mod, '.', n) # typically stale exports + continue + end + f = Base.unwrap_unionall(getfield(mod, n)) + if isa(f, Module) && f !== mod && parentmodule(f) === mod && nameof(f) === n + push!(work, f) + elseif isa(f, DataType) && isdefined(f.name, :mt) && f.name.module === mod && f.name.name === n && f.name.mt !== Symbol.name.mt && f.name.mt !== DataType.name.mt + examine(f.name.mt) end end end + examine(Symbol.name.mt) + examine(DataType.name.mt) return collect(ambs) end """ - detect_unbound_args(mod1, mod2...; imported=false, recursive=false) + detect_unbound_args(mod1, mod2...; recursive=false) Returns a vector of `Method`s which may have unbound type parameters. -Use `imported=true` if you wish to also test functions that were -imported into these modules from elsewhere. Use `recursive=true` to test in all submodules. """ function detect_unbound_args(mods...; - imported::Bool = false, recursive::Bool = false) + @nospecialize mods ambs = Set{Method}() - for mod in mods - for n in names(mod, all = true, imported = imported) + mods = collect(mods)::Vector{Module} + function examine(mt::Core.MethodTable) + for m in Base.MethodList(mt) + has_unbound_vars(m.sig) || continue + is_in_mods(m.module, recursive, mods) || continue + tuple_sig = Base.unwrap_unionall(m.sig)::DataType + if Base.isvatuple(tuple_sig) + params = tuple_sig.parameters[1:(end - 1)] + tuple_sig = Base.rewrap_unionall(Tuple{params...}, m.sig) + mf = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tuple_sig, typemax(UInt)) + if mf !== nothing && mf !== m && mf.sig <: tuple_sig + continue + end + end + push!(ambs, m) + end + end + work = Base.loaded_modules_array() + while !isempty(work) + mod = pop!(work) + for n in names(mod, all = true) Base.isdeprecated(mod, n) && continue if !isdefined(mod, n) - println("Skipping ", mod, '.', n) # typically stale exports + is_in_mods(mod, recursive, mods) && println("Skipping ", mod, '.', n) # typically stale exports continue end f = Base.unwrap_unionall(getfield(mod, n)) - if recursive && isa(f, Module) && f !== mod && parentmodule(f) === mod && nameof(f) === n - subambs = detect_unbound_args(f, imported=imported, recursive=recursive) - union!(ambs, subambs) - elseif isa(f, DataType) && isdefined(f.name, :mt) - mt = Base.MethodList(f.name.mt) - for m in mt - if has_unbound_vars(m.sig) - tuple_sig = Base.unwrap_unionall(m.sig)::DataType - if Base.isvatuple(tuple_sig) - params = tuple_sig.parameters[1:(end - 1)] - tuple_sig = Base.rewrap_unionall(Tuple{params...}, m.sig) - mf = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tuple_sig, typemax(UInt)) - if mf !== nothing && mf !== m && mf.sig <: tuple_sig - continue - end - end - push!(ambs, m) - end - end + if isa(f, Module) && f !== mod && parentmodule(f) === mod && nameof(f) === n + push!(work, f) + elseif isa(f, DataType) && isdefined(f.name, :mt) && f.name.module === mod && f.name.name === n && f.name.mt !== Symbol.name.mt && f.name.mt !== DataType.name.mt + examine(f.name.mt) end end end + examine(Symbol.name.mt) + examine(DataType.name.mt) return collect(ambs) end diff --git a/test/ambiguous.jl b/test/ambiguous.jl index 9c9927ece4b45..22dbf808d62d8 100644 --- a/test/ambiguous.jl +++ b/test/ambiguous.jl @@ -11,8 +11,6 @@ ambig(x::Int, y::Int) = 4 ambig(x::Number, y) = 5 # END OF LINE NUMBER SENSITIVITY -using LinearAlgebra, SparseArrays - # For curmod_* include("testenv.jl") @@ -149,21 +147,45 @@ end ambs = detect_ambiguities(Ambig5) @test length(ambs) == 2 + +using LinearAlgebra, SparseArrays, SuiteSparse + # Test that Core and Base are free of ambiguities # not using isempty so this prints more information when it fails -@test detect_ambiguities(Core, Base; imported=true, recursive=true, ambiguous_bottom=false) == [] -# some ambiguities involving Union{} type parameters are expected, but not required -@test !isempty(detect_ambiguities(Core, Base; imported=true, ambiguous_bottom=true)) +@testset "detect_ambiguities" begin + let ambig = Set{Any}(((m1.sig, m2.sig) for (m1, m2) in detect_ambiguities(Core, Base; recursive=true, ambiguous_bottom=false))) + @test isempty(ambig) + expect = [] + good = true + while !isempty(ambig) + sigs = pop!(ambig) + i = findfirst(==(sigs), expect) + if i === nothing + println(stderr, "push!(expect, (", sigs[1], ", ", sigs[2], "))") + good = false + continue + end + deleteat!(expect, i) + end + @test isempty(expect) + @test good + end -module AmbigStdlib -using Test + # some ambiguities involving Union{} type parameters are expected, but not required + let ambig = Set(detect_ambiguities(Core; recursive=true, ambiguous_bottom=true)) + @test !isempty(ambig) + end -# List standard libraries. Exclude modules such as Main. -modules = [mod for (pkg, mod) in Base.loaded_modules if pkg.uuid !== nothing] + STDLIB_DIR = Sys.STDLIB + STDLIBS = filter!(x -> x != "LinearAlgebra" && x != "SparseArrays" && # Some packages run this test themselves + isfile(joinpath(STDLIB_DIR, x, "src", "$(x).jl")), + readdir(STDLIB_DIR)) -# not using isempty so this prints more information when it fails -@test detect_ambiguities(modules...; imported=true, recursive=true) == [] -end # module + # List standard libraries. Exclude modules such as Main, Base, and Core. + let modules = [mod for (pkg, mod) in Base.loaded_modules if pkg.uuid !== nothing && String(pkg.name) in STDLIBS] + @test isempty(detect_ambiguities(modules...; recursive=true)) + end +end amb_1(::Int8, ::Int) = 1 amb_1(::Integer, x) = 2 @@ -252,7 +274,7 @@ end for f in (Ambig8.f, Ambig8.g) @test length(methods(f, (Integer,))) == 2 # 1 is also acceptable @test length(methods(f, (Signed,))) == 1 # 2 is also acceptable - @test length(Base.methods_including_ambiguous(f, (Signed,))) == 3 + @test length(Base.methods_including_ambiguous(f, (Signed,))) == 2 @test f(0x00) == 1 @test f(Ambig8.Irrational2()) == 2 @test f(MathConstants.γ) == 3 @@ -298,7 +320,6 @@ end @test_broken need_to_handle_undef_sparam == Set() pop!(need_to_handle_undef_sparam, which(Base._cat, Tuple{Any, AbstractArray})) pop!(need_to_handle_undef_sparam, which(Base.byteenv, (Union{AbstractArray{Pair{T,V}, 1}, Tuple{Vararg{Pair{T,V}}}} where {T<:AbstractString,V},))) - pop!(need_to_handle_undef_sparam, which(Base._cat, (Any, SparseArrays._TypedDenseConcatGroup{T} where T))) pop!(need_to_handle_undef_sparam, which(Base.float, Tuple{AbstractArray{Union{Missing, T},N} where {T, N}})) pop!(need_to_handle_undef_sparam, which(Base.zero, Tuple{Type{Union{Missing, T}} where T})) pop!(need_to_handle_undef_sparam, which(Base.one, Tuple{Type{Union{Missing, T}} where T})) @@ -324,8 +345,18 @@ f35983(::Type, ::Type) = 2 @test first(Base.methods_including_ambiguous(f35983, (Any, Any))).sig == Tuple{typeof(f35983), Type, Type} @test length(Base.methods(f35983, (Any, Any))) == 2 @test first(Base.methods(f35983, (Any, Any))).sig == Tuple{typeof(f35983), Type, Type} +let ambig = Int32[0] + ms = Base._methods_by_ftype(Tuple{typeof(f35983), Type, Type}, -1, typemax(UInt), true, UInt[typemin(UInt)], UInt[typemax(UInt)], ambig) + @test length(ms) == 1 + @test ambig[1] == 0 +end f35983(::Type{Int16}, ::Any) = 3 -@test length(Base.methods_including_ambiguous(f35983, (Type, Type))) == 3 +@test length(Base.methods_including_ambiguous(f35983, (Type, Type))) == 2 @test length(Base.methods(f35983, (Type, Type))) == 2 +let ambig = Int32[0] + ms = Base._methods_by_ftype(Tuple{typeof(f35983), Type, Type}, -1, typemax(UInt), true, UInt[typemin(UInt)], UInt[typemax(UInt)], ambig) + @test length(ms) == 2 + @test ambig[1] == 1 +end -nothing # don't return a module from the remote include +nothing