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

type system revision and new subtype algorithm #18457

Merged
merged 18 commits into from
Jan 16, 2017
Merged

type system revision and new subtype algorithm #18457

merged 18 commits into from
Jan 16, 2017

Conversation

JeffBezanson
Copy link
Member

@JeffBezanson JeffBezanson commented Sep 12, 2016

This implements the plan in #8974. It is now working well enough to build a system image and get a REPL, albeit with inference disabled. I have some demos.

In the internals there are "only" three changes:

  • Types are explicitly quantified by wrapping in UnionAll types, which have two fields: a TypeVar and a type body:
julia> Array
Array{T,N} where N where T
  • Union types now have two fields, pointing to two types to union together (instead of an svec of types). This is just a representation change.
  • There is a new algorithm for computing the subtype relation. It gives drastically more meaningful results, and will replace the code for subtype and intersection in jltypes.c with a significant net code deletion. Working examples:
julia> Tuple{Union{Int,String}} <: Union{Tuple{Int},Tuple{String}}
true

julia> NTuple <: Tuple, Tuple <: NTuple
(true,false)

The second one is issue #18450.

UnionAll types nest arbitrarily, and the signatures of methods with static parameters are UnionAll types, so we now have "triangular" dispatch:

julia> f{T<:Real, A<:AbstractArray{T}}(x::T, y::Vector{A}) = 0
f (generic function with 1 method)

julia> f(1.0f0, [rand(4,4)])
ERROR: MethodError: no method matching f(::Float32, 
 in eval(::Module, ::Any) at ./boot.jl:235SYSTEM: show(lasterr) caused an error

julia> f(1.0f0, [rand(Float32, 4,4)])
0

(Clearly showing the method error isn't working, but that should just be a matter of updating the showing and reflection code in Base.)

The next steps are to get inference working and then fully replace intersection and morespecific with implementations based on the new algorithm.

Edit: Fixes #19159 #18892 #18450 #18348 #17943 #16922 #13165 #12596 #11803 #8915 #8625 #6721 #2552

@tkelman tkelman added the types and dispatch Types, subtyping and method dispatch label Sep 12, 2016
@tkelman tkelman added this to the 0.6.0 milestone Sep 12, 2016
return jl_types_equal_generic(a, b, useenv);
}
return jl_subtype(a, b, 0) && jl_subtype(b, a, 0);
return jl_types_equal(a, b);
Copy link
Member

Choose a reason for hiding this comment

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

❤️
(can now get rid of this function, those comments, and rip out useenv entirely :)

@TotalVerb
Copy link
Contributor

TotalVerb commented Sep 12, 2016

Theoretically, any tuple is NTuple{N, Any} for some value of N, so why shouldn't Tuple <: NTuple?

Nevertheless, this is incredible. 👍

@StefanKarpinski
Copy link
Member

I'm a bit confused by Tuple{Union{Int,String}} <: Union{Tuple{Int},Tuple{String}} being true. Aren't instances of the LHS any tuples of Ints and Strings while instances of the RHS are tuples of Ints or tuples of Strings but not mixes of both? Shouldn't the inclusion be the other way around?

@johnmyleswhite
Copy link
Member

I thought Tuple{Union{Int,String}} means either Tuple{Int} or Tuple{String} since there's exactly one element.

@StefanKarpinski
Copy link
Member

Ah, yes. I was forgetting how the new Tuple type syntax works. Thanks, @johnmyleswhite.

@StefanKarpinski
Copy link
Member

(I was thinking of Tuple as a hypothetical TTuple type that's like NTuple but with N and T in reverse order.)

@tbreloff
Copy link

so we now have "triangular" dispatch

Oh snap! Flashback to my giddy excitement during your JuliaCon 2015 talk :)

@JeffBezanson
Copy link
Member Author

@TotalVerb Good point. !(Tuple <: NTuple) is caused by the "diagonal rule". For example a method f{T}(x::T...) only matches arguments that are all of the same concrete type, so it is not the same as f(x::Any...). The type system models this by saying that certain type vars only range over concrete types.

From there, we have two choices:

  1. Disallow NTuple{N, Any}. Since the second parameter can only range over concrete types, substituting Any is not valid.
  2. Allow it, meaning the property U{T} <: U does not hold.

We currently pick (2). While U{T} <: U intuitively seems like it should be true, in practice I don't believe we actually need this property for anything.

@TotalVerb
Copy link
Contributor

Ah. So the current rule for function dispatch, that typevars in covariant position only range over concrete types, will be applied to type calculations too? It is certainly nice that this will be made more consistent than it is presently:

julia> test{T}(::Vararg{T,2}) = T
test (generic function with 1 method)

julia> test(1,1.0)
ERROR: MethodError: no method matching test(::Int64, ::Float64)
Closest candidates are:
  test{T}(::T...) at REPL[128]:1

julia> typeof(test).name.mt.defs.sig
Tuple{#test,Vararg{T,2}}

julia> isa((test,1,1.0), ans)
true

!(U{T} <: U) is certainly unintuitive, but I also don't think it's a huge loss. This is already the case with Union, and as far as I can tell will only be the case with things typealiased to Tuple (as no other type is covariant).

@JeffBezanson
Copy link
Member Author

will be applied to type calculations too?

Yes. That certain typevars range over concrete types is now just a structural property of types, to be followed by all type operations.

unionlen(x::Union) = unionlen(x.a) + unionlen(x.b)
unionlen(x::ANY) = 1

_uniontypes(x::Union, ts) = (_uniontypes(x.a,ts); _uniontypes(x.b,ts); ts)
Copy link
Member

Choose a reason for hiding this comment

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

flip argument order?

Copy link
Member Author

Choose a reason for hiding this comment

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

The underscore means you're not allowed to complain about it :)

Copy link
Member

Choose a reason for hiding this comment

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

OK 😄

@tkelman
Copy link
Contributor

tkelman commented Jan 16, 2017

There's some performance we need to claw back, looks like this added about 10 minutes to the test time on appveyor. Probably comparable on Travis.

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.