fix some more Vararg issues

- fix #11480, don't use `Type{_} where _<:Vararg` in inference - disallow `Vararg` in type var bounds - disallow `Vararg` as a type parameter in general - move some checks from lower-level functions to apply_type
JuliaLang · Jan 24, 2017 · f252ca5 · f252ca5
1 parent c7c1062
commit f252ca5
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 53 deletions.
diff --git a/base/inference.jl b/base/inference.jl
@@ -860,6 +860,9 @@ function apply_type_tfunc(headtypetype::ANY, args::ANY...)
         uncertain = true
     end
     !uncertain && return Const(appl)
+    if isvarargtype(headtype)
+        return Type
+    end
     if type_too_complex(appl,0)
         return Type{_} where _<:headtype
     end
@@ -1686,7 +1689,16 @@ function ⊑(a::ANY, b::ANY)
     end
 end
 
-widenconst(c::Const) = isa(c.val, Type) ? Type{c.val} : typeof(c.val)
+function widenconst(c::Const)
+    if isa(c.val, Type)
+        if isvarargtype(c.val)
+            return Type
+        end
+        return Type{c.val}
+    else
+        return typeof(c.val)
+    end
+end
 widenconst(t::ANY) = t
 
 issubstate(a::VarState, b::VarState) = (a.typ ⊑ b.typ && a.undef <= b.undef)

diff --git a/src/builtins.c b/src/builtins.c
@@ -1012,13 +1012,63 @@ JL_DLLEXPORT void jl_show(jl_value_t *stream, jl_value_t *v)
 
 // internal functions ---------------------------------------------------------
 
+static int valid_type_param(jl_value_t *v)
+{
+    if (jl_is_tuple(v)) {
+        // NOTE: tuples of symbols are not currently bits types, but have been
+        // allowed as type parameters. this is a bit ugly.
+        jl_value_t *tt = jl_typeof(v);
+        size_t i, l = jl_nparams(tt);
+        for(i=0; i < l; i++) {
+            jl_value_t *pi = jl_tparam(tt,i);
+            if (!(pi == (jl_value_t*)jl_sym_type || jl_isbits(pi)))
+                return 0;
+        }
+        return 1;
+    }
+    if (jl_is_vararg_type(v))
+        return 0;
+    // TODO: maybe more things
+    return jl_is_type(v) || jl_is_typevar(v) || jl_is_symbol(v) || jl_isbits(jl_typeof(v));
+}
+
 JL_CALLABLE(jl_f_apply_type)
 {
     JL_NARGSV(apply_type, 1);
-    if (!jl_is_unionall(args[0]) && args[0] != (jl_value_t*)jl_anytuple_type &&
-        args[0] != (jl_value_t*)jl_uniontype_type)
-        jl_type_error("Type{...} expression", (jl_value_t*)jl_unionall_type, args[0]);
-    return jl_apply_type(args[0], &args[1], nargs-1);
+    int i;
+    if (args[0] == (jl_value_t*)jl_anytuple_type) {
+        for(i=1; i < nargs; i++) {
+            jl_value_t *pi = args[i];
+            // TODO: should possibly only allow Types and TypeVars, but see
+            // https://github.com/JuliaLang/julia/commit/85f45974a581ab9af955bac600b90d9ab00f093b#commitcomment-13041922
+            if (jl_is_vararg_type(pi)) {
+                if (i != nargs-1)
+                    jl_type_error_rt("Tuple", "non-final parameter", (jl_value_t*)jl_type_type, pi);
+            }
+            else if (!valid_type_param(pi)) {
+                jl_type_error_rt("Tuple", "parameter", (jl_value_t*)jl_type_type, pi);
+            }
+        }
+        return (jl_value_t*)jl_apply_tuple_type_v(&args[1], nargs-1);
+    }
+    else if (args[0] == (jl_value_t*)jl_uniontype_type) {
+        // Union{} has extra restrictions, so it needs to be checked after
+        // substituting typevars (a valid_type_param check here isn't sufficient).
+        return (jl_value_t*)jl_type_union(&args[1], nargs-1);
+    }
+    else if (jl_is_unionall(args[0])) {
+        for(i=1; i < nargs; i++) {
+            jl_value_t *pi = args[i];
+            if (!valid_type_param(pi)) {
+                jl_type_error_rt("Type", "parameter",
+                                 jl_isa(pi, (jl_value_t*)jl_number_type) ?
+                                 (jl_value_t*)jl_long_type : (jl_value_t*)jl_type_type,
+                                 pi);
+            }
+        }
+        return jl_apply_type(args[0], &args[1], nargs-1);
+    }
+    jl_type_error("Type{...} expression", (jl_value_t*)jl_unionall_type, args[0]);
 }
 
 // generic function reflection ------------------------------------------------

diff --git a/src/jltypes.c b/src/jltypes.c
@@ -375,9 +375,9 @@ JL_DLLEXPORT jl_value_t *jl_type_union(jl_value_t **ts, size_t n)
 
 JL_DLLEXPORT jl_tvar_t *jl_new_typevar(jl_sym_t *name, jl_value_t *lb, jl_value_t *ub)
 {
-    if (lb != jl_bottom_type && !jl_is_type(lb) && !jl_is_typevar(lb))
+    if ((lb != jl_bottom_type && !jl_is_type(lb) && !jl_is_typevar(lb)) || jl_is_vararg_type(lb))
         jl_type_error_rt("TypeVar", "lower bound", (jl_value_t*)jl_type_type, lb);
-    if (ub != (jl_value_t*)jl_any_type && !jl_is_type(ub) && !jl_is_typevar(ub))
+    if ((ub != (jl_value_t*)jl_any_type && !jl_is_type(ub) && !jl_is_typevar(ub)) || jl_is_vararg_type(ub))
         jl_type_error_rt("TypeVar", "upper bound", (jl_value_t*)jl_type_type, ub);
     jl_ptls_t ptls = jl_get_ptls_states();
     jl_tvar_t *tv = (jl_tvar_t*)jl_gc_alloc(ptls, sizeof(jl_tvar_t), jl_tvar_type);
@@ -658,25 +658,6 @@ jl_value_t *jl_cache_type_(jl_datatype_t *type)
 
 // type instantiation
 
-static int valid_type_param(jl_value_t *v)
-{
-    if (jl_is_tuple(v)) {
-        // NOTE: tuples of symbols are not currently bits types, but have been
-        // allowed as type parameters. this is a bit ugly.
-        jl_value_t *tt = jl_typeof(v);
-        size_t i;
-        size_t l = jl_nparams(tt);
-        for(i=0; i < l; i++) {
-            jl_value_t *pi = jl_tparam(tt,i);
-            if (!(pi == (jl_value_t*)jl_sym_type || jl_isbits(pi)))
-                return 0;
-        }
-        return 1;
-    }
-    // TODO: maybe more things
-    return jl_is_type(v) || jl_is_typevar(v) || jl_is_symbol(v) || jl_isbits(jl_typeof(v));
-}
-
 static int within_typevar(jl_value_t *t, jl_value_t *vlb, jl_value_t *vub)
 {
     jl_value_t *lb = t, *ub = t;
@@ -705,14 +686,6 @@ jl_value_t *jl_apply_type(jl_value_t *tc, jl_value_t **params, size_t n)
         if (!jl_is_unionall(tc))
             jl_error("too many parameters for type");
         jl_value_t *pi = params[i];
-
-        if (!valid_type_param(pi)) {
-            jl_type_error_rt("Type", "parameter",
-                             jl_isa(pi, (jl_value_t*)jl_number_type) ?
-                             (jl_value_t*)jl_long_type : (jl_value_t*)jl_type_type,
-                             pi);
-        }
-
         jl_unionall_t *ua = (jl_unionall_t*)tc;
         if (!jl_has_free_typevars(ua->var->lb) && !jl_has_free_typevars(ua->var->ub) &&
             !within_typevar(pi, ua->var->lb, ua->var->ub)) {
@@ -1139,23 +1112,11 @@ static jl_value_t *inst_datatype(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **i
     return (jl_value_t*)ndt;
 }
 
-static void check_tuple_parameter(jl_value_t *pi, size_t i, size_t np)
-{
-    // TODO: should possibly only allow Types and TypeVars, but see
-    // https://github.com/JuliaLang/julia/commit/85f45974a581ab9af955bac600b90d9ab00f093b#commitcomment-13041922
-    if (!valid_type_param(pi))
-        jl_type_error_rt("Tuple", "parameter", (jl_value_t*)jl_type_type, pi);
-    if (i != np-1 && jl_is_vararg_type(pi))
-        jl_type_error_rt("Tuple", "non-final parameter", (jl_value_t*)jl_type_type, pi);
-}
-
 static jl_tupletype_t *jl_apply_tuple_type_v_(jl_value_t **p, size_t np, jl_svec_t *params)
 {
     int cacheable = 1;
     for(size_t i=0; i < np; i++) {
-        jl_value_t *pi = p[i];
-        check_tuple_parameter(pi, i, np);
-        if (!jl_is_leaf_type(pi))
+        if (!jl_is_leaf_type(p[i]))
             cacheable = 0;
     }
     jl_datatype_t *ndt = (jl_datatype_t*)inst_datatype(jl_anytuple_type, params, p, np,
@@ -1235,20 +1196,17 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
         iparams = jl_svec_data(ip_heap);
     }
     int cacheable = 1;
-    if (jl_is_va_tuple(tt)) {
+    if (jl_is_va_tuple(tt))
         cacheable = 0;
-    }
     int i;
     for(i=0; i < ntp; i++) {
         jl_value_t *elt = jl_svecref(tp, i);
         jl_value_t *pi = (jl_value_t*)inst_type_w_(elt, env, stack, 0);
         iparams[i] = pi;
         if (ip_heap)
             jl_gc_wb(ip_heap, pi);
-        check_tuple_parameter(pi, i, ntp);
-        if (cacheable && !jl_is_leaf_type(pi)) {
+        if (cacheable && !jl_is_leaf_type(pi))
             cacheable = 0;
-        }
     }
     jl_value_t *result = inst_datatype((jl_datatype_t*)tt, ip_heap, iparams, ntp, cacheable,
                                        stack, env);

diff --git a/test/core.jl b/test/core.jl
@@ -15,6 +15,10 @@ f47{T}(x::Vector{Vector{T}}) = 0
 @test_throws TypeError ([T] where T)
 @test_throws TypeError (Array{T} where T<:[])
 @test_throws TypeError (Array{T} where T>:[])
+@test_throws TypeError (Array{T} where T<:Vararg)
+@test_throws TypeError (Array{T} where T>:Vararg)
+@test_throws TypeError (Array{T} where T<:Vararg{Int})
+@test_throws TypeError (Array{T} where T<:Vararg{Int,2})
 
 # issue #8652
 args_morespecific(a, b) = ccall(:jl_type_morespecific, Cint, (Any,Any), a, b) != 0
@@ -3086,10 +3090,12 @@ end
 
 # don't allow Vararg{} in Union{} type constructor
 @test_throws TypeError Union{Int,Vararg{Int}}
+@test_throws TypeError Union{Vararg{Int}}
 
-# don't allow Vararg{} in Tuple{} type constructor in non-trailing position
+# only allow Vararg{} in last position of Tuple{ }
 @test_throws TypeError Tuple{Vararg{Int32},Int64,Float64}
 @test_throws TypeError Tuple{Int64,Vararg{Int32},Float64}
+@test_throws TypeError Array{Vararg}
 
 # don't allow non-types in Union
 @test_throws TypeError Union{1}

diff --git a/test/inference.jl b/test/inference.jl
@@ -478,6 +478,17 @@ function maybe_vararg_tuple_2()
 end
 @test Type{Tuple{Vararg{Int}}} <: Base.return_types(maybe_vararg_tuple_2, ())[1]
 
+# issue #11480
+@noinline f11480(x,y) = x
+let A = Ref
+    function h11480(x::A{A{A{A{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
+    @test !Base.isvarargtype(Base.return_types(h11480, (Any,))[1])
+end
+
 # Issue 19641
 foo19641() = let a = 1.0
     Core.Inference.return_type(x -> x + a, Tuple{Float64})