reduce usage of _pure_meta #32427
reduce usage of _pure_meta #32427
Conversation
| @@ -14,6 +14,11 @@ true | |||
| """ | |||
| NTuple | |||
|
|
|||
| # convenience function for extracting N from a Tuple (if defined) | |||
| # else return `nothing` for anything else given (such as Vararg or other non-sized Union) | |||
| _counttuple(::Type{<:NTuple{N,Any}}) where {N} = N | |||
There was a problem hiding this comment.
Should you have a @nospecialize on the argument in this case? I'm still not 💯% clear on it, but since we don't need to generate any more specializations/method-instances other than the two provided, should these be marked @nospecialize? (Or is that done automatically somehow given their tiny function bodies?)
vtjnash
force-pushed
the
jn/Tuple-pure
branch
from
36c0b0a
to
aa28710
Compare
|
Fails CI due to #32392 (type system bug) |
vtjnash
force-pushed
the
jn/Tuple-pure
branch
2 times, most recently
from
5836d4f
to
d4981b4
Compare
Realized I was probably just being foolish, and rewrote this test to be hopefully more sensible (typeintersect using top instead of subtype using bottom). Should be good for review now. |
| _counttuple(::Type{<:NTuple{N,Any}}) where {N} = N | ||
| _counttuple(::Type) = nothing |
There was a problem hiding this comment.
Per the discussion here:
#32495 (comment)
Can we name this something not private and export it? This is something we use all the time in our code at RelationalAI, where we do a lot of stuff with tuples. It also seems like something that other code will use a lot, like DataFrames, CSV, etc.
In that other comment I suggested arity(::Type{<:Tuple}) as one suggestion. I still like that best, but open to others!
arity()tupletypewidth()/tuple_type_width()tupletypecount()/tuple_type_count()tuplearity()
Or maybe we can break out a separate issue to track this, so that it doesn't hold up this PR?
There was a problem hiding this comment.
Yeah, I made this slightly internal just so it wouldn't delay this PR. Makes sense to me to track this as a separate issue to consider exporting it!
There was a problem hiding this comment.
Thanks. Opened #36820.
Resolved.
also fix the concrete_min answer for `Union{}` (which is a valid result
for the NTuple TypeVar to receive)
Avoid specifying subtype constraints on concrete types also, as this makes them more complicated, but no different.
| end | ||
|
|
||
| tuple_type_cons(::Type, ::Type{Union{}}) = Union{} | ||
| function tuple_type_cons(::Type{S}, ::Type{T}) where T<:Tuple where S |
There was a problem hiding this comment.
Some important packages seem to use this; we should add it back in base/deprecated.jl.
There was a problem hiding this comment.
Ah, tuple_type_head as well.
There was a problem hiding this comment.
But not tuple_type_tail?
There was a problem hiding this comment.
Ah, right, that one was still in use by _totuple
In preparation for increasing the optimizations that are enabled by this flag (#32368), let's try to reduce the number of places we are using it.
The new single convert method here even lets us do things like
convert(Tuple{Vararg{AbstractFloat}}, (2,)), which we couldn't previously express through dispatch.