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 incorrect innervar handling under circular env #54545

Merged
merged 3 commits into from
May 28, 2024
Merged

typeintersect: fix incorrect innervar handling under circular env #54545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15bc055
typeintersect: fix incorrect innervar handling under circular env
N5N3 May 22, 2024
51b2b54
make analyzegc happy
N5N3 May 22, 2024
0bccf43
fix miss counting of `ivarbinding`
N5N3 May 23, 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
124 changes: 101 additions & 23 deletions src/subtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2814,6 +2814,50 @@ static jl_value_t *omit_bad_union(jl_value_t *u, jl_tvar_t *t)
return res;
}

// TODO: fuse with reachable_var?
static int has_typevar_via_flatten_env(jl_value_t *x, jl_tvar_t *t, jl_ivarbinding_t *allvars, int8_t *checked) {
if (jl_is_unionall(x)) {
jl_tvar_t *var = ((jl_unionall_t *)x)->var;
if (has_typevar_via_flatten_env(var->lb, t, allvars, checked) ||
has_typevar_via_flatten_env(var->ub, t, allvars, checked))
return 1;
return has_typevar_via_flatten_env(((jl_unionall_t *)x)->body, t, allvars, checked);
}
else if (jl_is_uniontype(x)) {
return has_typevar_via_flatten_env(((jl_uniontype_t *)x)->a, t, allvars, checked) ||
has_typevar_via_flatten_env(((jl_uniontype_t *)x)->b, t, allvars, checked);
}
else if (jl_is_vararg(x)) {
jl_vararg_t *v = (jl_vararg_t *)x;
return (v->T && has_typevar_via_flatten_env(v->T, t, allvars, checked)) ||
(v->N && has_typevar_via_flatten_env(v->N, t, allvars, checked));
}
else if (jl_is_datatype(x)) {
for (size_t i = 0; i < jl_nparams(x); i++) {
if (has_typevar_via_flatten_env(jl_tparam(x, i), t, allvars, checked))
return 1;
}
return 0;
}
else if (jl_is_typevar(x)) {
if (t == (jl_tvar_t *)x)
return 1;
size_t ind = 0;
jl_ivarbinding_t *itemp = allvars;
while (itemp && *itemp->var != (jl_tvar_t *)x)
{
ind++;
itemp = itemp->next;
}
if (itemp == NULL || checked[ind])
return 0;
checked[ind] = 1;
return has_typevar_via_flatten_env(*itemp->lb, t, allvars, checked) ||
has_typevar_via_flatten_env(*itemp->ub, t, allvars, checked);
}
return 0;
}

// Caller might not have rooted `res`
static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbinding_t *vb, jl_unionall_t *u, jl_stenv_t *e)
{
Expand Down Expand Up @@ -2911,8 +2955,11 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
// remove/replace/rewrap free occurrences of this var in the environment
int wrapped = 0;
jl_ivarbinding_t *pwrap = NULL;
int vcount = icount + current_env_length(e);
int8_t *checked = (int8_t *)alloca(vcount);
for (jl_ivarbinding_t *btemp = allvars, *pbtemp = NULL; btemp != NULL; btemp = btemp->next) {
int bdepth0 = btemp->root->depth0;
int innerflag = 0;
ivar = *btemp->var;
ilb = *btemp->lb;
iub = *btemp->ub;
Expand All @@ -2934,18 +2981,9 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
else if (ilb == (jl_value_t*)vb->var) {
*btemp->lb = vb->lb;
}
else if (bdepth0 == vb->depth0 && !jl_has_typevar(vb->lb, ivar) && !jl_has_typevar(vb->ub, ivar)) {
// if our variable is T, and some outer variable has constraint S = Ref{T},
// move the `where T` outside `where S` instead of putting it here. issue #21243.
if (newvar != vb->var)
*btemp->lb = jl_substitute_var(ilb, vb->var, (jl_value_t*)newvar);
if (!wrapped) pwrap = pbtemp;
wrapped = 1;
}
else {
*btemp->lb = jl_new_struct(jl_unionall_type, vb->var, ilb);
innerflag |= 1;
}
assert((jl_value_t*)ivar != *btemp->lb);
}
if (jl_has_typevar(iub, vb->var)) {
assert(btemp->root->var == ivar || bdepth0 == vb->depth0);
Expand All @@ -2971,14 +3009,35 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
// Tuple{Float64, T3, T4} where {S3, T3<:Tuple{S3}, T4<:S3}
*btemp->ub = vb->ub;
}
else if (bdepth0 == vb->depth0 && !jl_has_typevar(vb->lb, ivar) && !jl_has_typevar(vb->ub, ivar)) {
if (newvar != vb->var)
*btemp->ub = jl_substitute_var(iub, vb->var, (jl_value_t*)newvar);
if (!wrapped) pwrap = pbtemp;
wrapped = 1;
else {
innerflag |= 2;
}
else
*btemp->ub = jl_new_struct(jl_unionall_type, vb->var, iub);
if (innerflag) {
memset(checked, 0, vcount);
if (bdepth0 != vb->depth0 ||
has_typevar_via_flatten_env(vb->lb, ivar, allvars, checked) ||
has_typevar_via_flatten_env(vb->ub, ivar, allvars, checked)) {
if (innerflag & 1)
*btemp->lb = jl_new_struct(jl_unionall_type, vb->var, ilb);
if (innerflag & 2)
*btemp->ub = jl_new_struct(jl_unionall_type, vb->var, iub);
}
else {
assert(btemp->root != vb);
// if our variable is T, and some outer variable has constraint S = Ref{T},
// move the `where T` outside `where S` instead of putting it here. issue #21243.
if (newvar != vb->var) {
if (innerflag & 1)
*btemp->lb = jl_substitute_var(ilb, vb->var, (jl_value_t*)newvar);
if (innerflag & 2)
*btemp->ub = jl_substitute_var(iub, vb->var, (jl_value_t*)newvar);
}
if (!wrapped)
pwrap = pbtemp;
wrapped = 1;
}
}
assert((jl_value_t*)ivar != *btemp->lb);
assert((jl_value_t*)ivar != *btemp->ub);
}
pbtemp = btemp;
Expand All @@ -2997,24 +3056,31 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
pwrap->next = inew;
else
allvars = inew;
vcount++;
}

// Re-sort the innervar inside the (reversed) var list.
// `jl_has_typevar` is used as the partial-ordering predicate.
// If this is slow, we could possibly switch to a simpler graph sort, such as Tarjan's SCC.
if (icount > 0) {
jl_ivarbinding_t *pib1 = NULL;
#ifndef NDEBUG
size_t sort_count = 0;
#endif
while (1) {
jl_ivarbinding_t *ib1 = pib1 == NULL ? allvars : pib1->next;
if (ib1 == NULL) break;
if (jl_has_free_typevars(*ib1->lb) || jl_has_free_typevars(*ib1->ub)) {
assert((++sort_count) <= (vcount * (vcount + 1)) >> 1);
int lbfree = jl_has_free_typevars(*ib1->lb);
int ubfree = jl_has_free_typevars(*ib1->ub);
if (lbfree || ubfree) {
int changed = 0;
jl_ivarbinding_t *pib2 = ib1, *ib2 = ib1->next;
while (ib2 != NULL) {
int isinnervar = ib2->root->var != *ib2->var;
if (isinnervar && ib1->root->depth0 == ib2->root->depth0 &&
(jl_has_typevar(*ib1->lb, *ib2->var) ||
jl_has_typevar(*ib1->ub, *ib2->var))) {
((lbfree && jl_has_typevar(*ib1->lb, *ib2->var)) ||
(ubfree && jl_has_typevar(*ib1->ub, *ib2->var)))) {
pib2->next = ib2->next;
ib2->next = ib1;
ib2->root = ib1->root;
Expand Down Expand Up @@ -3052,6 +3118,13 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
if (jl_has_typevar(iub, ivar))
*btemp2->ub = jl_substitute_var(iub, ivar, nivar);
}
if (!wrapped && !varval) {
// newvar also needs bounds substitution.
if (jl_has_typevar(vb->lb, ivar))
vb->lb = jl_substitute_var(vb->lb, ivar, nivar);
if (jl_has_typevar(vb->ub, ivar))
vb->ub = jl_substitute_var(vb->ub, ivar, nivar);
}
*btemp1->var = (jl_tvar_t *)nivar;
}
}
Expand All @@ -3067,11 +3140,13 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
}
if (root != vb) icount--;
if (root->innervars != NULL) {
size_t len = jl_array_nrows(root->innervars);
jl_array_t *rinnervars = root->innervars;
JL_GC_PROMISE_ROOTED(rinnervars);
size_t len = jl_array_nrows(rinnervars);
if (icount > len)
jl_array_grow_end(root->innervars, icount - len);
jl_array_grow_end(rinnervars, icount - len);
if (icount < len)
jl_array_del_end(root->innervars, len - icount);
jl_array_del_end(rinnervars, len - icount);
}
else if (icount > 0) {
root->innervars = jl_alloc_array_1d(jl_array_any_type, icount);
Expand Down Expand Up @@ -3104,6 +3179,9 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
res = jl_bottom_type;
}
else {
// re-fresh newvar if bounds changed.
if (vb->lb != newvar->lb || vb->ub != newvar->ub)
newvar = jl_new_typevar(newvar->name, vb->lb, vb->ub);
if (newvar != vb->var)
res = jl_substitute_var(res, vb->var, (jl_value_t*)newvar);
varval = (jl_value_t*)newvar;
Expand Down
7 changes: 7 additions & 0 deletions test/subtype.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2650,3 +2650,10 @@ let S = Tuple{Val, Val{T}} where {T}, R = Tuple{Val{Val{T}}, Val{T}} where {T},
@testintersect(Tuple{Val{A}, A} where {B, A<:Union{Val{B}, GenericMemory{B}}}, S{1}, R{1})
@testintersect(Tuple{Val{A}, A} where {B, A<:Union{Val{B}, GenericMemory{:not_atomic,Int,B}}}, S{1}, R{1})
end

#issue 54516
let S = Tuple{Val{<:T}, Union{Int,T}} where {T},
T = Tuple{Union{Int,T}, Val{<:T}} where {T}
@testintersect(S, T, !Union{})
@test !Base.has_free_typevars(typeintersect(S, T))
end
Loading