Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

typeintersect: fix UnionAll unaliasing bug caused by innervars. #53553

Merged
merged 2 commits into from
Mar 6, 2024
Merged

typeintersect: fix UnionAll unaliasing bug caused by innervars. #53553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a556af2
typeintersect: fuse `unalias_unionall`
N5N3 Mar 2, 2024
02f27c2
typeintersect: more organized innervar wrapping
N5N3 Mar 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 55 additions & 16 deletions src/subtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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++) {
N5N3 marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
}
Expand All @@ -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;
Expand Down
26 changes: 26 additions & 0 deletions test/subtype.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading