Skip to content

Commit

Permalink
fix another case where we might return free TypeVar
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@vtjnash
vtjnash committed Dec 12, 2022
1 parent 770f5a3 commit d33a849
Show file tree
Hide file tree
Showing 3 changed files with 181 additions and 51 deletions.
65 changes: 47 additions & 18 deletions src/subtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/compiler/inference.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
165 changes: 133 additions & 32 deletions test/subtype.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1181,51 +1181,121 @@ 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
@testintersect(a, b, Val{Tuple{Val{T},Val{T}}} where T)
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
@test typeintersect(a, b) <: b
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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1865,10 +1938,31 @@ 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_broken !Base.has_free_typevars(I)
J = (Union{Tuple{Type{T}, Ref{T}, Ref{T}} where T<:Ref,
Tuple{Type{T}, Ref{T}, T1} where {T<:Ref, T1<:Ref{T}},
Tuple{Type{T}, Ref{T}, Ref} where {T<:Union{Ref, Ref{Tx}}}} where Tx).body # uh oh, Tx is not good
@test I == _type_intersect(A, B) == J
J = Union{Tuple{Type{T}, Ref{T}, Ref{T}} where T<:Ref,
Tuple{Type{T}, Ref{T}, T} where {T1, T<:Union{Ref{T1}, Ref}}}
@test_broken I == J

I = typeintersect(A, B)
@test_broken !Base.has_free_typevars(I)
J = (Tuple{Union{Type{T}, Type{T2}, Type{T1}},
Union{Ref{T}, Ref{T2}, Ref{T1}},
Union{Ref{T}, Ref, T2}} where
{T<:Ref, T1<:Ref, T2<:Ref{T1}, T1<:Union{Ref, Ref{Tx}}}
where Tx).body
@test I == typeintersect(A, B) == J

end

# issue #39218
Expand Down Expand Up @@ -1900,9 +1994,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
Expand Down

0 comments on commit d33a849

Please sign in to comment.