ambiguity detection: more optimal code order (#48846)

src/gf.c

@@ -3435,22 +3435,9 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
  }
  if (ti != jl_bottom_type) {
  disjoint = 0;
- // 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 = i; 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[j - 1] = i; // ambiguity covering range [i:j)
- isect2 = NULL;
- break;
- }
+ ambig_groupid[j - 1] = i; // ambiguity covering range [i:j)
+ isect2 = NULL;
+ break;
  }
  isect2 = NULL;
  }
@@ -3529,19 +3516,89 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
  // Compute whether anything could be ambiguous by seeing if any two
  // remaining methods in the result are in the same ambiguity group.
  assert(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;
+ if (!has_ambiguity) {
+ // quick test
+ 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];
+ }
+ }
+ // laborious test, checking for existence and coverage of m3
+ if (has_ambiguity) {
+ // some method is ambiguous, but let's see if we can find another method (m3)
+ // outside of the ambiguity group that dominates any ambiguous methods,
+ // and means we can ignore this for has_ambiguity
+ has_ambiguity = 0;
+ for (i = 0; i < len; i++) {
+ if (skip[i])
+ continue;
+ uint32_t agid = ambig_groupid[i];
+ jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i);
+ jl_method_t *m = matc->method;
+ int subt = matc->fully_covers == FULLY_COVERS; // jl_subtype((jl_value_t*)type, (jl_value_t*)m->sig)
+ for (j = agid; j < len && ambig_groupid[j] == agid; j++) {
+ // n.b. even if we skipped them earlier, they still might
+ // contribute to the ambiguities (due to lock of transitivity of
+ // morespecific over subtyping)
+ if (j == i)
+ continue;
+ jl_method_match_t *matc2 = (jl_method_match_t*)jl_array_ptr_ref(env.t, j);
+ jl_method_t *m2 = matc2->method;
+ int subt2 = matc2->fully_covers == FULLY_COVERS; // jl_subtype((jl_value_t*)type, (jl_value_t*)m2->sig)
+ // if they aren't themselves simply ordered
+ if (jl_type_morespecific((jl_value_t*)m->sig, (jl_value_t*)m2->sig) ||
+ jl_type_morespecific((jl_value_t*)m2->sig, (jl_value_t*)m->sig))
+ continue;
+ 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;
+ }
+ // and their intersection contributes to the ambiguity cycle
+ if (ti != jl_bottom_type) {
+ // now look for a third method m3 outside of this ambiguity group that fully resolves this intersection
+ size_t k;
+ for (k = agid; k > 0; k--) {
+ jl_method_match_t *matc3 = (jl_method_match_t*)jl_array_ptr_ref(env.t, k);
+ 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)) {
+ //if (jl_subtype(matc->spec_types, ti) || jl_subtype(matc->spec_types, matc3->m3->sig))
+ // // check if it covered not only this intersection, but all intersections with matc
+ // // if so, we do not need to check all of them separately
+ // j = len;
+ break;
+ }
+ }
+ if (k == 0)
+ has_ambiguity = 1;
+ isect2 = NULL;
+ }
+ if (has_ambiguity)
+ break;
+ }
+ if (has_ambiguity)
+ break;
  }
- agid = ambig_groupid[i];
  }
  }
  // If we're only returning possible matches, now filter out any method
  // whose intersection is fully ambiguous with the group it is in.
- if (!include_ambiguous) {
+ if (!include_ambiguous && has_ambiguity) {
  for (i = 0; i < len; i++) {
  if (skip[i])
  continue;
@@ -3559,7 +3616,7 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
  int subt2 = matc2->fully_covers == FULLY_COVERS; // jl_subtype((jl_value_t*)type, (jl_value_t*)m2->sig)
  // if their intersection contributes to the ambiguity cycle
  if (subt || subt2 || !jl_has_empty_intersection((jl_value_t*)ti, m2->sig)) {
- // and the contribution of m is ambiguous with the portion of the cycle from m2
+ // and the contribution of m is fully ambiguous with the portion of the cycle from m2
  if (subt2 || jl_subtype((jl_value_t*)ti, m2->sig)) {
  // but they aren't themselves simply ordered (here
  // we don't consider that a third method might be