Skip to content

Commit 2c22634

Browse files
N5N3Drvi
authored andcommitted
typeintersect: fix UnionAll unaliasing bug caused by innervars. (JuliaLang#53553)
typeintersect: fix `UnionAll` unaliasing bug caused by innervars. (cherry picked from commit 56f1c8a)
1 parent d3268f8 commit 2c22634

File tree

2 files changed

+81
-16
lines changed

2 files changed

+81
-16
lines changed

src/subtype.c

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -872,10 +872,20 @@ static jl_unionall_t *unalias_unionall(jl_unionall_t *u, jl_stenv_t *e)
872872
// in the environment, rename to get a fresh var.
873873
JL_GC_PUSH1(&u);
874874
while (btemp != NULL) {
875-
if (btemp->var == u->var ||
876-
// outer var can only refer to inner var if bounds changed
875+
int aliased = btemp->var == u->var ||
876+
// outer var can only refer to inner var if bounds changed (mainly for subtyping path)
877877
(btemp->lb != btemp->var->lb && jl_has_typevar(btemp->lb, u->var)) ||
878-
(btemp->ub != btemp->var->ub && jl_has_typevar(btemp->ub, u->var))) {
878+
(btemp->ub != btemp->var->ub && jl_has_typevar(btemp->ub, u->var));
879+
if (!aliased && btemp->innervars != NULL) {
880+
for (size_t i = 0; i < jl_array_len(btemp->innervars); i++) {
881+
jl_tvar_t *ivar = (jl_tvar_t*)jl_array_ptr_ref(btemp->innervars, i);
882+
if (ivar == u->var) {
883+
aliased = 1;
884+
break;
885+
}
886+
}
887+
}
888+
if (aliased) {
879889
u = jl_rename_unionall(u);
880890
break;
881891
}
@@ -2833,7 +2843,7 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
28332843

28342844
// I. Handle indirect innervars (make them behave like direct innervars).
28352845
// 1) record if btemp->lb/ub has indirect innervars.
2836-
// 2) subtitute `vb->var` with `varval`/`varval`
2846+
// 2) substitute `vb->var` with `varval`/`newvar`
28372847
// note: We only store the innervar in the outmost `varbinding`,
28382848
// thus we must check all inner env to ensure the recording/subtitution
28392849
// is complete
@@ -2897,6 +2907,7 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
28972907
}
28982908
envind++;
28992909
}
2910+
// FIXME: innervar that depend on `ivar` should also be updated.
29002911
}
29012912
}
29022913
}
@@ -3012,7 +3023,8 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
30123023
}
30133024

30143025
if (vb->innervars != NULL) {
3015-
for (size_t i = 0; i < jl_array_len(vb->innervars); i++) {
3026+
size_t len = jl_array_nrows(vb->innervars), count = 0;
3027+
for (size_t i = 0; i < len; i++) {
30163028
jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
30173029
// the `btemp->prev` walk is only giving a sort of post-order guarantee (since we are
30183030
// iterating 2 trees at once), so once we set `wrap`, there might remain other branches
@@ -3026,11 +3038,45 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
30263038
if (wrap) {
30273039
if (wrap->innervars == NULL)
30283040
wrap->innervars = jl_alloc_array_1d(jl_array_any_type, 0);
3041+
// FIXME: `var`'s dependence should also be pushed into `wrap->innervars`.
30293042
jl_array_ptr_1d_push(wrap->innervars, (jl_value_t*)var);
3043+
jl_array_ptr_set(vb->innervars, i, (jl_value_t*)NULL);
3044+
}
3045+
}
3046+
for (size_t i = 0; i < len; i++) {
3047+
jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
3048+
if (var) {
3049+
if (count < i)
3050+
jl_array_ptr_set(vb->innervars, count, (jl_value_t*)var);
3051+
count++;
3052+
}
3053+
}
3054+
if (count != len)
3055+
jl_array_del_end(vb->innervars, len - count);
3056+
if (res != jl_bottom_type) {
3057+
while (count > 1) {
3058+
int changed = 0;
3059+
// Now need to re-sort the vb->innervars using the partial-ordering predicate `jl_has_typevar`.
3060+
// If this is slow, we could possibly switch to a simpler graph sort than this triple loop, such as Tarjan's SCC.
3061+
// But for now we use a variant on selection sort for partial-orders.
3062+
for (size_t i = 0; i < count - 1; i++) {
3063+
jl_tvar_t *vari = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
3064+
for (size_t j = i+1; j < count; j++) {
3065+
jl_tvar_t *varj = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, j);
3066+
if (jl_has_typevar(varj->lb, vari) || jl_has_typevar(varj->ub, vari)) {
3067+
jl_array_ptr_set(vb->innervars, j, (jl_value_t*)vari);
3068+
jl_array_ptr_set(vb->innervars, i, (jl_value_t*)varj);
3069+
changed = 1;
3070+
break;
3071+
}
3072+
}
3073+
if (changed) break;
3074+
}
3075+
if (!changed) break;
30303076
}
3031-
else if (res != jl_bottom_type) {
3032-
if (jl_has_typevar(res, var))
3033-
res = jl_type_unionall((jl_tvar_t*)var, res);
3077+
for (size_t i = 0; i < count; i++) {
3078+
jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i);
3079+
res = jl_type_unionall(var, res);
30343080
}
30353081
}
30363082
}
@@ -3050,23 +3096,16 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
30503096
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)
30513097
{
30523098
jl_varbinding_t *btemp = e->vars;
3053-
// if the var for this unionall (based on identity) already appears somewhere
3054-
// in the environment, rename to get a fresh var.
3055-
// TODO: might need to look inside types in btemp->lb and btemp->ub
30563099
int envsize = 0;
30573100
while (btemp != NULL) {
30583101
envsize++;
30593102
if (envsize > 120) {
30603103
vb->limited = 1;
30613104
return t;
30623105
}
3063-
if (btemp->var == u->var || btemp->lb == (jl_value_t*)u->var ||
3064-
btemp->ub == (jl_value_t*)u->var) {
3065-
u = jl_rename_unionall(u);
3066-
break;
3067-
}
30683106
btemp = btemp->prev;
30693107
}
3108+
u = unalias_unionall(u, e);
30703109
JL_GC_PUSH1(&u);
30713110
vb->var = u->var;
30723111
e->vars = vb;

test/subtype.jl

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2563,6 +2563,32 @@ let a = Tuple{Union{Nothing, Type{Pair{T1}} where T1}}
25632563
@test !Base.has_free_typevars(typeintersect(a, b))
25642564
end
25652565

2566+
#issue 53366
2567+
let Y = Tuple{Val{T}, Val{Val{T}}} where T
2568+
A = Val{Val{T}} where T
2569+
T = TypeVar(:T, UnionAll(A.var, Val{A.var}))
2570+
B = UnionAll(T, Val{T})
2571+
X = Tuple{A, B}
2572+
@testintersect(X, Y, !Union{})
2573+
end
2574+
2575+
#issue 53621 (requires assertions enabled)
2576+
abstract type A53621{T, R, C, U} <: AbstractSet{Union{C, U}} end
2577+
struct T53621{T, R<:Real, C, U} <: A53621{T, R, C, U} end
2578+
let
2579+
U = TypeVar(:U)
2580+
C = TypeVar(:C)
2581+
T = TypeVar(:T)
2582+
R = TypeVar(:R)
2583+
CC = TypeVar(:CC, Union{C, U})
2584+
UU = TypeVar(:UU, Union{C, U})
2585+
S1 = UnionAll(T, UnionAll(R, Type{UnionAll(C, UnionAll(U, T53621{T, R, C, U}))}))
2586+
S2 = UnionAll(C, UnionAll(U, UnionAll(CC, UnionAll(UU, UnionAll(T, UnionAll(R, T53621{T, R, CC, UU}))))))
2587+
S = Tuple{S1, S2}
2588+
T = Tuple{Type{T53621{T, R}}, AbstractSet{T}} where {T, R}
2589+
@testintersect(S, T, !Union{})
2590+
end
2591+
25662592
#issue 53371
25672593
struct T53371{A,B,C,D,E} end
25682594
S53371{A} = Union{Int, <:A}

0 commit comments

Comments
 (0)