Skip to content
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.

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

tuple type introspection via indexing/iteration #22687

Closed
wants to merge 2 commits into from

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jul 5, 2017

This PR simplifies introspection of tuple types by allowing you to iterate/index tuple types almost as if they were simply a tuple of types. For example, it allows you to do:

julia> T = Tuple{Int8,String,Float32}
Tuple{Int8,String,Float32}

julia> length(T)
3

julia> T[2]
String

julia> collect(Type, T)
3-element Array{Type,1}:
 Int8
 String
 Float32

Without this PR, you have to do a lot of digging through Julia internals to do introspection of tuple types (via the undocumented T.types field), and it is especially difficult to introspect vararg tuple types (via undocumented functions like Base.unwrapva).

I encountered this when generalizing tuple conversion for PyCall (JuliaPy/PyCall.jl#404), and the difficulty of tuple introspection has also caused some fragility in JLD (#10999).

@stevengj stevengj added the types and dispatch Types, subtyping and method dispatch label Jul 5, 2017
@quinnj
Copy link
Member

quinnj commented Jul 5, 2017

+1, I've definitely wanted this lately.

@stevengj
Copy link
Member Author

stevengj commented Jul 5, 2017

See also #22350.

# typeof(Tuple) is simply DataType
_iteratorsize(::Type{T}) where {T<:Tuple} = (@_pure_meta; isvatuple(T) ? IsInfinite() : HasLength())
_length(::HasLength, T::Type{<:Tuple}) = length(T.types)
_length(::IsInfinite, T::Type{<:Tuple}) = typemax(Int)
Copy link
Contributor

Choose a reason for hiding this comment

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

little odd for something that's supposed to be infinite to return a finite length?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should it throw an error instead? (I could just delete this line and it would throw a MethodError in that case.)

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Yes, length usually gives a MethodError on infinite iterators.

@stevengj
Copy link
Member Author

stevengj commented Jul 5, 2017

It also might be useful to export a new function like isvararg(T::Tuple) that returns true for Vararg tuple types.

(Basically, this just means cleaning up and exporting Base.isvatuple.)

@stevengj
Copy link
Member Author

stevengj commented Jul 5, 2017

One problem here is that Union{Tuple{Any,Int},Tuple{Any}} <: Tuple (see also tonyhffong/Lint.jl#215), so the current version of this PR will do the wrong thing on unions of tuples.

Ideally, I'd like to be able to introspect the components of unions in the same way, so adding some additional methods to this PR would do the trick. However, I'm confused by the behavior of dispatch here:

julia> foo(::Type{<:Union}) = 1
       foo(::Type{<:Tuple}) = 2
       foo(::Type{Union{<:Tuple}}) = 3

julia> foo(Union{Tuple{Int},Tuple{Any}})
2

I would have expected 1 or 3, not 2 ... how can you dispatch on a union of tuples?

@mbauman
Copy link
Sponsor Member

mbauman commented Jul 5, 2017

Ref #11547.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 5, 2017

In #22051, I observed that while accessing the fields of the tuple datatype is generally not valid (because of the possibility of Union), and thus also not possible to infer, the fieldtype function may be a solution. It should be dealing with those other possibilities correctly anyways, so it already largely must have the right semantics.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 5, 2017

foo(::Type{Union{<:Tuple}})

This is just Type{Tuple} and matches precisely Tuple

foo(::Type{<:Union}) = 1

This is just Type{Union} and matches precisely Union

foo(::Union) = 1 might work, but dispatch to it may not be reliable (I can't find the right issue number just now), and I'm surprised at this sort order, since kinds usually are expected to be sorted after Types:

julia> methods(foo)
# 6 methods for generic function "foo":
[1] foo(::Core.TypeofBottom) in Main at REPL[9]:1
[2] foo(::Union) in Main at REPL[2]:1
[3] foo(::Type{Union{}}) in Main at REPL[8]:1
[4] foo(::Type{#s1} where #s1<:Tuple) in Main at REPL[6]:1
[5] foo(::DataType) in Main at REPL[1]:1
[6] foo(::UnionAll) in Main at REPL[4]:1

@stevengj
Copy link
Member Author

stevengj commented Jul 5, 2017

@vtjnash, thanks for the tip about fieldtype. It seems to work, including the vararg case.
e.g. fieldtype(Tuple{Int, Vararg{String}}, i) returns Int for i==1 and String for i≥1.

In order to use this effectively, however, it would be good to export and document some version of Base.isvatuple, however, and have a documented way to get the number of fields other than length(fieldnames(T)).

@JeffBezanson
Copy link
Sponsor Member

Yes, in this case it's important to distinguish looking at the structure of a type (which is generally painful and difficult to get right) from "type-domain indexing", which is what getfield_tfunc computes:

julia> Core.Inference.getfield_tfunc(Union{Tuple{Int},Tuple{String}}, Int)
Union{Int64, String}

That's telling you what you'd get from accessing an instance of the given type.

@martinholters
Copy link
Member

While a better introspection API sounds useful, I'm not sure I like the getindex approach here:

julia> Int[1]
1-element Array{Int64,1}:
 1

julia> Tuple{Int}[(1,)]
1-element Array{Tuple{Int64},1}:
 (1,)

julia> Tuple{Int}[1] # surprise!
Int64

Putting this behind fieldtype has less potential for confusion, I'd say. If iteration is needed, maybe a new functionfieldtypes could yield an iterable (and indexable) wrapper?

@andyferris
Copy link
Member

Anything that makes working with these easier is very welcome to me. :)

Would a tfunc be necessary to make getindex return types inferred, or does this work already?

@stevengj
Copy link
Member Author

stevengj commented Jul 6, 2017

I'm not so concerned about iteration (I can always loop over indices), but I am concerned with knowing the length of the tuple, whether it is a vararg tuple, and documenting this introspection technique; having multiple packages looking at undocumented internals is broken.

It would also be nice to make sure these things were known at compile time, but this is an optimization that could be added later if needed.

I will close this PR since the consensus seems to be against indexing (and I wasn't even aware of #11547). Will try again with another PR based on fieldtype.

@bramtayl
Copy link
Contributor

bramtayl commented Sep 2, 2017

This works nicely:

@generated function inner_types(atype)
    Expr(:tuple, map(1:nfields(atype)) do i
        :(fieldtype($atype, $i))
    end...)
end

@vtjnash
Copy link
Sponsor Member

vtjnash commented Sep 4, 2017

Or more succinctly, inner_types(atype::Type) = ntuple(i -> fieldtype(atype, i), Val(nfields(atype))) (provided, of course, that you are only calling this on a leaftype, and not something else like a Vararg Tuple)

@bramtayl
Copy link
Contributor

bramtayl commented Sep 4, 2017

Cool. Is there a way to get fieldtype to inline?

@bramtayl
Copy link
Contributor

bramtayl commented Sep 4, 2017

Boiled down the type instability to this:

atype = Tuple{Int64, String}
    
test(f, a) = f(a, 1), f(a, 2)
test2(f, a) = begin
    f2(i) = f(a, i)
    f2(1), f2(2)
end

Test.@inferred test(fieldtype, atype)
Test.@inferred test2(fieldtype, atype) # fails

@TotalVerb
Copy link
Contributor

@martinholters I'd argue that it's really the T[x, y] syntax that's problematic there (as it's a pun for getindex), but I guess that's too late to change...

@bramtayl
Copy link
Contributor

bramtayl commented Sep 5, 2017

This should be fully inferable. Dunno why though.

inner_value(::Val{T}) where T = T
function inner_types(atype)
    const_type = Val{atype}()
    Base.@pure inner_function(i) = fieldtype(inner_value(const_type), i)
    ntuple(inner_function, Val{nfields(atype)}())
end

@bramtayl
Copy link
Contributor

bramtayl commented Sep 5, 2017

Safer:

inner_value(::Val{T}) where T = T
function inner_types(val_type)
    Base.@pure inner_function(i) = Val{fieldtype(inner_value(val_type), i)}()
    ntuple(inner_function, Val{nfields(inner_value(val_type))}())
end
inner_types(Val{Tuple{Int64, String}}())

I'm gathering that if you are going to use types in inference, they should be passed around wrapped in Vals, and unpacked at point of use with inner_value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants