fix review comments and noise

JuliaLang · Aug 7, 2017 · ec169f1 · ec169f1
1 parent c82277a
commit ec169f1
Show file tree

Hide file tree

Showing 14 changed files with 154 additions and 150 deletions.
diff --git a/base/essentials.jl b/base/essentials.jl
@@ -171,12 +171,12 @@ convert(::Type{T}, x::Tuple{Any, Vararg{Any}}) where {T<:Tuple} =
 # TODO: the following definitions are equivalent (behaviorally) to the above method
 # I think they may be faster / more efficient for inference,
 # if we could enable them, but are they?
-# TODO: These currently can't be used (#21026) since with
-#     z(::Type{Tuple{Val{T}} where T}) = T
+# TODO: These currently can't be used (#21026, #23017) since with
+#     z(::Type{<:Tuple{Vararg{T}}}) where {T} = T
 #   calling
-#     z(Tuple{Val})
+#     z(Tuple{Val{T}} where T)
 #   fails, even though `Type{Tuple{Val}} == Type{Tuple{Val{S}} where S}`
-#   and so T should be Val{S} where S
+#   and so T should be `Val` (aka `Val{S} where S`)
 #convert(_::Type{Tuple{S}}, x::Tuple{S}) where {S} = x
 #convert(_::Type{Tuple{S}}, x::Tuple{Any}) where {S} = (convert(S, x[1]),)
 #convert(_::Type{T}, x::T) where {S, N, T<:Tuple{S, Vararg{S, N}}} = x

diff --git a/base/inference.jl b/base/inference.jl
@@ -1448,7 +1448,7 @@ function invoke_tfunc(@nospecialize(f), @nospecialize(types), @nospecialize(argt
         return Any
     end
     meth = entry.func
-    (ti, env) = ccall(:jl_env_from_type_intersection, Any, (Any, Any), argtype, meth.sig)::SimpleVector
+    (ti, env) = ccall(:jl_type_intersection_with_env, Any, (Any, Any), argtype, meth.sig)::SimpleVector
     rt, edge = typeinf_edge(meth::Method, ti, env, sv)
     edge !== nothing && add_backedge!(edge::MethodInstance, sv)
     return rt
@@ -1825,7 +1825,7 @@ function abstract_call_method(method::Method, @nospecialize(f), @nospecialize(si
 
     # if sig changed, may need to recompute the sparams environment
     if isa(method.sig, UnionAll) && isempty(sparams)
-        recomputed = ccall(:jl_match_method, Any, (Any, Any), sig, method.sig)::SimpleVector
+        recomputed = ccall(:jl_type_intersection_with_env, Any, (Any, Any), sig, method.sig)::SimpleVector
         sig = recomputed[1]
         if !isa(unwrap_unionall(sig), DataType) # probably Union{}
             return Any
@@ -4253,7 +4253,7 @@ function inlineable(@nospecialize(f), @nospecialize(ft), e::Expr, atypes::Vector
     else
         invoke_data = invoke_data::InvokeData
         method = invoke_data.entry.func
-        (metharg, methsp) = ccall(:jl_match_method, Any, (Any, Any),
+        (metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
                                   atype_unlimited, method.sig)::SimpleVector
         methsp = methsp::SimpleVector
     end

diff --git a/base/reflection.jl b/base/reflection.jl
@@ -737,7 +737,7 @@ function _dump_function(@nospecialize(f), @nospecialize(t), native::Bool, wrappe
     t = to_tuple_type(t)
     ft = isa(f, Type) ? Type{f} : typeof(f)
     tt = Tuple{ft, t.parameters...}
-    (ti, env) = ccall(:jl_match_method, Any, (Any, Any), tt, meth.sig)::SimpleVector
+    (ti, env) = ccall(:jl_type_intersection_with_env, Any, (Any, Any), tt, meth.sig)::SimpleVector
     meth = func_for_method_checked(meth, ti)
     linfo = ccall(:jl_specializations_get_linfo, Ref{Core.MethodInstance}, (Any, Any, Any, UInt), meth, ti, env, world)
     # get the code for it

diff --git a/base/test.jl b/base/test.jl
@@ -1258,7 +1258,7 @@ function detect_ambiguities(mods...;
 end
 
 """
-    detect_unbound_args(mod1, mod2...; imported=false)
+    detect_unbound_args(mod1, mod2...; imported=false, recursive=false)
 
 Returns a vector of `Method`s which may have unbound type parameters.
 Use `imported=true` if you wish to also test functions that were
@@ -1304,17 +1304,17 @@ end
 
 # find if var will be constrained to have a definite value
 # in any concrete leaftype subtype of typ
-function constrains_param(var::TypeVar, @nospecialize(typ), cov::Bool)
+function constrains_param(var::TypeVar, @nospecialize(typ), covariant::Bool)
     typ === var && return true
     while typ isa UnionAll
-        cov && constrains_param(var, typ.var.ub, cov) && return true
+        covariant && constrains_param(var, typ.var.ub, covariant) && return true
         # typ.var.lb doesn't constrain var
         typ = typ.body
     end
     if typ isa Union
         # for unions, verify that both options would constrain var
-        ba = constrains_param(var, typ.a, cov)
-        bb = constrains_param(var, typ.b, cov)
+        ba = constrains_param(var, typ.a, covariant)
+        bb = constrains_param(var, typ.b, covariant)
         (ba && bb) && return true
     elseif typ isa DataType
         # return true if any param constrains var
@@ -1324,16 +1324,16 @@ function constrains_param(var::TypeVar, @nospecialize(typ), cov::Bool)
                 # vararg tuple needs special handling
                 for i in 1:(fc - 1)
                     p = typ.parameters[i]
-                    constrains_param(var, p, cov) && return true
+                    constrains_param(var, p, covariant) && return true
                 end
                 lastp = typ.parameters[fc]
                 vararg = Base.unwrap_unionall(lastp)
                 if vararg isa DataType && vararg.name === Base._va_typename
                     N = vararg.parameters[2]
-                    constrains_param(var, N, cov) && return true
+                    constrains_param(var, N, covariant) && return true
                     # T = vararg.parameters[1] doesn't constrain var
                 else
-                    constrains_param(var, lastp, cov) && return true
+                    constrains_param(var, lastp, covariant) && return true
                 end
             else
                 for i in 1:fc

diff --git a/base/tuple.jl b/base/tuple.jl
@@ -63,9 +63,18 @@ first(t::Tuple) = t[1]
 # eltype
 
 eltype(::Type{Tuple{}}) = Bottom
-#eltype(::Type{Tuple{Vararg{E}}}) where {E} = E
-eltype(::Type{<:Tuple{Vararg{E}}}) where {E} = E
-function eltype(t::Type{<:Tuple})
+eltype(::Type{Tuple{Vararg{E}}}) where {E} = E
+function eltype(t::Type{<:Tuple{Vararg{E}}}) where {E}
+    if @isdefined(E)
+        return E
+    else
+        # TODO: need to guard against E being miscomputed by subtyping (ref #23017)
+        # and compute the result manually in this case
+        return _compute_eltype(t)
+    end
+end
+eltype(t::Type{<:Tuple}) = _compute_eltype(t)
+function _compute_eltype(t::Type{<:Tuple})
     @_pure_meta
     t isa Union && return typejoin(eltype(t.a), eltype(t.b))
     t´ = unwrap_unionall(t)
@@ -200,7 +209,8 @@ fill_to_length(t::Tuple{}, val, ::Val{2}) = (val, val)
 # constructing from an iterator
 
 # only define these in Base, to avoid overwriting the constructors
-if isdefined(Main, :Base)
+# NOTE: this means this constructor must be avoided in Inference!
+if module_name(@__MODULE__) === :Base
 
 (::Type{T})(x::Tuple) where {T<:Tuple} = convert(T, x)  # still use `convert` for tuples
 

diff --git a/src/gf.c b/src/gf.c
@@ -821,17 +821,17 @@ JL_DLLEXPORT int jl_is_cacheable_sig(
     return 1;
 }
 
-static jl_method_instance_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *cache, jl_value_t *parent,
-                                      jl_tupletype_t *type, // the specialized type signature for type lambda
-                                      jl_tupletype_t *tt, // the original tupletype of the signature
-                                      jl_typemap_entry_t *m,
-                                      size_t world,
-                                      jl_svec_t *sparams,
-                                      int allow_exec)
+static jl_method_instance_t *cache_method(
+        jl_methtable_t *mt, union jl_typemap_t *cache, jl_value_t *parent,
+        jl_tupletype_t *type, // the specialized type signature for type lambda
+        jl_tupletype_t *tt, // the original tupletype of the signature
+        jl_method_t *definition,
+        size_t world,
+        jl_svec_t *sparams,
+        int allow_exec)
 {
     // caller must hold the mt->writelock
-    jl_method_t *definition = m->func.method;
-    jl_value_t *decl = (jl_value_t*)m->sig;
+    jl_value_t *decl = (jl_value_t*)definition->sig;
     jl_value_t *temp = NULL;
     jl_value_t *temp2 = NULL;
     jl_value_t *temp3 = NULL;
@@ -891,7 +891,7 @@ static jl_method_instance_t *cache_method(jl_methtable_t *mt, union jl_typemap_t
             if (nsp > 0) {
                 jl_svec_t *env = jl_alloc_svec_uninit(2 * nsp);
                 temp2 = (jl_value_t*)env;
-                jl_unionall_t *ua = (jl_unionall_t*)m->sig;
+                jl_unionall_t *ua = (jl_unionall_t*)definition->sig;
                 for (j = 0; j < nsp; j++) {
                     assert(jl_is_unionall(ua));
                     jl_svecset(env, j * 2, ua->var);
@@ -1055,7 +1055,7 @@ static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype
                 nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
             }
             else {
-                nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, tt, entry, world, env, allow_exec);
+                nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, tt, m, world, env, allow_exec);
             }
         }
     }
@@ -1673,6 +1673,8 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types, size_t world
     if (/* TODO: !jl_is_cacheable_sig((jl_value_t*)types) &&*/ !jl_is_leaf_type((jl_value_t*)types))
         return NULL;
     if (jl_has_free_typevars((jl_value_t*)types))
+        return NULL; // don't poison the cache due to a malformed query
+    if (!jl_has_concrete_subtype((jl_value_t*)types))
         return NULL;
 
     // find if exactly 1 method matches (issue #7302)
@@ -1690,15 +1692,21 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types, size_t world
     jl_tupletype_t *ti = (jl_tupletype_t*)jl_unwrap_unionall(jl_svecref(match, 0));
     jl_method_instance_t *nf = NULL;
     if (ti == types && !jl_has_call_ambiguities(types, m)) {
+        jl_datatype_t *dt = jl_first_argument_datatype(jl_unwrap_unionall((jl_value_t*)types));
+        assert(jl_is_datatype(dt));
+        jl_methtable_t *mt = dt->name->mt;
         sig = join_tsig(ti, (jl_tupletype_t*)m->sig);
-        int need_guard_entries = 0;
-        int makesimplesig = 0;
-        jl_cacheable_sig(sig, ti, (jl_tupletype_t*)m->sig, m,
-                         (jl_svec_t**)&newparams, &need_guard_entries, &makesimplesig);
-        if (newparams)
-            sig = jl_apply_tuple_type(newparams);
-        nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
-        //cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, tt, entry, world, env, allow_exec);
+        //// get the specialization without caching it
+        //int need_guard_entries = 0;
+        //int makesimplesig = 0;
+        //jl_cacheable_sig(sig, ti, (jl_tupletype_t*)m->sig, m,
+        //                 (jl_svec_t**)&newparams, &need_guard_entries, &makesimplesig);
+        //if (newparams)
+        //    sig = jl_apply_tuple_type(newparams);
+        //nf = jl_specializations_get_linfo(m, (jl_value_t*)sig, env, world);
+        JL_LOCK(&mt->writelock);
+        nf = cache_method(mt, &mt->cache, (jl_value_t*)mt, sig, ti, m, world, env, /*allow_exec*/1);
+        JL_UNLOCK(&mt->writelock);
     }
     assert(nf == NULL || (nf->min_world <= world && nf->max_world >= world));
     JL_GC_POP();
@@ -1981,17 +1989,16 @@ jl_value_t *jl_gf_invoke(jl_tupletype_t *types0, jl_value_t **args, size_t nargs
         }
         else {
             tt = arg_type_tuple(args, nargs);
-            if (jl_is_unionall(entry->sig)) {
-                int sub = jl_subtype_matching((jl_value_t*)tt, (jl_value_t*)entry->sig, &tpenv);
+            if (jl_is_unionall(method->sig)) {
+                int sub = jl_subtype_matching((jl_value_t*)tt, (jl_value_t*)method->sig, &tpenv);
                 assert(sub); (void)sub;
             }
-            sig = join_tsig(tt, entry->sig);
-            jl_method_t *func = entry->func.method;
 
-            if (func->invokes.unknown == NULL)
-                func->invokes.unknown = jl_nothing;
+            if (method->invokes.unknown == NULL)
+                method->invokes.unknown = jl_nothing;
 
-            mfunc = cache_method(mt, &func->invokes, entry->func.value, sig, tt, entry, world, tpenv, 1);
+            sig = join_tsig(tt, (jl_tupletype_t*)method->sig);
+            mfunc = cache_method(mt, &method->invokes, entry->func.value, sig, tt, method, world, tpenv, 1);
         }
         JL_UNLOCK(&method->writelock);
     }
@@ -2068,14 +2075,13 @@ JL_DLLEXPORT jl_value_t *jl_get_invoke_lambda(jl_methtable_t *mt,
         assert(ti != (jl_value_t*)jl_bottom_type);
         (void)ti;
     }
-    sig = join_tsig(tt, entry->sig);
-    jl_method_t *func = entry->func.method;
 
-    if (func->invokes.unknown == NULL)
-        func->invokes.unknown = jl_nothing;
+    if (method->invokes.unknown == NULL)
+        method->invokes.unknown = jl_nothing;
 
-    jl_method_instance_t *mfunc = cache_method(mt, &func->invokes, entry->func.value,
-                                               sig, tt, entry, world, tpenv, 1);
+    sig  =  join_tsig(tt, (jl_tupletype_t*)method->sig);
+    jl_method_instance_t *mfunc = cache_method(mt, &method->invokes, entry->func.value,
+                                               sig, tt, method, world, tpenv, 1);
     JL_GC_POP();
     JL_UNLOCK(&method->writelock);
     return (jl_value_t*)mfunc;
@@ -2301,6 +2307,31 @@ static jl_value_t *ml_matches(union jl_typemap_t defs, int offs,
     return env.t;
 }
 
+// see if it might be possible to construct an instance of `typ`
+// if ninitialized == nfields, but a fieldtype is Union{},
+// that type will not be constructable, for example, tested recursively
+int jl_has_concrete_subtype(jl_value_t *typ)
+{
+    if (typ == jl_bottom_type)
+        return 0;
+    typ = jl_unwrap_unionall(typ);
+    if (jl_is_vararg_type(typ))
+        typ = jl_unwrap_vararg(typ);
+    if (!jl_is_datatype(typ))
+        return 1;
+    jl_svec_t *fields = ((jl_datatype_t*)typ)->types;
+    size_t i, l = jl_svec_len(fields);
+    if (l != ((jl_datatype_t*)typ)->ninitialized)
+        if (((jl_datatype_t*)typ)->name != jl_tuple_typename)
+            return 1;
+    for (i = 0; i < l; i++) {
+        jl_value_t *ft = jl_svecref(fields, i);
+        if (!jl_has_concrete_subtype(ft))
+            return 0;
+    }
+    return 1;
+}
+
 // TODO: separate the codegen and typeinf locks
 //   currently using a coarser lock seems like
 //   the best way to avoid acquisition priority

diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -459,6 +459,7 @@ int jl_tuple_isa(jl_value_t **child, size_t cl, jl_datatype_t *pdt);
 
 int jl_has_intersect_type_not_kind(jl_value_t *t);
 int jl_subtype_invariant(jl_value_t *a, jl_value_t *b, int ta);
+int jl_has_concrete_subtype(jl_value_t *typ);
 jl_datatype_t *jl_inst_concrete_tupletype_v(jl_value_t **p, size_t np);
 jl_datatype_t *jl_inst_concrete_tupletype(jl_svec_t *p);
 JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method, jl_tupletype_t *simpletype);
@@ -482,7 +483,6 @@ void jl_assign_bits(void *dest, jl_value_t *bits);
 jl_expr_t *jl_exprn(jl_sym_t *head, size_t n);
 jl_function_t *jl_new_generic_function(jl_sym_t *name, jl_module_t *module);
 jl_function_t *jl_new_generic_function_with_supertype(jl_sym_t *name, jl_module_t *module, jl_datatype_t *st, int iskw);
-jl_function_t *jl_module_call_func(jl_module_t *m);
 int jl_is_submodule(jl_module_t *child, jl_module_t *parent);
 
 jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int expanded);

diff --git a/src/precompile.c b/src/precompile.c
@@ -89,28 +89,6 @@ void jl_write_compiler_output(void)
     JL_GC_POP();
 }
 
-static int any_bottom_field(jl_value_t *typ)
-{
-    if (typ == jl_bottom_type)
-        return 1;
-    typ = jl_unwrap_unionall(typ);
-    if (jl_is_vararg_type(typ))
-        typ = jl_unwrap_vararg(typ);
-    if (!jl_is_datatype(typ))
-        return 0;
-    jl_svec_t *fields = ((jl_datatype_t*)typ)->types;
-    size_t i, l = jl_svec_len(fields);
-    if (l != ((jl_datatype_t*)typ)->ninitialized)
-        if (((jl_datatype_t*)typ)->name != jl_tuple_typename)
-            return 0;
-    for (i = 0; i < l; i++) {
-        jl_value_t *ft = jl_svecref(fields, i);
-        if (any_bottom_field(ft))
-            return 1;
-    }
-    return 0;
-}
-
 // f{<:Union{...}}(...) is a common pattern
 // and expanding the Union may give a leaf function
 static void _compile_all_tvar_union(jl_value_t *methsig)
@@ -146,7 +124,7 @@ static void _compile_all_tvar_union(jl_value_t *methsig)
         JL_CATCH {
             goto getnext; // sigh, we found an invalid type signature. should we warn the user?
         }
-        if (any_bottom_field(sig))
+        if (!jl_has_concrete_subtype(sig))
             goto getnext; // signature wouldn't be callable / is invalid -- skip it
         if (jl_is_leaf_type(sig)) {
             if (jl_compile_hint((jl_tupletype_t*)sig))