Skip to content

Commit 0c52cc8

Browse files
committed
types: per comment, avoid copy when t is not bound
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.
1 parent 4b0c8e7 commit 0c52cc8

File tree

1 file changed

+43
-45
lines changed

1 file changed

+43
-45
lines changed

src/jltypes.c

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,8 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
12731273
jl_cache_type_(ndt);
12741274
}
12751275
jl_svec_t *ftypes = dt->types;
1276+
if (ftypes == NULL)
1277+
ftypes = ((jl_datatype_t*)jl_unwrap_unionall(tn->wrapper))->types;
12761278
if (ftypes == NULL || dt->super == NULL) {
12771279
// in the process of creating this type definition:
12781280
// need to instantiate the super and types fields later
@@ -1287,7 +1289,10 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
12871289
}
12881290
else if (cacheable) {
12891291
// recursively instantiate the types of the fields
1290-
ndt->types = inst_ftypes(ftypes, env, stack);
1292+
if (dt->types == NULL)
1293+
ndt->types = jl_compute_fieldtypes(ndt);
1294+
else
1295+
ndt->types = inst_ftypes(ftypes, env, stack);
12911296
jl_gc_wb(ndt, ndt->types);
12921297
}
12931298
}
@@ -1419,12 +1424,13 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
14191424
jl_value_t **iparams;
14201425
int onstack = ntp < jl_page_size/sizeof(jl_value_t*);
14211426
JL_GC_PUSHARGS(iparams, onstack ? ntp : 1);
1422-
jl_svec_t *ip_heap=NULL;
1427+
jl_svec_t *ip_heap = NULL;
14231428
if (!onstack) {
14241429
ip_heap = jl_alloc_svec(ntp);
14251430
iparams[0] = (jl_value_t*)ip_heap;
14261431
iparams = jl_svec_data(ip_heap);
14271432
}
1433+
int bound = 0;
14281434
int cacheable = 1;
14291435
if (jl_is_va_tuple(tt))
14301436
cacheable = 0;
@@ -1435,12 +1441,14 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
14351441
iparams[i] = pi;
14361442
if (ip_heap)
14371443
jl_gc_wb(ip_heap, pi);
1444+
bound |= (pi != elt);
14381445
if (cacheable && !jl_is_concrete_type(pi))
14391446
cacheable = 0;
14401447
}
1441-
jl_value_t *result = inst_datatype((jl_datatype_t*)tt, ip_heap, iparams, ntp, cacheable, stack);
1448+
if (bound)
1449+
t = inst_datatype(tt, ip_heap, iparams, ntp, cacheable, stack);
14421450
JL_GC_POP();
1443-
return result;
1451+
return t;
14441452
}
14451453

14461454
static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t *stack, int check)
@@ -1451,88 +1459,78 @@ static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t
14511459
while (e != NULL) {
14521460
if (e->var == (jl_tvar_t*)t) {
14531461
jl_value_t *val = e->val;
1454-
// TODO jb/subtype this seems unnecessary
1455-
//if (check && !jl_is_typevar(val) && !within_typevar(val, (jl_tvar_t*)t)) {
1456-
// jl_type_error_rt("type parameter",
1457-
// jl_symbol_name(((jl_tvar_t*)t)->name), t, val);
1458-
//}
14591462
return val;
14601463
}
14611464
e = e->prev;
14621465
}
1463-
return (jl_value_t*)t;
1466+
return t;
14641467
}
14651468
if (jl_is_unionall(t)) {
1466-
if (!jl_has_free_typevars(t))
1467-
return t;
14681469
jl_unionall_t *ua = (jl_unionall_t*)t;
1469-
jl_value_t *res=NULL, *lb=ua->var->lb, *ub=ua->var->ub;
1470-
JL_GC_PUSH3(&lb, &ub, &res);
1471-
res = jl_new_struct(jl_unionall_type, ua->var, NULL);
1472-
if (jl_has_bound_typevars(ua->var->lb, env))
1473-
lb = inst_type_w_(ua->var->lb, env, stack, check);
1474-
if (jl_has_bound_typevars(ua->var->ub, env))
1475-
ub = inst_type_w_(ua->var->ub, env, stack, check);
1476-
if (lb != ua->var->lb || ub != ua->var->ub) {
1477-
((jl_unionall_t*)res)->var = jl_new_typevar(ua->var->name, lb, ub);
1478-
jl_gc_wb(res, ((jl_unionall_t*)res)->var);
1470+
jl_value_t *lb = NULL;
1471+
jl_value_t *var = NULL;
1472+
jl_value_t *newbody = NULL;
1473+
JL_GC_PUSH3(&lb, &var, &newbody);
1474+
lb = inst_type_w_(ua->var->lb, env, stack, check);
1475+
var = inst_type_w_(ua->var->ub, env, stack, check);
1476+
if (lb != ua->var->lb || var != ua->var->ub) {
1477+
var = (jl_value_t*)jl_new_typevar(ua->var->name, lb, var);
1478+
}
1479+
else {
1480+
var = (jl_value_t*)ua->var;
14791481
}
1480-
jl_typeenv_t newenv = { ua->var, (jl_value_t*)((jl_unionall_t*)res)->var, env };
1481-
jl_value_t *newbody = inst_type_w_(ua->body, &newenv, stack, check);
1482+
jl_typeenv_t newenv = { ua->var, var, env };
1483+
newbody = inst_type_w_(ua->body, &newenv, stack, check);
14821484
if (newbody == (jl_value_t*)jl_emptytuple_type) {
14831485
// NTuple{0} => Tuple{} can make a typevar disappear
1484-
res = (jl_value_t*)jl_emptytuple_type;
1486+
t = (jl_value_t*)jl_emptytuple_type;
14851487
}
1486-
else {
1487-
((jl_unionall_t*)res)->body = newbody;
1488-
jl_gc_wb(res, newbody);
1488+
else if (newbody != ua->body || var != (jl_value_t*)ua->var) {
1489+
// if t's parameters are not bound in the environment, return it uncopied (#9378)
1490+
t = jl_new_struct(jl_unionall_type, var, newbody);
14891491
}
14901492
JL_GC_POP();
1491-
return res;
1493+
return t;
14921494
}
14931495
if (jl_is_uniontype(t)) {
14941496
jl_uniontype_t *u = (jl_uniontype_t*)t;
14951497
jl_value_t *a = inst_type_w_(u->a, env, stack, check);
14961498
jl_value_t *b = NULL;
14971499
JL_GC_PUSH2(&a, &b);
14981500
b = inst_type_w_(u->b, env, stack, check);
1499-
jl_value_t *res;
1500-
if (a == u->a && b == u->b) {
1501-
res = t;
1502-
}
1503-
else {
1501+
if (a != u->a || b != u->b) {
15041502
jl_value_t *uargs[2] = {a, b};
1505-
res = jl_type_union(uargs, 2);
1503+
t = jl_type_union(uargs, 2);
15061504
}
15071505
JL_GC_POP();
1508-
return res;
1506+
return t;
15091507
}
15101508
if (!jl_is_datatype(t))
15111509
return t;
15121510
jl_datatype_t *tt = (jl_datatype_t*)t;
15131511
jl_svec_t *tp = tt->parameters;
15141512
if (tp == jl_emptysvec)
1515-
return (jl_value_t*)t;
1513+
return t;
15161514
jl_typename_t *tn = tt->name;
15171515
if (tn == jl_tuple_typename)
15181516
return inst_tuple_w_(t, env, stack, check);
15191517
size_t ntp = jl_svec_len(tp);
15201518
jl_value_t **iparams;
15211519
JL_GC_PUSHARGS(iparams, ntp);
15221520
int cacheable = 1, bound = 0;
1523-
for(i=0; i < ntp; i++) {
1521+
for (i = 0; i < ntp; i++) {
15241522
jl_value_t *elt = jl_svecref(tp, i);
1525-
iparams[i] = inst_type_w_(elt, env, stack, check);
1526-
bound |= (iparams[i] != elt);
1527-
if (cacheable && jl_has_free_typevars(iparams[i]))
1523+
jl_value_t *pi = inst_type_w_(elt, env, stack, check);
1524+
iparams[i] = pi;
1525+
bound |= (pi != elt);
1526+
if (cacheable && jl_has_free_typevars(pi))
15281527
cacheable = 0;
15291528
}
15301529
// if t's parameters are not bound in the environment, return it uncopied (#9378)
1531-
if (!bound) { JL_GC_POP(); return (jl_value_t*)t; }
1532-
1533-
jl_value_t *result = inst_datatype((jl_datatype_t*)tt, NULL, iparams, ntp, cacheable, stack);
1530+
if (bound)
1531+
t = inst_datatype_inner(tt, NULL, iparams, ntp, cacheable, stack, env);
15341532
JL_GC_POP();
1535-
return result;
1533+
return t;
15361534
}
15371535

15381536
static jl_value_t *instantiate_with(jl_value_t *t, jl_value_t **env, size_t n, jl_typeenv_t *te)

0 commit comments

Comments
 (0)