Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
May 31, 2019
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 74 additions & 80 deletions src/jltypes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -1262,6 +1278,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;
}
Expand All @@ -1273,10 +1290,12 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
jl_cache_type_(ndt);
}
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 if (!isnamedtuple && !istuple) {
Expand All @@ -1287,7 +1306,10 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
}
else if (cacheable) {
// recursively instantiate the types of the fields
ndt->types = inst_ftypes(ftypes, env, stack);
if (dt->types == NULL)
ndt->types = jl_compute_fieldtypes(ndt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this is needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's reveled from an interaction of the PR with lazy-ftypes (although just the type-stack alone could theoretically run into the same problem with this PR). Previously, on arriving in this function, the dt was always the wrapper object and so the env was already that which will be created by jl_compute_fieldtypes. With the PR, it now could be a partially computed type that just needs to be copied, but that also means it could now be the final parameter application to create a concrete type (where with lazy-ftypes, we are now not been computing the ftypes of that intermediate work).

else
ndt->types = inst_ftypes(ftypes, env, stack);
jl_gc_wb(ndt, ndt->types);
}
}
Expand Down Expand Up @@ -1315,25 +1337,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;
Expand All @@ -1342,9 +1345,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)
Expand All @@ -1359,12 +1360,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_ftypes(jl_svec_t *p, jl_typeenv_t *env, jl_typestack_t *stack)
Expand Down Expand Up @@ -1393,7 +1394,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
Expand All @@ -1419,12 +1420,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;
Expand All @@ -1435,12 +1437,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)
Expand All @@ -1451,88 +1455,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] = 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;
}

static jl_value_t *instantiate_with(jl_value_t *t, jl_value_t **env, size_t n, jl_typeenv_t *te)
Expand All @@ -1549,21 +1543,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);
}

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;
Expand Down