Skip to content

Commit 278f86a

Browse files
vtjnashKristofferC
vtjnash
authored and
KristofferC
committed
ambiguity detection: more optimal code order (#48846)
(cherry picked from commit ff5bd4b)
1 parent 6c05d8b commit 278f86a

File tree

1 file changed

+82
-25
lines changed

1 file changed

+82
-25
lines changed

‎src/gf.c

Lines changed: 82 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3435,22 +3435,9 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
34353435
}
34363436
if (ti != jl_bottom_type) {
34373437
disjoint = 0;
3438-
// m and m2 are ambiguous, but let's see if we can find another method (m3)
3439-
// that dominates their intersection, and means we can ignore this
3440-
size_t k;
3441-
for (k = i; k > 0; k--) {
3442-
jl_method_match_t *matc3 = (jl_method_match_t*)jl_array_ptr_ref(env.t, k - 1);
3443-
jl_method_t *m3 = matc3->method;
3444-
if ((jl_subtype(ti, m3->sig) || (isect2 && jl_subtype(isect2, m3->sig)))
3445-
&& jl_type_morespecific((jl_value_t*)m3->sig, (jl_value_t*)m->sig)
3446-
&& jl_type_morespecific((jl_value_t*)m3->sig, (jl_value_t*)m2->sig))
3447-
break;
3448-
}
3449-
if (k == 0) {
3450-
ambig_groupid[j - 1] = i; // ambiguity covering range [i:j)
3451-
isect2 = NULL;
3452-
break;
3453-
}
3438+
ambig_groupid[j - 1] = i; // ambiguity covering range [i:j)
3439+
isect2 = NULL;
3440+
break;
34543441
}
34553442
isect2 = NULL;
34563443
}
@@ -3529,19 +3516,89 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
35293516
// Compute whether anything could be ambiguous by seeing if any two
35303517
// remaining methods in the result are in the same ambiguity group.
35313518
assert(len > 0);
3532-
uint32_t agid = ambig_groupid[0];
3533-
for (i = 1; i < len; i++) {
3534-
if (!skip[i]) {
3535-
if (agid == ambig_groupid[i]) {
3536-
has_ambiguity = 1;
3537-
break;
3519+
if (!has_ambiguity) {
3520+
// quick test
3521+
uint32_t agid = ambig_groupid[0];
3522+
for (i = 1; i < len; i++) {
3523+
if (!skip[i]) {
3524+
if (agid == ambig_groupid[i]) {
3525+
has_ambiguity = 1;
3526+
break;
3527+
}
3528+
agid = ambig_groupid[i];
3529+
}
3530+
}
3531+
// laborious test, checking for existence and coverage of m3
3532+
if (has_ambiguity) {
3533+
// some method is ambiguous, but let's see if we can find another method (m3)
3534+
// outside of the ambiguity group that dominates any ambiguous methods,
3535+
// and means we can ignore this for has_ambiguity
3536+
has_ambiguity = 0;
3537+
for (i = 0; i < len; i++) {
3538+
if (skip[i])
3539+
continue;
3540+
uint32_t agid = ambig_groupid[i];
3541+
jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(env.t, i);
3542+
jl_method_t *m = matc->method;
3543+
int subt = matc->fully_covers == FULLY_COVERS; // jl_subtype((jl_value_t*)type, (jl_value_t*)m->sig)
3544+
for (j = agid; j < len && ambig_groupid[j] == agid; j++) {
3545+
// n.b. even if we skipped them earlier, they still might
3546+
// contribute to the ambiguities (due to lock of transitivity of
3547+
// morespecific over subtyping)
3548+
if (j == i)
3549+
continue;
3550+
jl_method_match_t *matc2 = (jl_method_match_t*)jl_array_ptr_ref(env.t, j);
3551+
jl_method_t *m2 = matc2->method;
3552+
int subt2 = matc2->fully_covers == FULLY_COVERS; // jl_subtype((jl_value_t*)type, (jl_value_t*)m2->sig)
3553+
// if they aren't themselves simply ordered
3554+
if (jl_type_morespecific((jl_value_t*)m->sig, (jl_value_t*)m2->sig) ||
3555+
jl_type_morespecific((jl_value_t*)m2->sig, (jl_value_t*)m->sig))
3556+
continue;
3557+
jl_value_t *ti;
3558+
if (subt) {
3559+
ti = (jl_value_t*)matc2->spec_types;
3560+
isect2 = NULL;
3561+
}
3562+
else if (subt2) {
3563+
ti = (jl_value_t*)matc->spec_types;
3564+
isect2 = NULL;
3565+
}
3566+
else {
3567+
jl_type_intersection2((jl_value_t*)matc->spec_types, (jl_value_t*)matc2->spec_types, &env.match.ti, &isect2);
3568+
ti = env.match.ti;
3569+
}
3570+
// and their intersection contributes to the ambiguity cycle
3571+
if (ti != jl_bottom_type) {
3572+
// now look for a third method m3 outside of this ambiguity group that fully resolves this intersection
3573+
size_t k;
3574+
for (k = agid; k > 0; k--) {
3575+
jl_method_match_t *matc3 = (jl_method_match_t*)jl_array_ptr_ref(env.t, k);
3576+
jl_method_t *m3 = matc3->method;
3577+
if ((jl_subtype(ti, m3->sig) || (isect2 && jl_subtype(isect2, m3->sig)))
3578+
&& jl_type_morespecific((jl_value_t*)m3->sig, (jl_value_t*)m->sig)
3579+
&& jl_type_morespecific((jl_value_t*)m3->sig, (jl_value_t*)m2->sig)) {
3580+
//if (jl_subtype(matc->spec_types, ti) || jl_subtype(matc->spec_types, matc3->m3->sig))
3581+
// // check if it covered not only this intersection, but all intersections with matc
3582+
// // if so, we do not need to check all of them separately
3583+
// j = len;
3584+
break;
3585+
}
3586+
}
3587+
if (k == 0)
3588+
has_ambiguity = 1;
3589+
isect2 = NULL;
3590+
}
3591+
if (has_ambiguity)
3592+
break;
3593+
}
3594+
if (has_ambiguity)
3595+
break;
35383596
}
3539-
agid = ambig_groupid[i];
35403597
}
35413598
}
35423599
// If we're only returning possible matches, now filter out any method
35433600
// whose intersection is fully ambiguous with the group it is in.
3544-
if (!include_ambiguous) {
3601+
if (!include_ambiguous && has_ambiguity) {
35453602
for (i = 0; i < len; i++) {
35463603
if (skip[i])
35473604
continue;
@@ -3559,7 +3616,7 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
35593616
int subt2 = matc2->fully_covers == FULLY_COVERS; // jl_subtype((jl_value_t*)type, (jl_value_t*)m2->sig)
35603617
// if their intersection contributes to the ambiguity cycle
35613618
if (subt || subt2 || !jl_has_empty_intersection((jl_value_t*)ti, m2->sig)) {
3562-
// and the contribution of m is ambiguous with the portion of the cycle from m2
3619+
// and the contribution of m is fully ambiguous with the portion of the cycle from m2
35633620
if (subt2 || jl_subtype((jl_value_t*)ti, m2->sig)) {
35643621
// but they aren't themselves simply ordered (here
35653622
// we don't consider that a third method might be

0 commit comments

Comments
 (0)