types: per comment, avoid copy when t is not bound (#32176)

As mentioned in #9378.
Fix recursion issue mentioned in #25796 by using inst_datatype_inner
instead of inst_datatype, so that we shouldn't be making copies of
any non-bound objects (anything maybe-cacheable) now.
(cherry picked from commit 217507f)

Author: vtjnash

src/jltypes.c

@@ -819,8 +819,21 @@ static int within_typevar(jl_value_t *t, jl_value_t *vlb, jl_value_t *vub)
 struct _jl_typestack_t;
 typedef struct _jl_typestack_t jl_typestack_t;
 
-static jl_value_t *inst_datatype(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
-                                 int cacheable, jl_typestack_t *stack);
+static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
+                                       int cacheable, jl_typestack_t *stack, jl_typeenv_t *env);
+
+// Build an environment mapping a TypeName's parameters to parameter values.
+// This is the environment needed for instantiating a type's supertype and field types.
+static jl_value_t *inst_datatype_env(jl_value_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
+                                     int cacheable, jl_typestack_t *stack, jl_typeenv_t *env, int c)
+{
+    if (jl_is_datatype(dt))
+        return inst_datatype_inner((jl_datatype_t*)dt, p, iparams, ntp, cacheable, stack, env);
+    assert(jl_is_unionall(dt));
+    jl_unionall_t *ua = (jl_unionall_t*)dt;
+    jl_typeenv_t e = { ua->var, iparams[c], env };
+    return inst_datatype_env(ua->body, p, iparams, ntp, cacheable, stack, &e, c + 1);
+}
 
 jl_value_t *jl_apply_type(jl_value_t *tc, jl_value_t **params, size_t n)
 {
@@ -836,12 +849,13 @@ jl_value_t *jl_apply_type(jl_value_t *tc, jl_value_t **params, size_t n)
         if (jl_is_datatype(u) && n == jl_nparams((jl_datatype_t*)u) &&
             ((jl_datatype_t*)u)->name->wrapper == tc) {
             int cacheable = 1;
-            for(i=0; i < n; i++) {
+            for (i = 0; i < n; i++) {
                 if (jl_has_free_typevars(params[i])) {
-                    cacheable = 0; break;
+                    cacheable = 0;
+                    break;
                 }
             }
-            return inst_datatype((jl_datatype_t*)u, NULL, params, n, cacheable, NULL);
+            return inst_datatype_env(tc, NULL, params, n, cacheable, NULL, NULL, 0);
         }
     }
     JL_GC_PUSH1(&tc);
@@ -1068,6 +1082,8 @@ static jl_value_t *normalize_vararg(jl_value_t *va)
     return va;
 }
 
+static jl_value_t *_jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals, jl_typeenv_t *prev, jl_typestack_t *stack);
+
 static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
                                        int cacheable, jl_typestack_t *stack, jl_typeenv_t *env)
 {
@@ -1244,6 +1260,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
     if (cacheable && !ndt->abstract)
         ndt->uid = jl_assign_type_uid();
 
+    jl_datatype_t *primarydt = ((jl_datatype_t*)jl_unwrap_unionall(tn->wrapper));
     if (istuple || isnamedtuple) {
         ndt->super = jl_any_type;
     }
@@ -1252,10 +1269,12 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
         jl_gc_wb(ndt, ndt->super);
     }
     jl_svec_t *ftypes = dt->types;
+    if (ftypes == NULL)
+        ftypes = primarydt->types;
     if (ftypes == NULL || dt->super == NULL) {
         // in the process of creating this type definition:
         // need to instantiate the super and types fields later
-        assert(inside_typedef && !istuple && !isnamedtuple);
+        assert((inside_typedef || primarydt->super) && !istuple && !isnamedtuple);
         arraylist_push(&partial_inst, ndt);
     }
     else {
@@ -1292,25 +1311,6 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
     return (jl_value_t*)ndt;
 }
 
-// Build an environment mapping a TypeName's parameters to parameter values.
-// This is the environment needed for instantiating a type's supertype and field types.
-static jl_value_t *inst_datatype_env(jl_value_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
-                                     int cacheable, jl_typestack_t *stack, jl_typeenv_t *env, int c)
-{
-    if (jl_is_datatype(dt))
-        return inst_datatype_inner((jl_datatype_t*)dt, p, iparams, ntp, cacheable, stack, env);
-    assert(jl_is_unionall(dt));
-    jl_unionall_t *ua = (jl_unionall_t*)dt;
-    jl_typeenv_t e = { ua->var, iparams[c], env };
-    return inst_datatype_env(ua->body, p, iparams, ntp, cacheable, stack, &e, c+1);
-}
-
-static jl_value_t *inst_datatype(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
-                                 int cacheable, jl_typestack_t *stack)
-{
-    return inst_datatype_env(dt->name->wrapper, p, iparams, ntp, cacheable, stack, NULL, 0);
-}
-
 static jl_tupletype_t *jl_apply_tuple_type_v_(jl_value_t **p, size_t np, jl_svec_t *params)
 {
     int cacheable = 1;
@@ -1319,9 +1319,7 @@ static jl_tupletype_t *jl_apply_tuple_type_v_(jl_value_t **p, size_t np, jl_svec
         if (!jl_is_concrete_type(p[i]))
             cacheable = 0;
     }
-    jl_datatype_t *ndt = (jl_datatype_t*)inst_datatype(jl_anytuple_type, params, p, np,
-                                                       cacheable, NULL);
-    return ndt;
+    return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, params, p, np, cacheable, NULL, NULL);
 }
 
 JL_DLLEXPORT jl_tupletype_t *jl_apply_tuple_type(jl_svec_t *params)
@@ -1336,12 +1334,12 @@ JL_DLLEXPORT jl_tupletype_t *jl_apply_tuple_type_v(jl_value_t **p, size_t np)
 
 jl_datatype_t *jl_inst_concrete_tupletype(jl_svec_t *p)
 {
-    return (jl_datatype_t*)inst_datatype(jl_anytuple_type, p, jl_svec_data(p), jl_svec_len(p), 1, NULL);
+    return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, p, jl_svec_data(p), jl_svec_len(p), 1, NULL, NULL);
 }
 
 jl_datatype_t *jl_inst_concrete_tupletype_v(jl_value_t **p, size_t np)
 {
-    return (jl_datatype_t*)inst_datatype(jl_anytuple_type, NULL, p, np, 1, NULL);
+    return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, NULL, p, np, 1, NULL, NULL);
 }
 
 static jl_svec_t *inst_all(jl_svec_t *p, jl_typeenv_t *env, jl_typestack_t *stack, int check)
@@ -1363,7 +1361,7 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
     jl_svec_t *tp = tt->parameters;
     size_t ntp = jl_svec_len(tp);
     // Instantiate NTuple{3,Int}
-    // Note this does not instantiate Tuple{Vararg{Int,3}}; that's done in inst_datatype
+    // Note this does not instantiate Tuple{Vararg{Int,3}}; that's done in inst_datatype_inner
     if (jl_is_va_tuple(tt) && ntp == 1) {
         // If this is a Tuple{Vararg{T,N}} with known N, expand it to
         // a fixed-length tuple
@@ -1389,12 +1387,13 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
     jl_value_t **iparams;
     int onstack = ntp < jl_page_size/sizeof(jl_value_t*);
     JL_GC_PUSHARGS(iparams, onstack ? ntp : 1);
-    jl_svec_t *ip_heap=NULL;
+    jl_svec_t *ip_heap = NULL;
     if (!onstack) {
         ip_heap = jl_alloc_svec(ntp);
         iparams[0] = (jl_value_t*)ip_heap;
         iparams = jl_svec_data(ip_heap);
     }
+    int bound = 0;
     int cacheable = 1;
     if (jl_is_va_tuple(tt))
         cacheable = 0;
@@ -1405,12 +1404,14 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
         iparams[i] = pi;
         if (ip_heap)
             jl_gc_wb(ip_heap, pi);
+        bound |= (pi != elt);
         if (cacheable && !jl_is_concrete_type(pi))
             cacheable = 0;
     }
-    jl_value_t *result = inst_datatype((jl_datatype_t*)tt, ip_heap, iparams, ntp, cacheable, stack);
+    if (bound)
+        t = inst_datatype_inner(tt, ip_heap, iparams, ntp, cacheable, stack, env);
     JL_GC_POP();
-    return result;
+    return t;
 }
 
 static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t *stack, int check)
@@ -1421,88 +1422,78 @@ static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t
         while (e != NULL) {
             if (e->var == (jl_tvar_t*)t) {
                 jl_value_t *val = e->val;
-                // TODO jb/subtype this seems unnecessary
-                //if (check && !jl_is_typevar(val) && !within_typevar(val, (jl_tvar_t*)t)) {
-                //    jl_type_error_rt("type parameter",
-                //                     jl_symbol_name(((jl_tvar_t*)t)->name), t, val);
-                //}
                 return val;
             }
             e = e->prev;
         }
-        return (jl_value_t*)t;
+        return t;
     }
     if (jl_is_unionall(t)) {
-        if (!jl_has_free_typevars(t))
-            return t;
         jl_unionall_t *ua = (jl_unionall_t*)t;
-        jl_value_t *res=NULL, *lb=ua->var->lb, *ub=ua->var->ub;
-        JL_GC_PUSH3(&lb, &ub, &res);
-        res = jl_new_struct(jl_unionall_type, ua->var, NULL);
-        if (jl_has_bound_typevars(ua->var->lb, env))
-            lb = inst_type_w_(ua->var->lb, env, stack, check);
-        if (jl_has_bound_typevars(ua->var->ub, env))
-            ub = inst_type_w_(ua->var->ub, env, stack, check);
-        if (lb != ua->var->lb || ub != ua->var->ub) {
-            ((jl_unionall_t*)res)->var = jl_new_typevar(ua->var->name, lb, ub);
-            jl_gc_wb(res, ((jl_unionall_t*)res)->var);
+        jl_value_t *lb = NULL;
+        jl_value_t *var = NULL;
+        jl_value_t *newbody = NULL;
+        JL_GC_PUSH3(&lb, &var, &newbody);
+        lb = inst_type_w_(ua->var->lb, env, stack, check);
+        var = inst_type_w_(ua->var->ub, env, stack, check);
+        if (lb != ua->var->lb || var != ua->var->ub) {
+            var = (jl_value_t*)jl_new_typevar(ua->var->name, lb, var);
+        }
+        else {
+            var = (jl_value_t*)ua->var;
         }
-        jl_typeenv_t newenv = { ua->var, (jl_value_t*)((jl_unionall_t*)res)->var, env };
-        jl_value_t *newbody = inst_type_w_(ua->body, &newenv, stack, check);
+        jl_typeenv_t newenv = { ua->var, var, env };
+        newbody = inst_type_w_(ua->body, &newenv, stack, check);
         if (newbody == (jl_value_t*)jl_emptytuple_type) {
             // NTuple{0} => Tuple{} can make a typevar disappear
-            res = (jl_value_t*)jl_emptytuple_type;
+            t = (jl_value_t*)jl_emptytuple_type;
         }
-        else {
-            ((jl_unionall_t*)res)->body = newbody;
-            jl_gc_wb(res, newbody);
+        else if (newbody != ua->body || var != (jl_value_t*)ua->var) {
+            // if t's parameters are not bound in the environment, return it uncopied (#9378)
+            t = jl_new_struct(jl_unionall_type, var, newbody);
         }
         JL_GC_POP();
-        return res;
+        return t;
     }
     if (jl_is_uniontype(t)) {
         jl_uniontype_t *u = (jl_uniontype_t*)t;
         jl_value_t *a = inst_type_w_(u->a, env, stack, check);
         jl_value_t *b = NULL;
         JL_GC_PUSH2(&a, &b);
         b = inst_type_w_(u->b, env, stack, check);
-        jl_value_t *res;
-        if (a == u->a && b == u->b) {
-            res = t;
-        }
-        else {
+        if (a != u->a || b != u->b) {
             jl_value_t *uargs[2] = {a, b};
-            res = jl_type_union(uargs, 2);
+            t = jl_type_union(uargs, 2);
         }
         JL_GC_POP();
-        return res;
+        return t;
     }
     if (!jl_is_datatype(t))
         return t;
     jl_datatype_t *tt = (jl_datatype_t*)t;
     jl_svec_t *tp = tt->parameters;
     if (tp == jl_emptysvec)
-        return (jl_value_t*)t;
+        return t;
     jl_typename_t *tn = tt->name;
     if (tn == jl_tuple_typename)
         return inst_tuple_w_(t, env, stack, check);
     size_t ntp = jl_svec_len(tp);
     jl_value_t **iparams;
     JL_GC_PUSHARGS(iparams, ntp);
     int cacheable = 1, bound = 0;
-    for(i=0; i < ntp; i++) {
+    for (i = 0; i < ntp; i++) {
         jl_value_t *elt = jl_svecref(tp, i);
-        iparams[i] = (jl_value_t*)inst_type_w_(elt, env, stack, check);
-        bound |= (iparams[i] != elt);
-        if (cacheable && jl_has_free_typevars(iparams[i]))
+        jl_value_t *pi = inst_type_w_(elt, env, stack, check);
+        iparams[i] = pi;
+        bound |= (pi != elt);
+        if (cacheable && jl_has_free_typevars(pi))
             cacheable = 0;
     }
     // if t's parameters are not bound in the environment, return it uncopied (#9378)
-    if (!bound) { JL_GC_POP(); return (jl_value_t*)t; }
-
-    jl_value_t *result = inst_datatype((jl_datatype_t*)tt, NULL, iparams, ntp, cacheable, stack);
+    if (bound)
+        t = inst_datatype_inner(tt, NULL, iparams, ntp, cacheable, stack, env);
     JL_GC_POP();
-    return result;
+    return t;
 }
 
 jl_value_t *instantiate_with(jl_value_t *t, jl_value_t **env, size_t n, jl_typeenv_t *te, jl_typestack_t *stack)
@@ -1519,21 +1510,21 @@ jl_value_t *jl_instantiate_type_with(jl_value_t *t, jl_value_t **env, size_t n)
     return instantiate_with(t, env, n, NULL, NULL);
 }
 
-static jl_value_t *_jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals, jl_typeenv_t *prev)
+static jl_value_t *_jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals, jl_typeenv_t *prev, jl_typestack_t *stack)
 {
     jl_typeenv_t en = { env->var, vals[0], prev };
     if (jl_is_unionall(env->body))
-        return _jl_instantiate_type_in_env(ty, (jl_unionall_t*)env->body, vals + 1, &en);
+        return _jl_instantiate_type_in_env(ty, (jl_unionall_t*)env->body, vals + 1, &en, stack);
     else
-        return inst_type_w_(ty, &en, NULL, 1);
+        return inst_type_w_(ty, &en, stack, 1);
 }
 
 JL_DLLEXPORT jl_value_t *jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals)
 {
     jl_value_t *typ = ty;
     if (jl_is_unionall(env)) {
         JL_TRY {
-            typ = _jl_instantiate_type_in_env(ty, env, vals, NULL);
+            typ = _jl_instantiate_type_in_env(ty, env, vals, NULL, NULL);
         }
         JL_CATCH {
             typ = jl_bottom_type;
@@ -1599,11 +1590,14 @@ void jl_reinstantiate_inner_types(jl_datatype_t *t) // can throw!
 
             int k;
             assert(ndt->types == NULL);
-            ndt->types = jl_alloc_svec(jl_svec_len(t->types));
-            jl_gc_wb(ndt, ndt->types);
+            jl_svec_t *ft = jl_alloc_svec(jl_svec_len(t->types));
+            JL_GC_PUSH1(&ft);
             for (k=0; k < jl_svec_len(t->types); k++) {
-                jl_svecset(ndt->types, k, instantiate_with(jl_svecref(t->types,k), env, n, NULL, &top));
+                jl_svecset(ft, k, instantiate_with(jl_svecref(t->types,k), env, n, NULL, &top));
             }
+            ndt->types = ft;
+            jl_gc_wb(ndt, ndt->types);
+            JL_GC_POP();
             if (ndt->uid) { // cacheable
                 jl_compute_field_offsets(ndt);
             }