From 99856797056926309e8c38923ff12d492c249f63 Mon Sep 17 00:00:00 2001 From: N5N3 <2642243996@qq.com> Date: Wed, 22 May 2024 19:41:56 +0800 Subject: [PATCH] typeintersect: fix incorrect innervar handling under circular env --- src/subtype.c | 115 +++++++++++++++++++++++++++++++++++++++--------- test/subtype.jl | 7 +++ 2 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/subtype.c b/src/subtype.c index 8e361f44914df..d459bd789fa19 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -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) { @@ -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; @@ -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); @@ -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; @@ -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; @@ -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 subtitution. + 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; } } @@ -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; diff --git a/test/subtype.jl b/test/subtype.jl index c65521d44ac5a..c26f4fc9d30e2 100644 --- a/test/subtype.jl +++ b/test/subtype.jl @@ -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