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

Fix typeintersect bug reported in #36443 #46446

Merged
merged 9 commits into from
Sep 2, 2022
Merged

Fix typeintersect bug reported in #36443 #46446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
01a4a30
Avoid setting `offset` when we intersect `Vararg`'s eltype.
N5N3 Aug 24, 2022
e6d2624
Skip subtype check if `intersect_invariant` calls `set_vat_to_const`.
N5N3 Aug 31, 2022
f29a013
Add tuple length offset when we intersect 2 `Vararg`'s length.
N5N3 Aug 23, 2022
38829a0
Always return the shorter `Vararg` length.
N5N3 Aug 23, 2022
b7f47d5
Avoid set `var`'s bounds if `offset != 0`
N5N3 Aug 23, 2022
a6a5f00
subtype: add tests for `Vararg` expansion.
N5N3 Aug 23, 2022
1268582
Make `bound_var_below` return `NULL` if the input typevar is not valid.
N5N3 Aug 29, 2022
2abedf6
Avoid set var's lb if intersect return a Vararg with free length. (Nu…
N5N3 Aug 29, 2022
4654cfa
subtyping: add tests for various Vararg bounds bugs
N5N3 Aug 29, 2022
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
147 changes: 102 additions & 45 deletions src/subtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ typedef struct jl_varbinding_t {
// 1 - var.ub = ub; return var
// 2 - either (var.ub = ub; return var), or return ub
int8_t constraintkind;
int8_t intvalued; // must be integer-valued; i.e. occurs as N in Vararg{_,N}
// intvalued: must be integer-valued; i.e. occurs as N in Vararg{_,N}
N5N3 marked this conversation as resolved.
Show resolved Hide resolved
// 0: No restriction
// 1: must be unbounded/ or fixed to a `Int`/typevar
// 2: we have some imprecise vararg length intersection that can be improved if this var is const valued.
int8_t intvalued;
int8_t limited;
int16_t depth0; // # of invariant constructors nested around the UnionAll type for this var
// when this variable's integer value is compared to that of another,
Expand Down Expand Up @@ -2165,13 +2169,24 @@ static jl_value_t *set_var_to_const(jl_varbinding_t *bb, jl_value_t *v JL_MAYBE_
offset = -othervar->offset;
assert(!othervar || othervar->offset == -offset);
if (bb->lb == jl_bottom_type && bb->ub == (jl_value_t*)jl_any_type) {
if (jl_is_long(v))
v = jl_box_long(jl_unbox_long(v) + offset);
bb->lb = bb->ub = v;
if (offset == 0)
bb->lb = bb->ub = v;
else if (jl_is_long(v)) {
size_t iv = jl_unbox_long(v);
v = jl_box_long(iv + offset);
bb->lb = bb->ub = v;
// Here we always return the shorter `Vararg`'s length.
if (offset > 0)
return jl_box_long(iv);
}
else
return jl_bottom_type;
}
else if (jl_is_long(v) && jl_is_long(bb->lb)) {
if (jl_unbox_long(v) != jl_unbox_long(bb->lb))
if (jl_unbox_long(v) + offset != jl_unbox_long(bb->lb))
N5N3 marked this conversation as resolved.
Show resolved Hide resolved
return jl_bottom_type;
// Here we always return the shorter `Vararg`'s length.
if (offset < 0) return bb->lb;
}
else if (!jl_egal(v, bb->lb)) {
return jl_bottom_type;
Expand All @@ -2186,11 +2201,17 @@ static jl_value_t *bound_var_below(jl_tvar_t *tv, jl_varbinding_t *bb, jl_stenv_
return jl_bottom_type;
record_var_occurrence(bb, e, 2);
if (jl_is_long(bb->lb)) {
if (bb->offset == 0)
return bb->lb;
if (jl_unbox_long(bb->lb) < bb->offset)
ssize_t blb = jl_unbox_long(bb->lb);
if ((blb < bb->offset) || (blb < 0))
return jl_bottom_type;
return jl_box_long(jl_unbox_long(bb->lb) - bb->offset);
// Here we always return the shorter `Vararg`'s length.
if (bb->offset <= 0)
return bb->lb;
return jl_box_long(blb - bb->offset);
}
if (bb->offset > 0) {
bb->intvalued = 2;
return NULL;
}
return (jl_value_t*)tv;
}
Expand Down Expand Up @@ -2280,6 +2301,26 @@ static int check_unsat_bound(jl_value_t *t, jl_tvar_t *v, jl_stenv_t *e) JL_NOTS
return 0;
}

static int has_free_vararg_length(jl_value_t *a, jl_stenv_t *e) {
if (jl_is_unionall(a))
a = jl_unwrap_unionall(a);
if (jl_is_datatype(a) && jl_is_tuple_type((jl_datatype_t *)a)) {
size_t lx = jl_nparams((jl_datatype_t *)a);
if (lx > 0) {
jl_value_t *la = jl_tparam((jl_datatype_t *)a, lx-1);
if (jl_is_vararg(la)) {
jl_value_t *len = jl_unwrap_vararg_num((jl_vararg_t *)la);
// return 1 if we meet a vararg with Null length
if (!len) return 1;
// or a typevar not in the current env.
if (jl_is_typevar(len))
return lookup(e, (jl_tvar_t *)len) == NULL;
}
}
}
return 0;
}

static jl_value_t *intersect_var(jl_tvar_t *b, jl_value_t *a, jl_stenv_t *e, int8_t R, int param)
{
jl_varbinding_t *bb = lookup(e, b);
Expand Down Expand Up @@ -2334,6 +2375,11 @@ static jl_value_t *intersect_var(jl_tvar_t *b, jl_value_t *a, jl_stenv_t *e, int
}
}
bb->ub = ub;
// We get a imprecise Tuple here. Don't change `lb` and return the typevar directly.
if (has_free_vararg_length(ub, e) && !has_free_vararg_length(a, e)) {
JL_GC_POP();
return (jl_value_t*)b;
}
bb->lb = ub;
}
JL_GC_POP();
Expand Down Expand Up @@ -2443,6 +2489,10 @@ static jl_value_t *finish_unionall(jl_value_t *res JL_MAYBE_UNROOTED, jl_varbind
}
}

// vb is still unbounded.
if (vb->intvalued == 2 && !(varval && jl_is_long(varval)))
vb->intvalued = 1;

// TODO: this can prevent us from matching typevar identities later
if (!varval && (vb->lb != vb->var->lb || vb->ub != vb->var->ub))
newvar = jl_new_typevar(vb->var->name, vb->lb, vb->ub);
Expand Down Expand Up @@ -2635,7 +2685,7 @@ static jl_value_t *intersect_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_
e->vars->limited = 1;
}
else if (res != jl_bottom_type) {
if (vb.concrete || vb.occurs_inv>1 || u->var->lb != jl_bottom_type || (vb.occurs_inv && vb.occurs_cov)) {
if (vb.concrete || vb.occurs_inv>1 || vb.intvalued > 1 || u->var->lb != jl_bottom_type || (vb.occurs_inv && vb.occurs_cov)) {
restore_env(e, NULL, &se);
vb.occurs_cov = vb.occurs_inv = 0;
vb.constraintkind = vb.concrete ? 1 : 2;
Expand Down Expand Up @@ -2670,7 +2720,7 @@ static int intersect_vararg_length(jl_value_t *v, ssize_t n, jl_stenv_t *e, int8
}

static jl_value_t *intersect_invariant(jl_value_t *x, jl_value_t *y, jl_stenv_t *e);
static jl_value_t *intersect_varargs(jl_vararg_t *vmx, jl_vararg_t *vmy, jl_stenv_t *e, int param)
static jl_value_t *intersect_varargs(jl_vararg_t *vmx, jl_vararg_t *vmy, ssize_t offset, jl_stenv_t *e, int param)
{
// Vararg: covariant in first parameter, invariant in second
jl_value_t *xp1=jl_unwrap_vararg(vmx), *xp2=jl_unwrap_vararg_num(vmx),
Expand All @@ -2688,19 +2738,24 @@ static jl_value_t *intersect_varargs(jl_vararg_t *vmx, jl_vararg_t *vmy, jl_sten
JL_GC_POP();
return ii;
}
jl_varbinding_t *xb = NULL, *yb = NULL;
if (xp2 && jl_is_typevar(xp2)) {
jl_varbinding_t *xb = lookup(e, (jl_tvar_t*)xp2);
if (xb) xb->intvalued = 1;
if (!yp2) {
i2 = bound_var_below((jl_tvar_t*)xp2, xb, e);
xb = lookup(e, (jl_tvar_t*)xp2);
if (xb) {
if (xb->intvalued == 0) xb->intvalued = 1;
xb->offset = offset;
}
if (!yp2)
i2 = bound_var_below((jl_tvar_t*)xp2, xb, e);
}
if (yp2 && jl_is_typevar(yp2)) {
jl_varbinding_t *yb = lookup(e, (jl_tvar_t*)yp2);
if (yb) yb->intvalued = 1;
if (!xp2) {
i2 = bound_var_below((jl_tvar_t*)yp2, yb, e);
yb = lookup(e, (jl_tvar_t*)yp2);
if (yb) {
if (yb->intvalued == 0) yb->intvalued = 1;
yb->offset = -offset;
}
if (!xp2)
i2 = bound_var_below((jl_tvar_t*)yp2, yb, e);
}
if (xp2 && yp2) {
// Vararg{T,N} <: Vararg{T2,N2}; equate N and N2
Expand All @@ -2711,6 +2766,8 @@ static jl_value_t *intersect_varargs(jl_vararg_t *vmx, jl_vararg_t *vmy, jl_sten
i2 = jl_bottom_type;
}
}
if (xb) xb->offset = 0;
if (yb) yb->offset = 0;
ii = i2 == jl_bottom_type ? (jl_value_t*)jl_bottom_type : (jl_value_t*)jl_wrap_vararg(ii, i2);
JL_GC_POP();
return ii;
Expand Down Expand Up @@ -2749,28 +2806,14 @@ static jl_value_t *intersect_tuple(jl_datatype_t *xd, jl_datatype_t *yd, jl_sten
res = (jl_value_t*)jl_apply_tuple_type_v(jl_svec_data(params), i);
break;
}
jl_varbinding_t *xb=NULL, *yb=NULL;
jl_value_t *ii = NULL;
if (vx && vy) {
// {A^n...,Vararg{T,N}} ∩ {Vararg{S,M}} = {(A∩S)^n...,Vararg{T∩S,N}} plus N = M-n
jl_value_t *xlen = jl_unwrap_vararg_num(xi);
if (xlen && jl_is_typevar(xlen)) {
xb = lookup(e, (jl_tvar_t*)xlen);
if (xb)
xb->offset = ly-lx;
}
jl_value_t *ylen = jl_unwrap_vararg_num(yi);
if (ylen && jl_is_typevar(ylen)) {
yb = lookup(e, (jl_tvar_t*)ylen);
if (yb)
yb->offset = lx-ly;
}
if (vx && vy)
ii = intersect_varargs((jl_vararg_t*)xi,
(jl_vararg_t*)yi,
ly - lx, // xi's offset: {A^n...,Vararg{T,N}} ∩ {Vararg{S,M}}
// {(A∩S)^n...,Vararg{T∩S,N}} plus N = M-n
e, param);
if (xb) xb->offset = 0;
if (yb) yb->offset = 0;
} else {
else {
if (vx)
xi = jl_unwrap_vararg(xi);
if (vy)
Expand All @@ -2779,6 +2822,13 @@ static jl_value_t *intersect_tuple(jl_datatype_t *xd, jl_datatype_t *yd, jl_sten
}
if (ii == jl_bottom_type) {
if (vx && vy) {
jl_varbinding_t *xb=NULL, *yb=NULL;
jl_value_t *xlen = jl_unwrap_vararg_num(xi);
if (xlen && jl_is_typevar(xlen))
xb = lookup(e, (jl_tvar_t*)xlen);
jl_value_t *ylen = jl_unwrap_vararg_num(yi);
if (ylen && jl_is_typevar(ylen))
yb = lookup(e, (jl_tvar_t*)ylen);
int len = i > j ? i : j;
if ((xb && jl_is_long(xb->lb) && lx-1+jl_unbox_long(xb->lb) != len) ||
(yb && jl_is_long(yb->lb) && ly-1+jl_unbox_long(yb->lb) != len)) {
Expand Down Expand Up @@ -2877,6 +2927,11 @@ static jl_value_t *intersect_invariant(jl_value_t *x, jl_value_t *y, jl_stenv_t
jl_value_t *ii = intersect(x, y, e, 2);
e->invdepth--;
e->Rinvdepth--;
// Skip the following subtype check if `ii` was returned from `set_vat_to_const`.
// As `var_gt`/`var_lt` might not handle `Vararg` length offset correctly.
// TODO: fix this on subtype side and remove this branch.
if (jl_is_long(ii) && ((jl_is_typevar(x) && jl_is_long(y)) || (jl_is_typevar(y) && jl_is_long(x))))
return ii;
if (jl_is_typevar(x) && jl_is_typevar(y) && (jl_is_typevar(ii) || !jl_is_type(ii)))
return ii;
if (ii == jl_bottom_type) {
Expand All @@ -2894,13 +2949,10 @@ static jl_value_t *intersect_invariant(jl_value_t *x, jl_value_t *y, jl_stenv_t
jl_savedenv_t se;
JL_GC_PUSH2(&ii, &root);
save_env(e, &root, &se);
if (!subtype_in_env_existential(x, y, e, 0, e->invdepth)) {
if (!subtype_in_env_existential(x, y, e, 0, e->invdepth))
ii = NULL;
else if (!subtype_in_env_existential(y, x, e, 0, e->invdepth))
ii = NULL;
}
else {
if (!subtype_in_env_existential(y, x, e, 0, e->invdepth))
ii = NULL;
}
restore_env(e, root, &se);
free_env(&se);
JL_GC_POP();
Expand Down Expand Up @@ -3035,19 +3087,24 @@ static jl_value_t *intersect(jl_value_t *x, jl_value_t *y, jl_stenv_t *e, int pa
lb = ylb;
else
lb = simple_join(xlb, ylb);
if (yy) {
if (yy && yy->offset == 0) {
yy->lb = lb;
if (!reachable_var(ub, (jl_tvar_t*)y, e))
yy->ub = ub;
assert(yy->ub != y);
assert(yy->lb != y);
}
if (xx && !reachable_var(y, (jl_tvar_t*)x, e)) {
if (xx && xx->offset == 0 && !reachable_var(y, (jl_tvar_t*)x, e)) {
xx->lb = y;
xx->ub = y;
assert(xx->ub != x);
}
JL_GC_POP();
// Here we always return the shorter `Vararg`'s length.
if ((xx && xx->offset < 0) || (yy && yy->offset > 0)) {
if (yy) yy->intvalued = 2;
return x;
}
return y;
}
record_var_occurrence(xx, e, param);
Expand Down
Loading