typeintersect: fix UnionAll unaliasing bug caused by innervars. (#5…

…3553)

typeintersect: fix `UnionAll` unaliasing bug caused by innervars.

src/subtype.c

@@ -881,10 +881,20 @@ static jl_unionall_t *unalias_unionall(jl_unionall_t *u, jl_stenv_t *e)
  // in the environment, rename to get a fresh var.
  JL_GC_PUSH1(&u);
  while (btemp != NULL) {
- if (btemp->var == u->var ||
- // outer var can only refer to inner var if bounds changed
+ int aliased = btemp->var == u->var ||
+ // outer var can only refer to inner var if bounds changed (mainly for subtyping path)
  (btemp->lb != btemp->var->lb && jl_has_typevar(btemp->lb, u->var)) ||
- (btemp->ub != btemp->var->ub && jl_has_typevar(btemp->ub, u->var))) {
+ (btemp->ub != btemp->var->ub && jl_has_typevar(btemp->ub, u->var));
+ if (!aliased && btemp->innervars != NULL) {
+ for (size_t i = 0; i < jl_array_len(btemp->innervars); i++) {
+ jl_tvar_t *ivar = (jl_tvar_t*)jl_array_ptr_ref(btemp->innervars, i);
+ if (ivar == u->var) {
+ aliased = 1;
+ break;
+ }
+ }
+ }
+ if (aliased) {
  u = jl_rename_unionall(u);
  break;
  }
@@ -2839,7 +2849,7 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
 
  // I. Handle indirect innervars (make them behave like direct innervars).
  // 1) record if btemp->lb/ub has indirect innervars.
- // 2) substitute `vb->var` with `varval`/`varval`
+ // 2) substitute `vb->var` with `varval`/`newvar`
  // note: We only store the innervar in the outmost `varbinding`,
  // thus we must check all inner env to ensure the recording/substitution
  // is complete
@@ -2903,6 +2913,7 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
  }
  envind++;
  }
+ // FIXME: innervar that depend on `ivar` should also be updated.
  }
  }
  }
@@ -3018,7 +3029,8 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
  }
 
  if (vb->innervars != NULL) {
- for (size_t i = 0; i < jl_array_nrows(vb->innervars); i++) {
+ size_t len = jl_array_nrows(vb->innervars), count = 0;
+ for (size_t i = 0; i < len; i++) {
  jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
  // the `btemp->prev` walk is only giving a sort of post-order guarantee (since we are
  // iterating 2 trees at once), so once we set `wrap`, there might remain other branches
@@ -3032,11 +3044,45 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
  if (wrap) {
  if (wrap->innervars == NULL)
  wrap->innervars = jl_alloc_array_1d(jl_array_any_type, 0);
+ // FIXME: `var`'s dependence should also be pushed into `wrap->innervars`.
  jl_array_ptr_1d_push(wrap->innervars, (jl_value_t*)var);
+ jl_array_ptr_set(vb->innervars, i, (jl_value_t*)NULL);
+ }
+ }
+ for (size_t i = 0; i < len; i++) {
+ jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
+ if (var) {
+ if (count < i)
+ jl_array_ptr_set(vb->innervars, count, (jl_value_t*)var);
+ count++;
+ }
+ }
+ if (count != len)
+ jl_array_del_end(vb->innervars, len - count);
+ if (res != jl_bottom_type) {
+ while (count > 1) {
+ int changed = 0;
+ // Now need to re-sort the vb->innervars using the partial-ordering predicate `jl_has_typevar`.
+ // If this is slow, we could possibly switch to a simpler graph sort than this triple loop, such as Tarjan's SCC.
+ // But for now we use a variant on selection sort for partial-orders.
+ for (size_t i = 0; i < count - 1; i++) {
+ jl_tvar_t *vari = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
+ for (size_t j = i+1; j < count; j++) {
+ jl_tvar_t *varj = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, j);
+ if (jl_has_typevar(varj->lb, vari) || jl_has_typevar(varj->ub, vari)) {
+ jl_array_ptr_set(vb->innervars, j, (jl_value_t*)vari);
+ jl_array_ptr_set(vb->innervars, i, (jl_value_t*)varj);
+ changed = 1;
+ break;
+ }
+ }
+ if (changed) break;
+ }
+ if (!changed) break;
  }
- else if (res != jl_bottom_type) {
- if (jl_has_typevar(res, var))
-  res = jl_type_unionall((jl_tvar_t*)var, res);
+ for (size_t i = 0; i < count; i++) {
+ jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
+ res = jl_type_unionall(var, res);
  }
  }
  }
@@ -3056,23 +3102,16 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
 static jl_value_t *intersect_unionall_(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8_t R, int param, jl_varbinding_t *vb)
 {
  jl_varbinding_t *btemp = e->vars;
- // if the var for this unionall (based on identity) already appears somewhere
- // in the environment, rename to get a fresh var.
- // TODO: might need to look inside types in btemp->lb and btemp->ub
  int envsize = 0;
  while (btemp != NULL) {
  envsize++;
  if (envsize > 120) {
  vb->limited = 1;
  return t;
  }
- if (btemp->var == u->var || btemp->lb == (jl_value_t*)u->var ||
- btemp->ub == (jl_value_t*)u->var) {
- u = jl_rename_unionall(u);
- break;
- }
  btemp = btemp->prev;
  }
+ u = unalias_unionall(u, e);
  JL_GC_PUSH1(&u);
  vb->var = u->var;
  e->vars = vb;

test/subtype.jl

@@ -2572,6 +2572,32 @@ let a = Tuple{Union{Nothing, Type{Pair{T1}} where T1}}
  @test !Base.has_free_typevars(typeintersect(a, b))
 end
 
+#issue 53366
+let Y = Tuple{Val{T}, Val{Val{T}}} where T
+ A = Val{Val{T}} where T
+ T = TypeVar(:T, UnionAll(A.var, Val{A.var}))
+ B = UnionAll(T, Val{T})
+ X = Tuple{A, B}
+ @testintersect(X, Y, !Union{})
+end
+
+#issue 53621 (requires assertions enabled)
+abstract type A53621{T, R, C, U} <: AbstractSet{Union{C, U}} end
+struct T53621{T, R<:Real, C, U} <: A53621{T, R, C, U} end
+let
+ U = TypeVar(:U)
+ C = TypeVar(:C)
+ T = TypeVar(:T)
+ R = TypeVar(:R)
+ CC = TypeVar(:CC, Union{C, U})
+ UU = TypeVar(:UU, Union{C, U})
+ S1 = UnionAll(T, UnionAll(R, Type{UnionAll(C, UnionAll(U, T53621{T, R, C, U}))}))
+ S2 = UnionAll(C, UnionAll(U, UnionAll(CC, UnionAll(UU, UnionAll(T, UnionAll(R, T53621{T, R, CC, UU}))))))
+ S = Tuple{S1, S2}
+ T = Tuple{Type{T53621{T, R}}, AbstractSet{T}} where {T, R}
+ @testintersect(S, T, !Union{})
+end
+
 #issue 53371
 struct T53371{A,B,C,D,E} end
 S53371{A} = Union{Int, <:A}