Subtype: avoid some false alarm in subtype_unionall

The current check is not correct if we set the typevar's bounds to an unwrapped `UnionAll`. Use `var_occurs_inside_skip` to skip the inside check.
N5N3 · Dec 16, 2022 · b5f7982 · b5f7982
1 parent b6f32bc
commit b5f7982
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 18 deletions.
diff --git a/src/subtype.c b/src/subtype.c
@@ -733,7 +733,12 @@ static jl_value_t *fix_inferred_var_bound(jl_tvar_t *var, jl_value_t *ty JL_MAYB
     return ty;
 }
 
-static int var_occurs_inside(jl_value_t *v, jl_tvar_t *var, int inside, int want_inv) JL_NOTSAFEPOINT;
+static int var_occurs_inside_skip(jl_value_t *v, jl_tvar_t *var, int inside, int want_inv, jl_value_t *skip) JL_NOTSAFEPOINT;
+
+static int var_occurs_inside(jl_value_t *v, jl_tvar_t *var, int inside, int want_inv) JL_NOTSAFEPOINT
+{
+    return var_occurs_inside_skip(v, var, inside, want_inv, NULL);
+}
 
 typedef int (*tvar_callback)(void*, int8_t, jl_stenv_t *, int);
 
@@ -813,13 +818,14 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8
 
     jl_varbinding_t *btemp = e->vars;
     if (vb.lb != vb.ub) {
+        jl_value_t *skip = jl_unwrap_unionall(u->body);
         while (btemp != NULL) {
             jl_value_t *vu = btemp->ub;
             jl_value_t *vl = btemp->lb;
             // TODO: this takes a significant amount of time
             if (btemp->depth0 != vb.depth0 &&
-                ((vu != (jl_value_t*)vb.var && btemp->var->ub != vu && var_occurs_inside(vu, vb.var, 0, 1)) ||
-                 (vl != (jl_value_t*)vb.var && btemp->var->lb != vl && var_occurs_inside(vl, vb.var, 0, 1)))) {
+                ((vu != (jl_value_t*)vb.var && btemp->var->ub != vu && var_occurs_inside_skip(vu, vb.var, 0, 1, skip)) ||
+                 (vl != (jl_value_t*)vb.var && btemp->var->lb != vl && var_occurs_inside_skip(vl, vb.var, 0, 1, skip)))) {
                 ans = 0; break;
             }
             btemp = btemp->prev;
@@ -2426,37 +2432,37 @@ static jl_value_t *intersect_var(jl_tvar_t *b, jl_value_t *a, jl_stenv_t *e, int
 // test whether `var` occurs inside constructors. `want_inv` tests only inside
 // invariant constructors. `inside` means we are currently inside a constructor of the
 // requested kind.
-static int var_occurs_inside(jl_value_t *v, jl_tvar_t *var, int inside, int want_inv) JL_NOTSAFEPOINT
+static int var_occurs_inside_skip(jl_value_t *v, jl_tvar_t *var, int inside, int want_inv, jl_value_t *skip) JL_NOTSAFEPOINT
 {
-    if (v == (jl_value_t*)var) {
+    if (v == (jl_value_t*)var)
         return inside;
-    }
-    else if (jl_is_uniontype(v)) {
-        return var_occurs_inside(((jl_uniontype_t*)v)->a, var, inside, want_inv) ||
-            var_occurs_inside(((jl_uniontype_t*)v)->b, var, inside, want_inv);
-    }
+    if (jl_is_uniontype(v))
+        return var_occurs_inside_skip(((jl_uniontype_t*)v)->a, var, inside, want_inv, skip) ||
+            var_occurs_inside_skip(((jl_uniontype_t*)v)->b, var, inside, want_inv, skip);
+    if (skip && in_union(skip, v))
+        return 0;
     else if (jl_is_unionall(v)) {
         jl_unionall_t *ua = (jl_unionall_t*)v;
         if (ua->var == var)
             return 0;
-        if (var_occurs_inside(ua->var->lb, var, inside, want_inv) || var_occurs_inside(ua->var->ub, var, inside, want_inv))
+        if (var_occurs_inside_skip(ua->var->lb, var, inside, want_inv, skip) || var_occurs_inside_skip(ua->var->ub, var, inside, want_inv, skip))
             return 1;
-        return var_occurs_inside(ua->body, var, inside, want_inv);
+        return var_occurs_inside_skip(ua->body, var, inside, want_inv, skip);
     }
     else if (jl_is_vararg(v)) {
         jl_vararg_t *vm = (jl_vararg_t*)v;
         if (vm->T) {
-            if (var_occurs_inside(vm->T, var, inside || !want_inv, want_inv))
+            if (var_occurs_inside_skip(vm->T, var, inside || !want_inv, want_inv, skip))
                 return 1;
-            return vm->N && var_occurs_inside(vm->N, var, 1, want_inv);
+            return vm->N && var_occurs_inside_skip(vm->N, var, 1, want_inv, skip);
         }
     }
     else if (jl_is_datatype(v)) {
         size_t i;
         int istuple = jl_is_tuple_type(v);
         for (i=0; i < jl_nparams(v); i++) {
             int ins_i = inside || !want_inv || !istuple;
-            if (var_occurs_inside(jl_tparam(v,i), var, ins_i, want_inv))
+            if (var_occurs_inside_skip(jl_tparam(v,i), var, ins_i, want_inv, skip))
                 return 1;
         }
     }

diff --git a/test/subtype.jl b/test/subtype.jl
@@ -2208,6 +2208,19 @@ T46784{B<:Val, M<:AbstractMatrix} = Tuple{<:Union{B, <:Val{<:B}}, M, Union{Abstr
 @testintersect(T46784{T,S} where {T,S}, T46784, !Union{})
 @test_broken T46784 <: T46784{T,S} where {T,S}
 
+# issue 24333
+@test Type{Union{Ref,Cvoid}} <: Type{Union{T,Cvoid}} where T
+@test Type{Union{Pair,Cvoid}} <: Type{Union{T,Cvoid}} where T
+@test Type{Union{Val{Val{T}} where {T},Cvoid}} <: Type{Union{T,Cvoid}} where T
+
+# issue 47654
+Vec47654{T} = Union{AbstractVector{T}, AbstractVector{Union{T,Nothing}}}
+struct Wrapper47654{T, V<:Vec47654{T}}
+    v::V
+end
+abstract type P47654{A} end
+@test Wrapper47654{P47654, Vector{Union{P47654,Nothing}}} <: Wrapper47654
+
 @testset "known subtype/intersect issue" begin
     #issue 45874
     # Causes a hang due to jl_critical_error calling back into malloc...
@@ -2248,9 +2261,6 @@ T46784{B<:Val, M<:AbstractMatrix} = Tuple{<:Union{B, <:Val{<:B}}, M, Union{Abstr
     #issue 26487
     @test_broken typeintersect(Tuple{Type{Tuple{T,Val{T}}}, Val{T}} where T, Tuple{Type{Tuple{Val{T},T}}, Val{T}} where T) <: Any
 
-    # issue 24333
-    @test_broken (Type{Union{Ref,Cvoid}} <: Type{Union{T,Cvoid}} where T)
-
     # issue 22123
     t1 = Ref{Ref{Ref{Union{Int64, T}}} where T}
     t2 = Ref{Ref{Ref{Union{T, S}}} where T} where S