From 63a49de6cc020416e8fd14941408eaf346bf2b07 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 6 Dec 2022 09:27:19 -0500 Subject: [PATCH] fix another case where we might return free TypeVar We had an environment here that looked like while computing the upper bound for J{S} where S: where S=T where T where I{T} where J{S} where S Then we started handling those, and filling in the values: First replacing S with T, which creates a `res` of `J{T}` where T where I{T} where J{S} where S Then we handled T, which is also going to set `wrap=0`, so our result for `J{T}` will not be made into `J{T} where T`. where I{T} (wrap 0) where J{S} where S Here we then had finished handling all the dependencies for J{S} where S, which resulted in an upper bound assignment of J{T} where I{T} where J{T} Next, we handle I{T}, though it is now unused, so while we will make `I{T} where T` (via innervars) here for it, this goes unuesd. And finally, we had our resulting clause: where J{T} But it is missing the `where T`, since `I` (from lhs) was discarded. Thus we need to add that back now, when handling some innervars, if we see our term got duplicated to a higher part of the bounds before reaching this handling for its placement movement. --- src/subtype.c | 65 +++++++++++++++++++++++++++----------- test/compiler/inference.jl | 2 +- test/subtype.jl | 61 ++++++++++++++++++++++++----------- 3 files changed, 91 insertions(+), 37 deletions(-) diff --git a/src/subtype.c b/src/subtype.c index 7af4e8b28b934..ddc032b2d59a7 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -2694,9 +2694,8 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind newvar = jl_new_typevar(vb->var->name, vb->lb, vb->ub); // remove/replace/rewrap free occurrences of this var in the environment - jl_varbinding_t *btemp = e->vars; - int wrap = 1; - while (btemp != NULL) { + jl_varbinding_t *wrap = NULL; + for (jl_varbinding_t *btemp = e->vars; btemp != NULL; btemp = btemp->prev) { if (jl_has_typevar(btemp->lb, vb->var)) { if (vb->lb == (jl_value_t*)btemp->var) { JL_GC_POP(); @@ -2712,20 +2711,12 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind } else if (btemp->lb == (jl_value_t*)vb->var) btemp->lb = vb->lb; - else if (btemp->depth0 == vb->depth0 && !jl_has_typevar(vb->lb, btemp->var) && - !jl_has_typevar(vb->ub, btemp->var) && jl_has_typevar(btemp->ub, vb->var)) { + else if (btemp->depth0 == vb->depth0 && !jl_has_typevar(vb->lb, btemp->var) && !jl_has_typevar(vb->ub, btemp->var)) { // 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 (btemp->innervars == NULL) - btemp->innervars = jl_alloc_array_1d(jl_array_any_type, 0); - if (newvar != vb->var) { + if (newvar != vb->var) btemp->lb = jl_substitute_var(btemp->lb, vb->var, (jl_value_t*)newvar); - btemp->ub = jl_substitute_var(btemp->ub, vb->var, (jl_value_t*)newvar); - } - jl_array_ptr_1d_push(btemp->innervars, (jl_value_t*)newvar); - wrap = 0; - btemp = btemp->prev; - continue; + wrap = btemp; } else btemp->lb = jl_new_struct(jl_unionall_type, vb->var, btemp->lb); @@ -2747,13 +2738,30 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind res = jl_bottom_type; } } - else if (btemp->ub == (jl_value_t*)vb->var) + else if (btemp->ub == (jl_value_t*)vb->var) { + // TODO: this loses some constraints, such as in this test, where we replace T4<:S3 (e.g. T4==S3 since T4 only appears covariantly once) with T4<:Any + // a = Tuple{Float64,T3,T4} where T4 where T3 + // b = Tuple{S2,Tuple{S3},S3} where S2 where S3 + // Tuple{Float64, T3, T4} where {S3, T3<:Tuple{S3}, T4<:S3} btemp->ub = vb->ub; + } + else if (btemp->depth0 == vb->depth0 && !jl_has_typevar(vb->lb, btemp->var) && !jl_has_typevar(vb->ub, btemp->var)) { + if (newvar != vb->var) + btemp->ub = jl_substitute_var(btemp->ub, vb->var, (jl_value_t*)newvar); + wrap = btemp; + } else btemp->ub = jl_new_struct(jl_unionall_type, vb->var, btemp->ub); assert((jl_value_t*)btemp->var != btemp->ub); } - btemp = btemp->prev; + } + + if (wrap) { + // 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 (wrap->innervars == NULL) + wrap->innervars = jl_alloc_array_1d(jl_array_any_type, 0); + jl_array_ptr_1d_push(wrap->innervars, (jl_value_t*)newvar); } // if `v` still occurs, re-wrap body in `UnionAll v` or eliminate the UnionAll @@ -2776,17 +2784,38 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind if (newvar != vb->var) res = jl_substitute_var(res, vb->var, (jl_value_t*)newvar); varval = (jl_value_t*)newvar; - if (wrap) + if (!wrap) res = jl_type_unionall((jl_tvar_t*)newvar, res); } } if (res != jl_bottom_type && vb->innervars != NULL) { int i; - for(i=0; i < jl_array_len(vb->innervars); i++) { + for (i = 0; i < jl_array_len(vb->innervars); i++) { jl_tvar_t *var = (jl_tvar_t*)jl_array_ptr_ref(vb->innervars, i); if (jl_has_typevar(res, var)) res = jl_type_unionall((jl_tvar_t*)var, res); + // TODO: full dominator analysis for when handling innervars + // 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 + // of the type walk that now may have incomplete bounds: finish those now too + jl_varbinding_t *btemp = e->vars; + while (btemp != NULL) { + //if (btemp->depth0 == vb->depth0 && (jl_has_typevar(btemp->lb, var) || jl_has_typevar(btemp->ub, var))) { + // if (!jl_has_typevar(vb->lb, var) && !jl_has_typevar(vb->ub, var)) { + // if (btemp->innervars == NULL) + // btemp->innervars = jl_alloc_array_1d(jl_array_any_type, 0); + // jl_array_ptr_1d_push(btemp->innervars, (jl_value_t*)var); + // } + //} + if (btemp->depth0 == vb->depth0) { + if (jl_has_typevar(btemp->lb, var)) + btemp->lb = jl_type_unionall((jl_tvar_t*)var, btemp->lb); + if (jl_has_typevar(btemp->ub, var)) + btemp->ub = jl_type_unionall((jl_tvar_t*)var, btemp->ub); + } + btemp = btemp->prev; + } } } diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index b7adcba297925..3a933f749e7fa 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -999,7 +999,7 @@ end # issue #21410 f21410(::V, ::Pair{V,E}) where {V, E} = E -@test code_typed(f21410, Tuple{Ref, Pair{Ref{T},Ref{T}} where T<:Number})[1].second == +@test only(Base.return_types(f21410, Tuple{Ref, Pair{Ref{T},Ref{T}} where T<:Number})) == Type{E} where E <: (Ref{T} where T<:Number) # issue #21369 diff --git a/test/subtype.jl b/test/subtype.jl index a182bb99909ee..e5329c91fd0da 100644 --- a/test/subtype.jl +++ b/test/subtype.jl @@ -904,11 +904,11 @@ function test_intersection() # both of these answers seem acceptable #@testintersect(Tuple{T,T} where T<:Union{UpperTriangular, UnitUpperTriangular}, # Tuple{AbstractArray{T,N}, AbstractArray{T,N}} where N where T, - # Union{Tuple{T,T} where T<:UpperTriangular, - # Tuple{T,T} where T<:UnitUpperTriangular}) + # Union{Tuple{T,T} where T<:UpperTriangular{T1}, + # Tuple{T,T} where T<:UnitUpperTriangular{T1}} where T) @testintersect(Tuple{T,T} where T<:Union{UpperTriangular, UnitUpperTriangular}, Tuple{AbstractArray{T,N}, AbstractArray{T,N}} where N where T, - Tuple{T,T} where T<:Union{UpperTriangular, UnitUpperTriangular}) + Tuple{T,T} where {T1, T<:Union{UpperTriangular{T1}, UnitUpperTriangular{T1}}}) @testintersect(DataType, Type, DataType) @testintersect(DataType, Type{T} where T<:Integer, Type{T} where T<:Integer) @@ -1209,12 +1209,12 @@ let a = Tuple{Float64,T3,T4} where T4 where T3, b = Tuple{S2,Tuple{S3},S3} where S2 where S3 I1 = typeintersect(a, b) I2 = typeintersect(b, a) - @test I1 <: I2 + @test_broken I1 <: I2 @test I2 <: I1 @test I1 <: a @test I2 <: a @test_broken I1 <: b - @test_broken I2 <: b + @test I2 <: b end let a = Tuple{T1,Tuple{T1}} where T1, b = Tuple{Float64,S3} where S3 @@ -1231,12 +1231,12 @@ let a = Tuple{5,T4,T5} where T4 where T5, b = Tuple{S2,S3,Tuple{S3}} where S2 where S3 I1 = typeintersect(a, b) I2 = typeintersect(b, a) - @test I1 <: I2 + @test_broken I1 <: I2 @test I2 <: I1 @test I1 <: a @test I2 <: a @test_broken I1 <: b - @test_broken I2 <: b + @test I2 <: b end let a = Tuple{T2,Tuple{T4,T2}} where T4 where T2, b = Tuple{Float64,Tuple{Tuple{S3},S3}} where S3 @@ -1246,12 +1246,12 @@ let a = Tuple{Tuple{T2,4},T6} where T2 where T6, b = Tuple{Tuple{S2,S3},Tuple{S2}} where S2 where S3 I1 = typeintersect(a, b) I2 = typeintersect(b, a) - @test I1 <: I2 + @test_broken I1 <: I2 @test I2 <: I1 @test I1 <: a @test I2 <: a @test_broken I1 <: b - @test_broken I2 <: b + @test I2 <: b end let a = Tuple{T3,Int64,Tuple{T3}} where T3, b = Tuple{S3,S3,S4} where S4 where S3 @@ -1881,11 +1881,9 @@ end # issue #38081 struct AlmostLU{T, S<:AbstractMatrix{T}} end -let X1 = Tuple{AlmostLU, Vector{T}} where T, - X2 = Tuple{AlmostLU{S, X} where X<:Matrix, Vector{S}} where S<:Union{Float32, Float64}, - I = Tuple{AlmostLU{T, S} where S<:Matrix{T}, Vector{T}} where T<:Union{Float32, Float64} - @testintersect(X1, X2, I) -end +@testintersect(Tuple{AlmostLU, Vector{T}} where T, + Tuple{AlmostLU{S, X} where X<:Matrix, Vector{S}} where S<:Union{Float32, Float64}, + Tuple{AlmostLU{T, X} where X<:Matrix{T}, Vector{T}} where T<:Union{Float32, Float64}) let # issue #22787 @@ -1939,10 +1937,23 @@ end # issue #34170 let A = Tuple{Type{T} where T<:Ref, Ref, Union{T, Union{Ref{T}, T}} where T<:Ref}, B = Tuple{Type{T}, Ref{T}, Union{Int, Ref{T}, T}} where T - I = typeintersect(A,B) # this was a case where <: disagreed with === (due to a badly-normalized type) - @test I == typeintersect(A,B) - @test I == Tuple{Type{T}, Ref{T}, Ref} where T<:Ref + I = _type_intersect(B, A) + @test I == _type_intersect(B, A) == Union{Tuple{Type{T}, Ref{T}, Ref{T}} where T<:Ref, Tuple{Type{T}, Ref{T}, T} where T<:Ref} + I = typeintersect(B, A) + @test I == typeintersect(B, A) == Tuple{Type{T}, Ref{T}, Union{Ref{T}, T}} where T<:Ref + + I = _type_intersect(A, B) + @test !Base.has_free_typevars(I) + J = Tuple{Type{T1}, Ref{T1}, Ref} where {T, T1<:Union{Ref, Ref{T}}} + @test I == _type_intersect(A, B) == J + @test_broken I == Tuple{Type{T}, Ref{T}, T1} where {T<:Ref, T1<:Union{T, Ref{T}}} # a better result, == to the result with arguments switched + + I = typeintersect(A, B) + @test !Base.has_free_typevars(I) + J = Tuple{Type{T1}, Ref{T1}, Ref} where {T, T1<:Union{Ref, Ref{T}}} + @test I == typeintersect(A, B) == J + end # issue #39218 @@ -1974,9 +1985,16 @@ end let T = Type{T} where T<:(AbstractArray{I}) where I<:(Base.IteratorsMD.CartesianIndex), S = Type{S} where S<:(Base.IteratorsMD.CartesianIndices{A, B} where B<:Tuple{Vararg{Any, A}} where A) I = typeintersect(T, S) + # This intersection returns B<:Tuple{Vararg{Any,N}}, while intersection with T + # should have added a constraint for it to be B<:Tuple{Vararg{OrdinalRange{Int64,Int64},N}} @test_broken I <: T @test I <: S - @test_broken I == typeintersect(S, T) + I2 = typeintersect(S, T) + @test_broken I2 <: T + @test I2 <: S + @test I == I2 + @test !Base.has_free_typevars(I) + @test !Base.has_free_typevars(I2) end # issue #39948 @@ -2412,3 +2430,10 @@ let A = Tuple{Type{T}, T} where T, C = Tuple{Type{MyType47877{W, V} where V<:Union{MyAbstract47877{W}, Base.BitInteger}}, MyType47877{W, V} where V<:Union{MyAbstract47877{W}, Base.BitInteger}} where W<:Base.BitInteger @test typeintersect(B, A) == C end + +let T = Tuple{Union{Type{T}, Type{S}}, Union{Val{T}, Val{S}}, Union{Val{T}, S}} where T<:Val{A} where A where S<:Val, + S = Tuple{Type{T}, T, Val{T}} where T<:(Val{S} where S<:Val) + # TODO: these should be Union[} + @test typeintersect(T, S) === Tuple{Type{T}, T, Val{T}} where T<:(Val{S} where S<:Val) + @test typeintersect(S, T) === Tuple{Union{Type{T}, Type{T1}}, Union{Val{T1}, Val{S1}, T}, Union{S, S1}} where {T<:(Val{S} where S<:Val), S<:Union{Val{T}, T}, T1<:(Val), S1<:Val{T1}} +end