Skip to content

Commit f3f525a

Browse files
N5N3DilumAluthge
N5N3
authored and
DilumAluthge
committed
typeintersect: fix incorrect innervar handling under circular env (#54545)
The infinite loop encountered in #54516 has been traced back to a circular bound during `finish_unionall`. As we insert innervar more eagerly now, the direct `jl_has_typevar` could not find all circularity. To address this, `has_typevar_via_flatten_env` is added to perform thorough check. Although there is some code duplication with `reachable_var`, it could be improved in future refactoring. #54516 also highlighted another free var escaping regression since v1.10. This regression is not solely the result of incomplete checks, it is also caused by the missing final substitution of `vb`'s bound, which has now been corrected. At last, this PR adds an assertion of sorting complexity, which should facilitate the detection of similar issues by PkgEval. close #54516
1 parent 30858f4 commit f3f525a

File tree

2 files changed

+108
-23
lines changed

2 files changed

+108
-23
lines changed

‎src/subtype.c

Lines changed: 101 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2814,6 +2814,50 @@ static jl_value_t *omit_bad_union(jl_value_t *u, jl_tvar_t *t)
28142814
return res;
28152815
}
28162816

2817+
// TODO: fuse with reachable_var?
2818+
static int has_typevar_via_flatten_env(jl_value_t *x, jl_tvar_t *t, jl_ivarbinding_t *allvars, int8_t *checked) {
2819+
if (jl_is_unionall(x)) {
2820+
jl_tvar_t *var = ((jl_unionall_t *)x)->var;
2821+
if (has_typevar_via_flatten_env(var->lb, t, allvars, checked) ||
2822+
has_typevar_via_flatten_env(var->ub, t, allvars, checked))
2823+
return 1;
2824+
return has_typevar_via_flatten_env(((jl_unionall_t *)x)->body, t, allvars, checked);
2825+
}
2826+
else if (jl_is_uniontype(x)) {
2827+
return has_typevar_via_flatten_env(((jl_uniontype_t *)x)->a, t, allvars, checked) ||
2828+
has_typevar_via_flatten_env(((jl_uniontype_t *)x)->b, t, allvars, checked);
2829+
}
2830+
else if (jl_is_vararg(x)) {
2831+
jl_vararg_t *v = (jl_vararg_t *)x;
2832+
return (v->T && has_typevar_via_flatten_env(v->T, t, allvars, checked)) ||
2833+
(v->N && has_typevar_via_flatten_env(v->N, t, allvars, checked));
2834+
}
2835+
else if (jl_is_datatype(x)) {
2836+
for (size_t i = 0; i < jl_nparams(x); i++) {
2837+
if (has_typevar_via_flatten_env(jl_tparam(x, i), t, allvars, checked))
2838+
return 1;
2839+
}
2840+
return 0;
2841+
}
2842+
else if (jl_is_typevar(x)) {
2843+
if (t == (jl_tvar_t *)x)
2844+
return 1;
2845+
size_t ind = 0;
2846+
jl_ivarbinding_t *itemp = allvars;
2847+
while (itemp && *itemp->var != (jl_tvar_t *)x)
2848+
{
2849+
ind++;
2850+
itemp = itemp->next;
2851+
}
2852+
if (itemp == NULL || checked[ind])
2853+
return 0;
2854+
checked[ind] = 1;
2855+
return has_typevar_via_flatten_env(*itemp->lb, t, allvars, checked) ||
2856+
has_typevar_via_flatten_env(*itemp->ub, t, allvars, checked);
2857+
}
2858+
return 0;
2859+
}
2860+
28172861
// Caller might not have rooted `res`
28182862
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)
28192863
{
@@ -2911,8 +2955,11 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
29112955
// remove/replace/rewrap free occurrences of this var in the environment
29122956
int wrapped = 0;
29132957
jl_ivarbinding_t *pwrap = NULL;
2958+
int vcount = icount + current_env_length(e);
2959+
int8_t *checked = (int8_t *)alloca(vcount);
29142960
for (jl_ivarbinding_t *btemp = allvars, *pbtemp = NULL; btemp != NULL; btemp = btemp->next) {
29152961
int bdepth0 = btemp->root->depth0;
2962+
int innerflag = 0;
29162963
ivar = *btemp->var;
29172964
ilb = *btemp->lb;
29182965
iub = *btemp->ub;
@@ -2934,18 +2981,9 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
29342981
else if (ilb == (jl_value_t*)vb->var) {
29352982
*btemp->lb = vb->lb;
29362983
}
2937-
else if (bdepth0 == vb->depth0 && !jl_has_typevar(vb->lb, ivar) && !jl_has_typevar(vb->ub, ivar)) {
2938-
// if our variable is T, and some outer variable has constraint S = Ref{T},
2939-
// move the `where T` outside `where S` instead of putting it here. issue #21243.
2940-
if (newvar != vb->var)
2941-
*btemp->lb = jl_substitute_var(ilb, vb->var, (jl_value_t*)newvar);
2942-
if (!wrapped) pwrap = pbtemp;
2943-
wrapped = 1;
2944-
}
29452984
else {
2946-
*btemp->lb = jl_new_struct(jl_unionall_type, vb->var, ilb);
2985+
innerflag |= 1;
29472986
}
2948-
assert((jl_value_t*)ivar != *btemp->lb);
29492987
}
29502988
if (jl_has_typevar(iub, vb->var)) {
29512989
assert(btemp->root->var == ivar || bdepth0 == vb->depth0);
@@ -2971,14 +3009,35 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
29713009
// Tuple{Float64, T3, T4} where {S3, T3<:Tuple{S3}, T4<:S3}
29723010
*btemp->ub = vb->ub;
29733011
}
2974-
else if (bdepth0 == vb->depth0 && !jl_has_typevar(vb->lb, ivar) && !jl_has_typevar(vb->ub, ivar)) {
2975-
if (newvar != vb->var)
2976-
*btemp->ub = jl_substitute_var(iub, vb->var, (jl_value_t*)newvar);
2977-
if (!wrapped) pwrap = pbtemp;
2978-
wrapped = 1;
3012+
else {
3013+
innerflag |= 2;
29793014
}
2980-
else
2981-
*btemp->ub = jl_new_struct(jl_unionall_type, vb->var, iub);
3015+
if (innerflag) {
3016+
memset(checked, 0, vcount);
3017+
if (bdepth0 != vb->depth0 ||
3018+
has_typevar_via_flatten_env(vb->lb, ivar, allvars, checked) ||
3019+
has_typevar_via_flatten_env(vb->ub, ivar, allvars, checked)) {
3020+
if (innerflag & 1)
3021+
*btemp->lb = jl_new_struct(jl_unionall_type, vb->var, ilb);
3022+
if (innerflag & 2)
3023+
*btemp->ub = jl_new_struct(jl_unionall_type, vb->var, iub);
3024+
}
3025+
else {
3026+
assert(btemp->root != vb);
3027+
// if our variable is T, and some outer variable has constraint S = Ref{T},
3028+
// move the `where T` outside `where S` instead of putting it here. issue #21243.
3029+
if (newvar != vb->var) {
3030+
if (innerflag & 1)
3031+
*btemp->lb = jl_substitute_var(ilb, vb->var, (jl_value_t*)newvar);
3032+
if (innerflag & 2)
3033+
*btemp->ub = jl_substitute_var(iub, vb->var, (jl_value_t*)newvar);
3034+
}
3035+
if (!wrapped)
3036+
pwrap = pbtemp;
3037+
wrapped = 1;
3038+
}
3039+
}
3040+
assert((jl_value_t*)ivar != *btemp->lb);
29823041
assert((jl_value_t*)ivar != *btemp->ub);
29833042
}
29843043
pbtemp = btemp;
@@ -2997,24 +3056,31 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
29973056
pwrap->next = inew;
29983057
else
29993058
allvars = inew;
3059+
vcount++;
30003060
}
30013061

30023062
// Re-sort the innervar inside the (reversed) var list.
30033063
// `jl_has_typevar` is used as the partial-ordering predicate.
30043064
// If this is slow, we could possibly switch to a simpler graph sort, such as Tarjan's SCC.
30053065
if (icount > 0) {
30063066
jl_ivarbinding_t *pib1 = NULL;
3067+
#ifndef NDEBUG
3068+
size_t sort_count = 0;
3069+
#endif
30073070
while (1) {
30083071
jl_ivarbinding_t *ib1 = pib1 == NULL ? allvars : pib1->next;
30093072
if (ib1 == NULL) break;
3010-
if (jl_has_free_typevars(*ib1->lb) || jl_has_free_typevars(*ib1->ub)) {
3073+
assert((++sort_count) <= (vcount * (vcount + 1)) >> 1);
3074+
int lbfree = jl_has_free_typevars(*ib1->lb);
3075+
int ubfree = jl_has_free_typevars(*ib1->ub);
3076+
if (lbfree || ubfree) {
30113077
int changed = 0;
30123078
jl_ivarbinding_t *pib2 = ib1, *ib2 = ib1->next;
30133079
while (ib2 != NULL) {
30143080
int isinnervar = ib2->root->var != *ib2->var;
30153081
if (isinnervar && ib1->root->depth0 == ib2->root->depth0 &&
3016-
(jl_has_typevar(*ib1->lb, *ib2->var) ||
3017-
jl_has_typevar(*ib1->ub, *ib2->var))) {
3082+
((lbfree && jl_has_typevar(*ib1->lb, *ib2->var)) ||
3083+
(ubfree && jl_has_typevar(*ib1->ub, *ib2->var)))) {
30183084
pib2->next = ib2->next;
30193085
ib2->next = ib1;
30203086
ib2->root = ib1->root;
@@ -3052,6 +3118,13 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
30523118
if (jl_has_typevar(iub, ivar))
30533119
*btemp2->ub = jl_substitute_var(iub, ivar, nivar);
30543120
}
3121+
if (!wrapped && !varval) {
3122+
// newvar also needs bounds substitution.
3123+
if (jl_has_typevar(vb->lb, ivar))
3124+
vb->lb = jl_substitute_var(vb->lb, ivar, nivar);
3125+
if (jl_has_typevar(vb->ub, ivar))
3126+
vb->ub = jl_substitute_var(vb->ub, ivar, nivar);
3127+
}
30553128
*btemp1->var = (jl_tvar_t *)nivar;
30563129
}
30573130
}
@@ -3067,11 +3140,13 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
30673140
}
30683141
if (root != vb) icount--;
30693142
if (root->innervars != NULL) {
3070-
size_t len = jl_array_nrows(root->innervars);
3143+
jl_array_t *rinnervars = root->innervars;
3144+
JL_GC_PROMISE_ROOTED(rinnervars);
3145+
size_t len = jl_array_nrows(rinnervars);
30713146
if (icount > len)
3072-
jl_array_grow_end(root->innervars, icount - len);
3147+
jl_array_grow_end(rinnervars, icount - len);
30733148
if (icount < len)
3074-
jl_array_del_end(root->innervars, len - icount);
3149+
jl_array_del_end(rinnervars, len - icount);
30753150
}
30763151
else if (icount > 0) {
30773152
root->innervars = jl_alloc_array_1d(jl_array_any_type, icount);
@@ -3104,6 +3179,9 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
31043179
res = jl_bottom_type;
31053180
}
31063181
else {
3182+
// re-fresh newvar if bounds changed.
3183+
if (vb->lb != newvar->lb || vb->ub != newvar->ub)
3184+
newvar = jl_new_typevar(newvar->name, vb->lb, vb->ub);
31073185
if (newvar != vb->var)
31083186
res = jl_substitute_var(res, vb->var, (jl_value_t*)newvar);
31093187
varval = (jl_value_t*)newvar;

‎test/subtype.jl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2650,3 +2650,10 @@ let S = Tuple{Val, Val{T}} where {T}, R = Tuple{Val{Val{T}}, Val{T}} where {T},
26502650
@testintersect(Tuple{Val{A}, A} where {B, A<:Union{Val{B}, GenericMemory{B}}}, S{1}, R{1})
26512651
@testintersect(Tuple{Val{A}, A} where {B, A<:Union{Val{B}, GenericMemory{:not_atomic,Int,B}}}, S{1}, R{1})
26522652
end
2653+
2654+
#issue 54516
2655+
let S = Tuple{Val{<:T}, Union{Int,T}} where {T},
2656+
T = Tuple{Union{Int,T}, Val{<:T}} where {T}
2657+
@testintersect(S, T, !Union{})
2658+
@test !Base.has_free_typevars(typeintersect(S, T))
2659+
end

0 commit comments

Comments
 (0)