Skip to content

improve construction of <:Tuple types from iterators #53265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

improve construction of <:Tuple types from iterators #53265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions base/Base.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ include("ctypes.jl")
include("gcutils.jl")
include("generator.jl")
include("reflection.jl")
include("type_intersect_exact.jl")
include("options.jl")

# define invoke(f, T, args...; kwargs...), without kwargs wrapping
Expand Down Expand Up @@ -196,6 +197,7 @@ end

# core operations & types
include("promotion.jl")
include("tuple_from_iterator.jl")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really need to exist this early? The code would be a lot cleaner if it could be moved to after int (so you can use +)

include("tuple.jl")
include("expr.jl")
include("pair.jl")
Expand Down
52 changes: 4 additions & 48 deletions base/tuple.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,11 +362,6 @@ const Any32{N} = Tuple{Any,Any,Any,Any,Any,Any,Any,Any,
Any,Any,Any,Any,Any,Any,Any,Any,
Any,Any,Any,Any,Any,Any,Any,Any,
Vararg{Any,N}}
const All32{T,N} = Tuple{T,T,T,T,T,T,T,T,
T,T,T,T,T,T,T,T,
T,T,T,T,T,T,T,T,
T,T,T,T,T,T,T,T,
Vararg{T,N}}
function map(f, t::Any32)
n = length(t)
A = Vector{Any}(undef, n)
Expand Down Expand Up @@ -449,52 +444,13 @@ end

(::Type{T})(x::Tuple) where {T<:Tuple} = x isa T ? x : convert(T, x) # still use `convert` for tuples

Tuple(x::Ref) = tuple(getindex(x)) # faster than iterator for one element
Tuple(x::Array{T,0}) where {T} = tuple(getindex(x))

(::Type{T})(itr) where {T<:Tuple} = _totuple(T, itr)

_totuple(::Type{Tuple{}}, itr, s...) = ()

function _totuple_err(@nospecialize T)
@noinline
throw(ArgumentError("too few elements for tuple type $T"))
end

function _totuple(::Type{T}, itr, s::Vararg{Any,N}) where {T,N}
@inline
y = iterate(itr, s...)
y === nothing && _totuple_err(T)
T1 = fieldtype(T, 1)
y1 = y[1]
t1 = y1 isa T1 ? y1 : convert(T1, y1)::T1
# inference may give up in recursive calls, so annotate here to force accurate return type to be propagated
rT = tuple_type_tail(T)
ts = _totuple(rT, itr, y[2])::rT
return (t1, ts...)::T
end

# use iterative algorithm for long tuples
function _totuple(T::Type{All32{E,N}}, itr) where {E,N}
len = N+32
elts = collect(E, Iterators.take(itr,len))
if length(elts) != len
_totuple_err(T)
function (::Type{T})(iter::S) where {T<:Tuple,S}
if IteratorSize(S) == IsInfinite()
throw(ArgumentError("can't construct a tuple from an infinite iterator"))
end
(elts...,)
TupleFromIterator.iterator_to_tuple_with_element_types(T, iter)::T
end

_totuple(::Type{Tuple{Vararg{E}}}, itr, s...) where {E} = (collect(E, Iterators.rest(itr,s...))...,)

_totuple(::Type{Tuple}, itr, s...) = (collect(Iterators.rest(itr,s...))...,)

# for types that `apply` knows about, just splatting is faster than collecting first
_totuple(::Type{Tuple}, itr::Array) = (itr...,)
_totuple(::Type{Tuple}, itr::SimpleVector) = (itr...,)
_totuple(::Type{Tuple}, itr::NamedTuple) = (itr...,)
_totuple(::Type{Tuple}, p::Pair) = (p.first, p.second)
_totuple(::Type{Tuple}, x::Number) = (x,) # to make Tuple(x) inferable

end

## find ##
Expand Down
169 changes: 169 additions & 0 deletions base/tuple_from_iterator.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

module TupleFromIterator

import ..TypeIntersectExact as TIE

incremented(n::Int) = Core.Intrinsics.add_int(n, 1)
decremented(n::Int) = Core.Intrinsics.sub_int(n, 1)

function _totuple_err(@nospecialize T)
@noinline
throw(ArgumentError("too few or too many elements for tuple type $T"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this use a LazyString?

end

function iterator_to_ntuple_recur(::Type{Tuple{}}, iter, state::Union{Tuple{},Tuple{Any}})
i = iterate(iter, state...)::Union{Nothing,Tuple{Any,Any}}
isnothing(i) ||
throw(ArgumentError("iterator has too many elements for the tuple type"))
()
end
function iterator_to_ntuple_recur(
::Type{Tuple{E,Vararg{E,lenm1}}}, iter, state::Union{Tuple{},Tuple{Any}}
) where {E,lenm1}
T2 = Tuple{E,Any}
ItUn = Union{Nothing,T2}
i = iterate(iter, state...)::ItUn
T = Tuple{E,Vararg{E,lenm1}}
isnothing(i) &&
throw(ArgumentError("iterator has too few elements for the tuple type"))
(e, s) = i::T2
Next = NTuple{lenm1,E}
t = iterator_to_ntuple_recur(Next, iter, (s,))::Next
(e, t...)::T
end

function iterator_to_ntuple(::Type{T}, iter) where {len,T<:NTuple{len,Any}}
if len < 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100 seems pretty high for this. Does the recursive version really infer that well?

iterator_to_ntuple_recur(T, iter, ())::T
else
# prevent stack overflow during type inference
let
f(i) = (collect(i)...,)
f(i::Tuple) = i
f(i::Union{NamedTuple,Core.SimpleVector,Array,Pair}) = (i...,)
f(i::Array{<:Any,0}) = (first(i),)
f(i::Union{AbstractArray{<:Any,0},Number,Ref}) = (first(i),)
f(i::Pair) = (first(i), last(i))
f(iter)::T
end
end::T
end

function iterator_to_tuple(::Type{R}, iter) where {R<:Tuple}
f(i) = collect(i)
f(i::Union{Tuple,NamedTuple,Core.SimpleVector,Array,AbstractArray{<:Any,0},Number,Ref,Pair}) = i

c = f(iter)
len = length(c)::Int
E = eltype(c)::Type
T = NTuple{len,E}
r = iterator_to_ntuple(T, c)::Tuple{Vararg{E}}::T
(r isa R) || _totuple_err(R)
r::R
end

"""
ntuple_any(::Type{<:NTuple{len,Any}}) where {len}

Like `ntuple(Returns(Any), Val(len))`.
"""
ntuple_any(::Type{Tuple{}}) = ()
function ntuple_any(::Type{<:Tuple{Any,Vararg{Any,lenm1}}}) where {lenm1}
T = NTuple{lenm1,DataType}
t = ntuple_any(T)::T
(Any, t...)::Tuple{DataType,Vararg{DataType,lenm1}}
end

"""
tuple_va_type_length(::Val)

Creates a `Vararg` `<:Tuple` type of specified minimal length.
"""
function tuple_va_type_length(::Val{len}) where {len}
Tuple{ntuple_any(NTuple{len,DataType})...,Vararg}
end
tuple_va_type_length(n::Int) = tuple_va_type_length(Val(n))::Type{<:Tuple}

tuple_length_type_va(::Type{T}, ::Val{n}, ::Type{S}) where {T<:Tuple,n,S<:Tuple} = Val(decremented(n))
function tuple_length_type_va(::Type{T}, ::Val{n}, ::Type{S}) where {n,S<:Tuple,T<:S}
v = Val(incremented(n))
tuple_length_type_va(T, v, tuple_va_type_length(v)::Type{<:Tuple})::Val
end

"""
tuple_length_type(::Type{<:Tuple})

Strips the element type information from a tuple type while keeping
the information about the length.
"""
function tuple_length_type(::Type{T}) where {T<:Tuple}
v = Val(1)
r = tuple_length_type_va(T, v, tuple_va_type_length(v)::Type{<:Tuple})::Val
tuple_va_type_length(r)::Type{<:Tuple}
end
tuple_length_type(::Type{T}) where {len,T<:NTuple{len,Any}} = NTuple{len,Any}

"""
fieldtype_typeintersect_ntuple(::Type{T}, ::Type{<:NTuple{len,Any}}, ::Val{ind}) where {T<:Tuple, len, ind}

Returns the type of the field `ind` in the type intersection of `T` with
`NTuple{len,Any}`. Failing to compute the exact intersection, the field `ind` of
`T` is returned.
"""
function fieldtype_typeintersect_ntuple(
(@nospecialize T::Type{<:Tuple}), ::Type{<:NTuple{len,Any}}, ::Val{ind}
) where {len,ind}
Core.@_foldable_meta
(1 ≤ (ind::Int) ≤ len) || throw(ArgumentError("`ind` out of bounds"))
S = NTuple{len,Any}
ST = typeintersect(S, T)::Type
TS = typeintersect(T, S)::Type
X = fieldtype(T, ind)::Type
Y = fieldtype(ST, ind)::Type
Z = fieldtype(TS, ind)::Type
type_intrs = TIE.type_intersect_exact(X, Y, Z)
TIE.get_result(X, type_intrs)::Type
end

function tuple_converted_elem(::Type{T}, t::NTuple{len,Any}, ::Val{ind}) where {T<:Tuple,len,ind}
(1 ≤ (ind::Int) ≤ len) || throw(ArgumentError("`ind` out of bounds"))
S = fieldtype_typeintersect_ntuple(T, NTuple{len,Any}, Val(ind))::Type
e = t[ind]
((e isa S) ? e : convert(S, e))::S
end

function tuple_with_converted_elems_recur(::Type{T}, ::NTuple{len,Any}, r::NTuple{len,Any}) where {T<:Tuple,len}
r::T
end
function tuple_with_converted_elems_recur(::Type{T}, t::NTuple{len,Any}, r::NTuple{n,Any}) where {T<:Tuple,len,n}
(n < len) || throw(ArgumentError("`n` out of bounds"))
m = incremented(n)
s = (r..., tuple_converted_elem(T, t, Val(m)))::NTuple{m,Any}
tuple_with_converted_elems_recur(T, t, s)::NTuple{len,Any}
end

function tuple_with_converted_elems(::Type{T}, t::NTuple{len,Any}) where {T<:Tuple,len}
NT = NTuple{len,Any}
type_intrs = TIE.type_intersect_exact(NT, T)
S = TIE.get_result(T, type_intrs)::Type{<:Tuple}
(S <: Union{}) && _totuple_err(T)
tuple_with_converted_elems_recur(S, t, ())::T::NT::S
end

function iterator_to_tuple_with_element_types(::Type{T}, iter) where {T<:Tuple}
R = tuple_length_type(T)::Type{<:Tuple}
t = iterator_to_tuple(R, iter)::R
tuple_with_converted_elems(T, t)::T
end

# As an optimization, special case some tuple types with no constraints on the
# types of the elements.
iterator_to_tuple_with_element_types(T::Type{NTuple{len,Any}}, i) where {len} = iterator_to_tuple(T, i)::T
iterator_to_tuple_with_element_types(T::Type{tuple_va_type_length(0)}, i) = iterator_to_tuple(T, i)::T
iterator_to_tuple_with_element_types(T::Type{tuple_va_type_length(1)}, i) = iterator_to_tuple(T, i)::T
iterator_to_tuple_with_element_types(T::Type{tuple_va_type_length(2)}, i) = iterator_to_tuple(T, i)::T
iterator_to_tuple_with_element_types(T::Type{tuple_va_type_length(3)}, i) = iterator_to_tuple(T, i)::T
iterator_to_tuple_with_element_types(T::Type{tuple_va_type_length(4)}, i) = iterator_to_tuple(T, i)::T

end
98 changes: 98 additions & 0 deletions base/type_intersect_exact.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

module TypeIntersectExact

struct OK end

"""
When `OK <: ok`, `R` is the exact type intersection. When `ok <: Union{}`, the
exact type intersection wasn't found and `R` has no significance.
"""
struct Result{R, ok<:OK} end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that this is implementing an Option type seems pretty sketchy to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it sketchy? It just seems like an elegant encoding to me?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to let the caller know if the exact intersection couldn't be found.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is, the idea is to enable the implementation of get_result.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's relatively sensible, the weird parts are that

  1. the OK value is a type? Why not a bool?
  2. Implimenting a custom option type specifically for this functionality seems somewhat weird. Why is an option type inherently embedded in the implementation of a typeintersect wrapper?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may be elegant, but something like that should be a separate PR, and not live under TypeIntersectExact namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about this again, OK isn't necessary at all, and thus Result is also unnecessary: I think it would be better to just return a Type when the search was successful and nothing when the search failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adienes what do you mean by "not live under TypeIntersectExact namespace"? Do you object to the name, or to the addition of a namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggestion for a better name?


result(::Type{T}) where {T} = Result{T, OK }()
failure() = Result{nothing, Union{}}()

"""
get_result(::Type, ::Result)::Type

Returns the exact type intersection if it was found, otherwise returns the
fallback.
"""
function get_result end

get_result(::Type{Fallback}, ::Result{<:Any, Union{}}) where {Fallback} = Fallback
get_result(::Type{Fallback}, ::Result{R, OK }) where {Fallback, R} = R::Type{R}

"""
type_intersect_exact(types...)::Result

Finds an exact type intersection or reports failure.
"""
function type_intersect_exact end

type_intersect_exact() = result(Any)::Result

function type_intersect_exact(@nospecialize A::Type)
Core.@_foldable_meta
result(A)::Result
end

function type_intersect_exact((@nospecialize A::Type), (@nospecialize B::Type))
Core.@_foldable_meta
if A <: B
result(A)
elseif B <: A
result(B)
else
let AB = typeintersect(A, B), BA = typeintersect(B, A)
if (AB <: A) && (AB <: B)
result(AB)
elseif (BA <: A) && (BA <: B)
result(BA)
else
failure()
end
end
end::Result
end

function type_intersect_exact(
(@nospecialize A::Type), (@nospecialize B::Type), (@nospecialize C::Type)
)
Core.@_foldable_meta # the loop below doesn't infer as terminating
candidates = let
AB = typeintersect(A, B)
BA = typeintersect(B, A)
AC = typeintersect(A, C)
CA = typeintersect(C, A)
BC = typeintersect(B, C)
CB = typeintersect(C, B)

AB_C = typeintersect(AB, C)
C_AB = typeintersect(C, AB)
BA_C = typeintersect(BA, C)
C_BA = typeintersect(C, BA)
AC_B = typeintersect(AC, B)
B_AC = typeintersect(B, AC)
CA_B = typeintersect(CA, B)
B_CA = typeintersect(B, CA)
BC_A = typeintersect(BC, A)
A_BC = typeintersect(A, BC)
CB_A = typeintersect(CB, A)
A_CB = typeintersect(A, CB)
Comment on lines +65 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you simplify this to something like

type_intersect_exact(C, type_intersect_exact(A,B))
type_intersect_exact(B, type_intersect_exact(A,C))
type_intersect_exact(A, type_intersect_exact(B,C))


(
A, B, C,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure A, B, C do not need to be considered because if one of them was valid, it would be successfully found as the intersection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're saying that typeintersect(A, B) can be relied on to return the "smaller" type of the two when they're ordered? Maybe you're right, but that's not documented, and I didn't read the C implementation, so I don't know if we can rely on that. If we can rely on that behavior of typeintersect, perhaps we should document it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're saying that typeintersect(A, B) can be relied on to return the "smaller" type of the two when they're ordered?

yes. If it didn't, it would be doing a really bad job of returning an intersection.

AB, BA, AC, CA, BC, CB,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.

AB_C, C_AB, BA_C, C_BA, AC_B, B_AC, CA_B, B_CA, BC_A, A_BC, CB_A, A_CB
)
end
for T ∈ candidates
is_exact = (T <: A) && (T <: B) && (T <: C)
is_exact && (return result(T)::Result)
end
failure()::Result
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a breaking change. We cannot merge the type_intersect_exact.jl file into Base in any form, as it promises something that it cannot provide

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not at all sure what you mean? It's just an internal helper function, given that it's not a change it can't be a breaking change??

Do you just don't like the name? The documentation is clear about the search for the exact intersection not always succeeding, but even so it's safe given that the result says whether the intersection was found or not.

end

end
1 change: 0 additions & 1 deletion test/ambiguous.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ end
end
let need_to_handle_undef_sparam =
Set{Method}(detect_unbound_args(Base; recursive=true, allowed_undefineds))
pop!(need_to_handle_undef_sparam, which(Base._totuple, (Type{Tuple{Vararg{E}}} where E, Any, Any)))
pop!(need_to_handle_undef_sparam, which(Base.eltype, Tuple{Type{Tuple{Any}}}))
pop!(need_to_handle_undef_sparam, first(methods(Base.same_names)))
@test_broken need_to_handle_undef_sparam == Set()
Expand Down
1 change: 0 additions & 1 deletion test/arrayops.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2102,7 +2102,6 @@ end
@test CartesianIndex{3}(1,2,3)*2 == CartesianIndex{3}(2,4,6)
@test_throws ErrorException iterate(CartesianIndex{3}(1,2,3))
@test CartesianIndices(CartesianIndex{3}(1,2,3)) == CartesianIndices((1, 2, 3))
@test Tuple{}(CartesianIndices{0,Tuple{}}(())) == ()

R = CartesianIndices(map(Base.IdentityUnitRange, (2:5, 3:5)))
@test eltype(R) <: CartesianIndex{2}
Expand Down
2 changes: 0 additions & 2 deletions test/core.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6883,8 +6883,6 @@ let a = Foo17149()
end

# issue #21004
const PTuple_21004{N,T} = NTuple{N,VecElement{T}}
@test_throws ArgumentError("too few elements for tuple type $PTuple_21004") PTuple_21004(1)
@test_throws UndefVarError(:T, :static_parameter) PTuple_21004_2{N,T} = NTuple{N, VecElement{T}}(1)

#issue #22792
Expand Down
Loading