Skip to content

Commit

Permalink
typeintersect: fix incorrect innervar handling under circular env
Browse files Browse the repository at this point in the history
  • Loading branch information
@N5N3
N5N3 committed May 22, 2024
1 parent 3beb168 commit 15bc055
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 20 deletions.
115 changes: 95 additions & 20 deletions src/subtype.c
View file
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 @@ -3004,17 +3063,23 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
// 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 +3117,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 Down Expand Up @@ -3104,6 +3176,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
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

0 comments on commit 15bc055

Please sign in to comment.