move type-signature correction to a separate function

this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches
this also improves the fix for #11840 to cover Union
but breaks the fix for #11355 -- fortunately, our improved cache should soon be up to the challenge of fixing it too

Author: vtjnash

src/gf.c

@@ -281,6 +281,78 @@ static jl_value_t *lookup_match(jl_value_t *a, jl_value_t *b, jl_svec_t **penv,
     return ti;
 }
 
+jl_value_t *jl_nth_slot_type(jl_tupletype_t *sig, size_t i)
+{
+    size_t len = jl_datatype_nfields(sig);
+    if (len == 0)
+        return NULL;
+    if (i < len-1)
+        return jl_tparam(sig, i);
+    if (jl_is_vararg_type(jl_tparam(sig,len-1)))
+        return jl_tparam0(jl_tparam(sig,len-1));
+    if (i == len-1)
+        return jl_tparam(sig, i);
+    return NULL;
+}
+
+// after intersection, the argument tuple type needs to be corrected to reflect the signature match
+// that occurred, if the arguments contained a Type but the signature matched on the kind
+static jl_tupletype_t *join_tsig(jl_tupletype_t *tt, jl_tupletype_t *sig)
+{
+    jl_svec_t *newparams = NULL;
+    JL_GC_PUSH1(&newparams);
+    int changed = 0;
+    size_t i, np;
+    for (i = 0, np = jl_nparams(tt); i < np; i++) {
+        jl_value_t *elt = jl_tparam(tt, i);
+        jl_value_t *newelt = NULL;
+        jl_value_t *decl_i = jl_nth_slot_type(sig, i);
+        int isva = jl_is_vararg_type(elt);
+        if (isva) elt = jl_tparam0(elt);
+
+        if (jl_is_type_type(elt)) {
+            // unless the type signature matched T in `::Type{T}`
+            // the result of matching the type signature
+            // needs to be corrected to the leaf type 'kind'
+            // (for performance and/or correctness)
+            jl_value_t *kind = jl_typeof(jl_tparam0(elt));
+            if (jl_subtype(kind, decl_i, 0)) {
+                if (jl_subtype((jl_value_t*)jl_type_type, decl_i, 0)) { // declared type was Any or Union{Type, ...}
+                    // if there are bound typevars, the user indicated this exact specialization was required
+                    // don't change it or it might mismatch the env
+                    if (!jl_has_typevars(decl_i))
+                        newelt = (jl_value_t*)jl_type_type;
+                }
+                else {
+                    // TypeConstructors are problematic because they can be alternate
+                    // representations of any type. If we matched this method because
+                    // it matched the leaf type TypeConstructor, then don't
+                    // cache something different since that doesn't necessarily actually apply
+                    //
+                    // similarly, if we matched Type{T<:Any}::DataType,
+                    // then we don't want to cache it that way
+                    // since lookup will think we matched ::Type{T}
+                    // and that is quite a different thing
+                    newelt = kind;
+                }
+            }
+        }
+        // prepare to build a new type with the replacement above
+        if (newelt) {
+            if (isva) newelt = (jl_value_t*)jl_wrap_vararg(newelt);
+            if (!changed) {
+                newparams = jl_svec_copy(tt->parameters);
+                changed = 1;
+            }
+            jl_svecset(newparams, i, newelt);
+        }
+    }
+    if (changed)
+        tt = jl_apply_tuple_type(newparams);
+    JL_GC_POP();
+    return tt;
+}
+
 static int sigs_eq(jl_value_t *a, jl_value_t *b, int useenv)
 {
     if (jl_has_typevars(a) || jl_has_typevars(b)) {
@@ -1056,27 +1128,6 @@ void jl_type_infer(jl_lambda_info_t *li, jl_value_t *toplevel)
 #endif
 }
 
-jl_value_t *jl_nth_slot_type(jl_tupletype_t *sig, size_t i)
-{
-    size_t len = jl_datatype_nfields(sig);
-    if (len == 0)
-        return NULL;
-    if (i < len-1)
-        return jl_tparam(sig, i);
-    if (jl_is_vararg_type(jl_tparam(sig,len-1)))
-        return jl_tparam0(jl_tparam(sig,len-1));
-    if (i == len-1)
-        return jl_tparam(sig, i);
-    return NULL;
-}
-
-static int very_general_type(jl_value_t *t)
-{
-    return (t && (t==(jl_value_t*)jl_any_type || t == (jl_value_t*)jl_type_type ||
-                  (jl_is_typevar(t) &&
-                   ((jl_tvar_t*)t)->ub==(jl_value_t*)jl_any_type)));
-}
-
 static int is_kind(jl_value_t *v)
 {
     return (v==(jl_value_t*)jl_uniontype_type ||
@@ -1110,27 +1161,6 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union _jl_opaque_cache
     for (i=0; i < np; i++) {
         jl_value_t *elt = jl_tparam(type,i);
         jl_value_t *decl_i = jl_nth_slot_type(decl,i);
-        if (!isstaged && jl_is_type_type(elt) && jl_is_tuple_type(jl_tparam0(elt)) &&
-            !(jl_subtype(decl_i, (jl_value_t*)jl_type_type, 0) && !is_kind(decl_i))) {
-            jl_methlist_t *curr = mt->defs;
-            int ok=1;
-            while (curr != (void*)jl_nothing) {
-                jl_value_t *slottype = jl_nth_slot_type(curr->sig, i);
-                if (slottype && curr->func != method) {
-                    if (jl_is_type_type(slottype) &&
-                        jl_type_intersection(slottype, decl_i) != jl_bottom_type) {
-                        ok=0;
-                        break;
-                    }
-                }
-                curr = curr->next;
-            }
-            if (ok) {
-                elt = jl_typeof(jl_tparam0(elt));
-                jl_svecset(newparams, i, elt);
-            }
-        }
-
         int set_to_any = 0;
         int notcalled_func = (i>0 && i<=8 && !(spec->called&(1<<(i-1))) &&
                               jl_subtype(elt,(jl_value_t*)jl_function_type,0));
@@ -1179,24 +1209,9 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union _jl_opaque_cache
                 }
             }
         }
+
         if (set_to_any || isstaged) {
         }
-        else if (jl_is_type_type(elt) && jl_is_typector(jl_tparam0(elt)) &&
-                 decl_i == (jl_value_t*)jl_typector_type) {
-            // TypeConstructors are problematic because they can be alternate
-            // representations of any type. If we matched this method because
-            // it matched the leaf type TypeConstructor, then don't
-            // cache something different since that doesn't necessarily actually apply
-            jl_svecset(newparams, i, jl_typector_type);
-        }
-        else if (jl_is_type_type(elt) && decl_i == (jl_value_t*)jl_datatype_type) {
-            // similarly, if we matched Type{T<:Any}::DataType,
-            // then we don't want to cache it that way
-            // since lookup will think we matched ::Type{T}
-            // and that is quite a different thing
-            jl_svecset(newparams, i, jl_datatype_type);
-            need_guard_entries = 1; // DataType has a UID so its precedence in the cache may be too high
-        }
         else if (jl_is_type_type(elt) && jl_is_type_type(jl_tparam0(elt)) &&
                  // give up on specializing static parameters for Type{Type{Type{...}}}
                  (jl_is_type_type(jl_tparam0(jl_tparam0(elt))) ||
@@ -1228,90 +1243,8 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union _jl_opaque_cache
             need_guard_entries = 1;
             assert(jl_svecref(newparams,i) != (jl_value_t*)jl_bottom_type);
         }
-        else if (jl_is_type_type(elt) && very_general_type(decl_i) &&
-                 !jl_has_typevars(decl_i)) {
-            /*
-              here's a fairly complex heuristic: if this argument slot's
-              declared type is Any, and no definition overlaps with Type
-              for this slot, then don't specialize for every Type that
-              might be passed.
-              Since every type x has its own type Type{x}, this would be
-              excessive specialization for an Any slot.
-
-              TypeConstructors are problematic because they can be alternate
-              representations of any type. Extensionally, TC == TC.body, but
-              typeof(TC) != typeof(TC.body). This creates an ambiguity:
-              Type{TC} is type-equal to Type{TC.body}, yet a slot
-              x::TypeConstructor matches the first but not the second, while
-              also matching all other TypeConstructors. This means neither
-              Type{TC} nor TypeConstructor is more specific.
-
-              To solve this, we identify "kind slots", which are slots
-              for which some definition specifies a kind (e.g. DataType).
-              Those tend to be in reflective functions that look at types
-              themselves. For these slots we specialize on jl_typeof(T) instead
-              of Type{T}, i.e. the kind of the type rather than the specific
-              type.
-            */
-            int ok=1, kindslot=0;
-            jl_methlist_t *curr = mt->defs;
-            jl_value_t *kind = (jl_value_t*)jl_typeof(jl_tparam0(elt));
-            while (curr != (void*)jl_nothing) {
-                jl_value_t *slottype = jl_nth_slot_type(curr->sig, i);
-                if (slottype && curr->func != method) {
-                    if (slottype == kind) {
-                        ok=0;
-                        break;
-                    }
-                    if (is_kind(slottype))
-                        kindslot=1;
-                }
-                curr = curr->next;
-            }
-            if (ok) {
-                if (kindslot) {
-                    jl_svecset(newparams, i, kind);
-                }
-                else {
-                    curr = mt->defs;
-                    while (curr != (void*)jl_nothing) {
-                        jl_value_t *slottype = jl_nth_slot_type(curr->sig, i);
-                        if (slottype && curr->func != method) {
-                            if (!very_general_type(slottype) &&
-                                jl_type_intersection(slottype, (jl_value_t*)jl_type_type) !=
-                                (jl_value_t*)jl_bottom_type) {
-                                ok=0;
-                                break;
-                            }
-                        }
-                        curr = curr->next;
-                    }
-                    if (ok) {
-                        jl_svecset(newparams, i, jl_typetype_type);
-                        need_guard_entries = 1;
-                    }
-                }
-            }
-        }
-        else if (is_kind(decl_i)) {
-            // if a slot is specialized for a particular kind, it can be
-            // considered a reflective method and so only needs to be
-            // specialized for type representation, not type extent.
-            jl_methlist_t *curr = mt->defs;
-            int ok=1;
-            while (curr != (void*)jl_nothing) {
-                jl_value_t *slottype = jl_nth_slot_type(curr->sig, i);
-                if (slottype && curr->func != method) {
-                    if (jl_is_type_type(slottype) &&
-                        jl_type_intersection(slottype, decl_i) != jl_bottom_type) {
-                        ok=0;
-                        break;
-                    }
-                }
-                curr = curr->next;
-            }
-            if (ok)
-                jl_svecset(newparams, i, decl_i);
+        else if (is_kind(elt)) {
+            need_guard_entries = 1;
         }
     }
 
@@ -1394,6 +1327,7 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union _jl_opaque_cache
                 }
             }
             if (unmatched_tvars) {
+                // FIXME: this is invalid (c.f. intersection of #11355 and #11840)
                 // if distinguishing a guard entry from the generalized signature
                 // would require matching type vars then bail out, since the
                 // method cache matching algorithm cannot do that.
@@ -1558,15 +1492,15 @@ static jl_lambda_info_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype_t *
     jl_methlist_t *m = NULL;
     jl_svec_t *env = jl_emptysvec;
     jl_lambda_info_t *func = NULL;
-    JL_GC_PUSH3(&env, &m, &func);
+    JL_GC_PUSH4(&env, &m, &func, &tt);
 
     m = jl_method_cache_assoc_by_type_(mt->defs, tt, inexact, &env);
     if (m == NULL) {
         JL_GC_POP();
         return NULL;
     }
 
-    assert(jl_is_svec(env));
+    tt = join_tsig(tt, m->sig);
     func = m->func;
     if (func->isstaged)
         func = jl_instantiate_staged(func, tt, env);
@@ -2234,6 +2168,7 @@ jl_value_t *jl_gf_invoke(jl_tupletype_t *types0, jl_value_t **args, size_t nargs
             assert(ti != (jl_value_t*)jl_bottom_type);
             (void)ti;
         }
+        tt = join_tsig(tt, m->sig);
         jl_lambda_info_t *func = m->func;
 
         if (func->invokes.nothing == NULL)