Skip to content

Commit

Permalink
Work around type intersection bug in _isequal, _isless, __inc, …
Browse files Browse the repository at this point in the history
…and `__dec`

The underlying intersection bug (ref. #39088) looks like
```
julia> typeintersect(Tuple{Tuple{Any, Vararg{Any, N}}, Tuple{Any, Vararg{Any, N}}} where N, Tuple{Tuple{Int,Int,Int}, Tuple})
Tuple{Tuple{Int64, Int64, Int64}, NTuple{4, Any}}
```

It leads to the argument types of the functions with signatures of this
type to be inferred wrongly, not actually matching the signature, which
easily leads to problems down the road.

For the four methods touched here, the fact that the two tuple arguments
must have the same length is not actually relevant for dispatch as they
are internal functions which are only called if this actually holds.
Also, this change will just replace the method error with another error
if, for some reason, these fuctions would get called for tuples of
unequal length.
  • Loading branch information
martinholters committed Oct 19, 2021
1 parent 7b76c7e commit e1afeda
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
8 changes: 4 additions & 4 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,11 @@ module IteratorsMD

# comparison
@inline isless(I1::CartesianIndex{N}, I2::CartesianIndex{N}) where {N} = _isless(0, I1.I, I2.I)
@inline function _isless(ret, I1::Tuple{Int,Vararg{Int,N}}, I2::Tuple{Int,Vararg{Int,N}}) where {N}
@inline function _isless(ret, I1::Tuple{Int,Vararg{Int}}, I2::Tuple{Int,Vararg{Int}})
newret = ifelse(ret==0, icmp(last(I1), last(I2)), ret)
t1, t2 = Base.front(I1), Base.front(I2)
# avoid dynamic dispatch by telling the compiler relational invariants
return isa(t1, Tuple{}) ? _isless(newret, (), ()) : _isless(newret, t1, t2::Tuple{Int,Vararg{Int}})
return isa(t1, Tuple{}) ? _isless(newret, (), t2::Tuple{}) : _isless(newret, t1, t2::Tuple{Int,Vararg{Int}})
end
_isless(ret, ::Tuple{}, ::Tuple{}) = ifelse(ret==1, true, false)
icmp(a, b) = ifelse(isless(a,b), 1, ifelse(a==b, 0, -1))
Expand Down Expand Up @@ -433,7 +433,7 @@ module IteratorsMD
valid = __is_valid_range(I, rng) && state[1] != last(rng)
return valid, (I, )
end
@inline function __inc(state::Tuple{Int,Int,Vararg{Int,N}}, indices::Tuple{OrdinalRangeInt,OrdinalRangeInt,Vararg{OrdinalRangeInt,N}}) where {N}
@inline function __inc(state::Tuple{Int,Int,Vararg{Int}}, indices::Tuple{OrdinalRangeInt,OrdinalRangeInt,Vararg{OrdinalRangeInt}})
rng = indices[1]
I = state[1] + step(rng)
if __is_valid_range(I, rng) && state[1] != last(rng)
Expand Down Expand Up @@ -546,7 +546,7 @@ module IteratorsMD
valid = __is_valid_range(I, rng) && state[1] != first(rng)
return valid, (I,)
end
@inline function __dec(state::Tuple{Int,Int,Vararg{Int,N}}, indices::Tuple{OrdinalRangeInt,OrdinalRangeInt,Vararg{OrdinalRangeInt,N}}) where {N}
@inline function __dec(state::Tuple{Int,Int,Vararg{Int}}, indices::Tuple{OrdinalRangeInt,OrdinalRangeInt,Vararg{OrdinalRangeInt}})
rng = indices[1]
I = state[1] - step(rng)
if __is_valid_range(I, rng) && state[1] != first(rng)
Expand Down
4 changes: 2 additions & 2 deletions base/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -382,11 +382,11 @@ filter(f, t::Tuple) = length(t) < 32 ? filter_rec(f, t) : Tuple(filter(f, collec

isequal(t1::Tuple, t2::Tuple) = length(t1) == length(t2) && _isequal(t1, t2)
_isequal(::Tuple{}, ::Tuple{}) = true
function _isequal(t1::Tuple{Any,Vararg{Any,N}}, t2::Tuple{Any,Vararg{Any,N}}) where {N}
function _isequal(t1::Tuple{Any,Vararg{Any}}, t2::Tuple{Any,Vararg{Any}})
isequal(t1[1], t2[1]) || return false
t1, t2 = tail(t1), tail(t2)
# avoid dynamic dispatch by telling the compiler relational invariants
return isa(t1, Tuple{}) ? true : _isequal(t1, t2::Tuple{Any,Vararg{Any}})
return isa(t1, Tuple{}) ? _isequal((), t2::Tuple{}) : _isequal(t1, t2::Tuple{Any,Vararg{Any}})
end
function _isequal(t1::Any32, t2::Any32)
for i = 1:length(t1)
Expand Down
9 changes: 9 additions & 0 deletions test/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -671,3 +671,12 @@ end

# https://github.com/JuliaLang/julia/issues/40814
@test Base.return_types(NTuple{3,Int}, (Vector{Int},)) == Any[NTuple{3,Int}]

# issue #42457
f42457(a::NTuple{3,Int}, b::Tuple)::Bool = Base.isequal(a, Base.inferencebarrier(b)::Tuple)
@test f42457((1, 1, 1), (1, 1, 1))
@test !isempty(methods(Base._isequal, (NTuple{3, Int}, Tuple)))
g42457(a, b) = Base.isequal(a, b) ? 1 : 2.0
@test only(Base.return_types(g42457, (NTuple{3, Int}, Tuple))) === Union{Float64, Int}
@test only(Base.return_types(g42457, (NTuple{3, Int}, NTuple))) === Union{Float64, Int}
@test only(Base.return_types(g42457, (NTuple{3, Int}, NTuple{4}))) === Float64

0 comments on commit e1afeda

Please sign in to comment.