Skip to content

Commit d9df1ba

Browse files
committed
gf: improve check_ambiguous_visitor filtering
relax the check here to more closely match the comment
1 parent 0c23e0c commit d9df1ba

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

src/gf.c

+12-6
Original file line numberDiff line numberDiff line change
@@ -1214,21 +1214,27 @@ static int check_ambiguous_visitor(jl_typemap_entry_t *oldentry, struct typemap_
12141214
}
12151215
// TODO: also complete detection of a specificity cycle
12161216

1217-
// if this type-intersection is exact, then we can
12181217
// see if the intersection is covered by another existing entry
12191218
// that will resolve the ambiguity (by being more specific than either)
1220-
// (if type-morespecific made a mistake, this also might end up finding
1219+
// (in some cases, this also might end up finding
12211220
// that isect == type or isect == sig and return the original match)
1222-
if (reorder && jl_subtype(isect, (jl_value_t*)sig) && jl_subtype(isect, (jl_value_t*)type)) {
1221+
if (reorder) {
12231222
size_t world = closure->newentry->min_world;
12241223
if (oldentry->min_world > world)
12251224
world = oldentry->min_world;
1225+
int exact1 = jl_subtype(isect, (jl_value_t*)sig);
1226+
int exact2 = jl_subtype(isect, (jl_value_t*)type);
1227+
// TODO: we might like to use `subtype = exact1 && exact2` here, but check_disabled_ambiguous_visitor
1228+
// won't be able to handle that, so we might end up making some unnecessary mambig entries here
1229+
// but I don't have any examples of such
12261230
jl_typemap_entry_t *l = jl_typemap_assoc_by_type(
12271231
closure->defs, isect, NULL, /*subtype*/0, /*offs*/0,
12281232
world, /*max_world_mask*/0);
1229-
if (l != NULL) { // ok, intersection is covered
1230-
// also need to check that `l` isn't the `after` entry
1231-
if (l != after) {
1233+
//assert((!subtype || l != after) && "bad typemap lookup result"); // should find `before` first
1234+
if (l != NULL && l != before && l != after) {
1235+
if ((exact1 || jl_type_morespecific_no_subtype((jl_value_t*)l->sig, (jl_value_t*)sig)) && // no subtype since `l->sig >: isect >: sig`
1236+
(exact2 || jl_type_morespecific_no_subtype((jl_value_t*)l->sig, (jl_value_t*)type))) { // no subtype since `l->sig >: isect >: type`
1237+
// ok, intersection is already covered by a more specific method
12321238
reorder = 0; // this lack of ordering doesn't matter (unless we delete this method--then it will)
12331239
shadowed = 0; // we wouldn't find anything that mattered
12341240
}

0 commit comments

Comments
 (0)