Stopgap for Vararg widening

[carnaval]

This is a not-very-good fix for the following problem. It turns out that
since Vararg is a type (which I would argue is a bad idea), it can get
widened into a <:Vararg type var. This is not handled correctly and
can cause inference to crash. This patch does not fix the fact that we
could potentially encouter something like Tuple{<:Any} which could
come from Tuple{Vararg{T}}.

A small repro :

abstract A{T}
@noinline f(x,y) = ()
function hh(x :: A{A{A{A{A{Int}}}}}) # enough for type_too_complex
    y :: Tuple{Vararg{typeof(x)}} = (x,) # apply_type(Vararg, too_complex) => TypeVar(_,Vararg)
    f(y[1], # fool getfield logic : Tuple{_<:Vararg}[1] => Vararg
      1) # make it crash by construction of the signature Tuple{Vararg,Int}
end
code_typed(hh,(Any,))

I would argue that Vararg as a type has to go away, since being handled as a special case everywhere it breaks some obvious rules like :

Vararg{Int} <: Vararg{Integer} # false
Tuple{Vararg{Int}} <: Tuple{Vararg{Integer}} # true

My vote is for Vararg to be a syntactic-only construct that can never be materialized outside of a tuple type. It would be a property of the tuple type alone.

cc @JeffBezanson

[JeffBezanson]

Well, I completely agree this is not a very good fix :)

It would be a property of the tuple type alone.

That is funny, because in #10380 I initially implemented exactly that, then later undid it. It turned out to be a big pain. Instead of looking for Vararg, you check a flag. Either way you have to remember to do it. I found it easier to have a type completely described by its .parameters field.

The big question is how this relates to #11242. Vararg can naturally be extended to Vararg{T,N}.

I agree Vararg itself is not a type. We could make it a non-Type object, but I don't think that will help much since it just adds more special cases everywhere. We could add some more checks, for example disallowing it in TypeVar bounds, and disallowing passing it directly to <:.

If you want to change this, we need ideas for what the syntax and internal representation should be for Vararg{T,N}.

I think we should just fix all the cases that handle Vararg wrong, such as the widening case you mention.

[timholy]

80b0032 might achieve the same thing?

[carnaval]

Well, I completely agree this is not a very good fix :)

yeah consider this PR a bug report but since I had to dirty fix it to find out what was wrong with the code that tripped up inference...

I'll believe you that it is too much of a pain to have info outside parameters (there must be at least one downside to have Tuple::DataType after all). I think the core of the problem is what is Tuple{<:Any}. To be correct we'd have to consider this a "maybe vararg" thing. It could be useful but having vararg be a parameter means that you "pay" for this flexibility. It allows you to write things like type A{T}; t::Tuple{T}; end and then A{Vararg{Int}}, does anyone uses this kind of construct ?

If we on the other hand we want to have Vararg be more "special", i.e., whenever inference instantiate a Tuple type, it knows whether it's a Vararg or not, then you have to forbid it from being a type parameter I think, and pretty much anywhere we could "hide" it.

I'm glad to see that your PR fixed this issue already @timholy. I think there may be a few other places where it's possible to hide a Vararg or a Type{Vararg} that we should be careful with. E.g. this segfaults on master (didn't try your branch) :

type B{T}; x::T; end
function f(x::B,z)
    y :: Tuple{x.x} = z
    y[2]
end
b = B{Type}(Vararg{Int})
@show code_typed(f, (typeof(b),Tuple{Int,Int}))
f(b,(1,2))

(inference only sees Tuple{::Type{T}} and does not consider the T=Vararg{S} case)

I'll try to think of other problems.

[JeffBezanson]

Yes, cases like that are not intended to be allowed. We should disallow more things, like substituting Vararg for a type variable.

The big problem is the metaprogramming case where you have x = Vararg{T}; ... ; Tuple{x}. Given Tuple{x::Type}, we should indeed infer Type{<:Tuple{Vararg{Any}}}. Excellent point.

I definitely see why making this part of the tuple type feels safer. But it means that the programmer is forced to write out, with a branch, the analysis type inference would ideally do:

if vararg
    Tuple{T, ...}  # special syntax
else
    Tuple{T}
end

[carnaval]

I think we could allow it in TypeVar bounds, and it could be "useful", as long as the getfield function is careful. If for example we implement one day the old tmerge TODO we could have :

abstract A{T}
tmerge(A{Tuple{Vararg{Int}}}, A{Tuple{Vararg{Float}}}) === A{Tuple{<:Vararg{<:Real}}}

Anyway, I would argue that most uses of isvatuple should be using getfield_tfunc instead. In fact, isvatuple is correct if one considers it an abstract predicate where false means Top. It probably should be named isknownlength or something, to be consistent with issubtype. So that !isknownlength does not allow you to go picking manually at the last argument without checking that it is actually a Vararg{T}.

[JeffBezanson]

No, I don't think we want it in TypeVar bounds. The covariance of Tuples makes that unnecessary; the type in your example could be written as A{<:Tuple{Vararg{Real}}}.

[carnaval]

Right. So the only question is whether Tuple{<:Any} can be vararg or not. If not we check it when we construct the tuple type and widen it to Tuple{Vararg{Any}}, so that Tuple{<:Any} can always safely be transformed to Tuple{Any} (or <:Tuple{Any} in invariant context). This way we keep things mostly as they are now.

The other possibility is to have the checks be done on getfield and be conservative if tt.parameters[end] ∩ Vararg != bottom.

[carnaval]

I should think more before posting, forget about the second possibility it would be obviously a huge mess for any tuple with an Any type at the end. We'll just have to be careful when constructing tuple types then.

[vtjnash]

i think this is also related to #10930

[vtjnash]

possibly related to #16530 also?

[StefanKarpinski]

This PR probably can't be applied anymore, but this is still a potential bug.

[JeffBezanson]

Yes, there are still some bugs here. What I'd do is

1. Disallow Vararg in a typevar bound, and disallow substituting it as the value of a typevar.
2. Make sure our t-functions and widenings handle cases like

function f()
    x = Type[Vararg{Int}][1]
    Tuple{x}
end

Currently we think this always returns some subtype of Tuple{Any}, which it doesn't.