From 4c7aeabc282fd264229825dd6e9af75e1c3b11a2 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 | 138 +++++++++++++++++++++++++++++-------- 3 files changed, 157 insertions(+), 48 deletions(-) diff --git a/src/subtype.c b/src/subtype.c index 1d9d3d875675d..b90b278af85f8 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -2507,9 +2507,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(); @@ -2525,20 +2524,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); @@ -2557,13 +2548,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 @@ -2586,17 +2594,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 1a6f49182975c..7d12eff826e75 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 70f3dd864cdbe..4fdef5b3204cc 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) @@ -1181,11 +1181,25 @@ ftwoparams(::TwoParams{<:Real,<:Real}) = 3 # a bunch of cases found by fuzzing let a = Tuple{Float64,T7} where T7, b = Tuple{S5,Tuple{S5}} where S5 - @test typeintersect(a, b) <: b + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test I1 <: b + @test I2 <: b end let a = Tuple{T1,T1} where T1, b = Tuple{Val{S2},S6} where S2 where S6 - @test typeintersect(a, b) == typeintersect(b, a) + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test I1 <: b + @test I2 <: b end let a = Val{Tuple{T1,T1}} where T1, b = Val{Tuple{Val{S2},S6}} where S2 where S6 @@ -1193,15 +1207,36 @@ let a = Val{Tuple{T1,T1}} where T1, end let a = Tuple{Float64,T3,T4} where T4 where T3, b = Tuple{S2,Tuple{S3},S3} where S2 where S3 - @test typeintersect(a, b) == typeintersect(b, a) + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test_broken I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test_broken I1 <: b + @test I2 <: b end let a = Tuple{T1,Tuple{T1}} where T1, b = Tuple{Float64,S3} where S3 - @test typeintersect(a, b) <: a + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test I1 <: b + @test I2 <: b end let a = Tuple{5,T4,T5} where T4 where T5, b = Tuple{S2,S3,Tuple{S3}} where S2 where S3 - @test typeintersect(a, b) == typeintersect(b, a) + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test_broken I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test_broken I1 <: 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 @@ -1209,23 +1244,58 @@ let a = Tuple{T2,Tuple{T4,T2}} where T4 where T2, end let a = Tuple{Tuple{T2,4},T6} where T2 where T6, b = Tuple{Tuple{S2,S3},Tuple{S2}} where S2 where S3 - @test typeintersect(a, b) == typeintersect(b, a) + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test_broken I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test_broken I1 <: b + @test I2 <: b end let a = Tuple{T3,Int64,Tuple{T3}} where T3, b = Tuple{S3,S3,S4} where S4 where S3 - @test_broken typeintersect(a, b) <: a + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test_broken I1 <: a + @test I2 <: a + @test I1 <: b + @test I2 <: b end let a = Tuple{T1,Val{T2},T2} where T2 where T1, b = Tuple{Float64,S1,S2} where S2 where S1 - @test typeintersect(a, b) == typeintersect(b, a) + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test_broken I1 <: a + @test_broken I2 <: a + @test I1 <: b + @test I2 <: b end let a = Tuple{T1,Val{T2},T2} where T2 where T1, b = Tuple{Float64,S1,S2} where S2 where S1 - @test_broken typeintersect(a, b) <: a + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test_broken I1 <: a + @test_broken I2 <: a + @test I1 <: b + @test I2 <: b end let a = Tuple{Float64,T1} where T1, b = Tuple{S1,Tuple{S1}} where S1 - @test typeintersect(a, b) <: b + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test I1 <: b + @test I2 <: b end let a = Tuple{Val{T1},T2,T2} where T2 where T1, b = Tuple{Val{Tuple{S2}},S3,Float64} where S2 where S3 @@ -1234,12 +1304,20 @@ end let a = Tuple{T1,T2,T2} where T1 where T2, b = Tuple{Val{S2},S2,Float64} where S2, x = Tuple{Val{Float64},Float64,Float64} - @test x <: typeintersect(a, b) -end -let a = Val{Tuple{T1,Val{T2},Val{Int64},Tuple{Tuple{T3,5,Float64},T4,T2,T5}}} where T1 where T5 where T4 where T3 where T2, - b = Val{Tuple{Tuple{S1,5,Float64},Val{S2},S3,Tuple{Tuple{Val{Float64},5,Float64},2,Float64,S4}}} where S2 where S3 where S1 where S4 - @test_skip typeintersect(b, a) -end + I1 = typeintersect(a, b) + I2 = typeintersect(b, a) + @test x <: I1 + @test x <: I2 + @test I1 <: I2 + @test I2 <: I1 + @test I1 <: a + @test I2 <: a + @test_broken I1 <: b + @test_broken I2 <: b +end +@testintersect(Val{Tuple{T1,Val{T2},Val{Int64},Tuple{Tuple{T3,5,Float64},T4,T2,T5}}} where T1 where T5 where T4 where T3 where T2, + Val{Tuple{Tuple{S1,5,Float64},Val{S2},S3,Tuple{Tuple{Val{Float64},5,Float64},2,Float64,S4}}} where S2 where S3 where S1 where S4, + Val{Tuple{Tuple{S1, 5, Float64}, Val{Float64}, Val{Int64}, Tuple{Tuple{Val{Float64}, 5, Float64}, 2, Float64, T5}}} where {T5, S1}) # issue #20992 abstract type A20992{T,D,d} end @@ -1803,15 +1881,10 @@ 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 = typeintersect(X1, X2) - # TODO: the quality of this intersection is not great; for now just test that it - # doesn't stack overflow - @test I<:X1 || I<:X2 - actual = Tuple{Union{AlmostLU{S, X} where X<:Matrix{S}, AlmostLU{S, <:Matrix}}, Vector{S}} where S<:Union{Float32, Float64} - @test I == actual -end +@testintersect(Tuple{AlmostLU, Vector{T}} where T, + Tuple{AlmostLU{S, X} where X<:Matrix, Vector{S}} where S<:Union{Float32, Float64}, + Tuple{AlmostLU{S, X} where X<:Matrix{S}, Vector{S}} where S<:Union{Float32, Float64}) +# actually returns Tuple{Union{AlmostLU{S, X} where X<:Matrix{S}, AlmostLU{S, X} where X<:Matrix{S}}, Vector{S}} where S<:Union{Float32, Float64} let # issue #22787 @@ -1900,9 +1973,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