Skip to content

Commit

Permalink
fix some bugs in diagonal subtyping
Browse files Browse the repository at this point in the history 
fixes #31824, fixes #24166
  • Loading branch information
@JeffBezanson
JeffBezanson committed Apr 25, 2019
1 parent 6421def commit 9acd05e
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 6 deletions.
69 changes: 66 additions & 3 deletions src/subtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,36 @@ static void restore_env(jl_stenv_t *e, jl_value_t *root, jl_savedenv_t *se) JL_N
memset(&e->envout[e->envidx], 0, (e->envsz - e->envidx)*sizeof(void*));
}

// restore just occurs_inv and occurs_cov from `se` back to `e`
static void restore_var_counts(jl_stenv_t *e, jl_savedenv_t *se) JL_NOTSAFEPOINT
{
jl_varbinding_t *v = e->vars;
int j = 0;
while (v != NULL) {
assert(se->buf);
v->occurs_inv = se->buf[j++];
v->occurs_cov = se->buf[j++];
v = v->prev;
}
}

// compute the maximum of the occurence counts in `e` and `se`, storing them in `se`
static void max_var_counts(jl_stenv_t *e, jl_savedenv_t *se) JL_NOTSAFEPOINT
{
jl_varbinding_t *v = e->vars;
int j = 0;
while (v != NULL) {
assert(se->buf);
if (v->occurs_inv > se->buf[j])
se->buf[j] = v->occurs_inv;
j++;
if (v->occurs_cov > se->buf[j])
se->buf[j] = v->occurs_cov;
j++;
v = v->prev;
}
}

// type utilities

// quickly test that two types are identical
Expand Down Expand Up @@ -720,7 +750,7 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8
else if (!is_leaf_bound(vb.lb)) {
ans = 0;
}
if (ans) {
if (ans && vb.lb != vb.ub) {
// if we occur as another var's lower bound, record the fact that we
// were concrete so that subtype can return true for that var.
/*
Expand All @@ -731,6 +761,17 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8
btemp = btemp->prev;
}
*/
// a diagonal var cannot be >: another diagonal var at a different invariant depth, e.g.
// Ref{Tuple{T,T} where T} !<: Ref{Tuple{T,T}} where T
btemp = vb.prev;
while (btemp != NULL) {
if (btemp->lb == (jl_value_t*)u->var && btemp->depth0 != vb.depth0 &&
(btemp->concrete || (btemp->occurs_cov > 1 && btemp->occurs_inv == 0))) {
ans = 0;
break;
}
btemp = btemp->prev;
}
}
}

Expand Down Expand Up @@ -811,7 +852,7 @@ static int subtype_tuple(jl_datatype_t *xd, jl_datatype_t *yd, jl_stenv_t *e, in
else if ((vvy && ly > lx+1) || (!vvy && lx != ly)) {
return 0;
}
param = (param == 0 ? 1 : param);
param = 1;
jl_value_t *lastx=NULL, *lasty=NULL;
while (i < lx) {
jl_value_t *xi = jl_tparam(xd, i);
Expand Down Expand Up @@ -1067,6 +1108,12 @@ static int forall_exists_equal(jl_value_t *x, jl_value_t *y, jl_stenv_t *e)
{
if (obviously_egal(x, y)) return 1;

jl_savedenv_t se; // original env
jl_savedenv_t me; // for accumulating maximum var counts
jl_value_t *saved=NULL;
save_env(e, &saved, &se);
save_env(e, &saved, &me);

jl_unionstate_t oldLunions = e->Lunions;
memset(e->Lunions.stack, 0, sizeof(e->Lunions.stack));
int sub;
Expand Down Expand Up @@ -1096,11 +1143,27 @@ static int forall_exists_equal(jl_value_t *x, jl_value_t *y, jl_stenv_t *e)
statestack_set(&e->Lunions, i, 0);
lastset = set - 1;
statestack_set(&e->Lunions, lastset, 1);
// take the maximum of var counts over all choices, to identify
// diagonal variables better.
max_var_counts(e, &me);
restore_var_counts(e, &se);
}
}

e->Lunions = oldLunions;
return sub && subtype(y, x, e, 0);
if (sub) {
// avoid double-counting variables when we check subtype in both directions.
// e.g. in `Ref{Tuple{T}}` the `T` occurs once even though we recursively
// call `subtype` on it twice.
max_var_counts(e, &me);
restore_var_counts(e, &se);
sub = subtype(y, x, e, 2);
max_var_counts(e, &me);
restore_var_counts(e, &me);
}
free(se.buf);
free(me.buf);
return sub;
}

static int exists_subtype(jl_value_t *x, jl_value_t *y, jl_stenv_t *e, jl_value_t *saved, jl_savedenv_t *se, int param)
Expand Down
8 changes: 8 additions & 0 deletions test/subtype.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ function test_diagonal()
@test issub(Tuple{Tuple{T, T}} where T>:Int, Tuple{Tuple{T, T} where T>:Int})
@test issub(Vector{Tuple{T, T} where Number<:T<:Number},
Vector{Tuple{Number, Number}})

@test !issub(Type{Tuple{T,Any} where T}, Type{Tuple{T,T}} where T)
@test !issub(Type{Tuple{T,Any,T} where T}, Type{Tuple{T,T,T}} where T)
@test issub(Type{Tuple{T} where T}, Type{Tuple{T}} where T)
@test !issub(Type{Tuple{T,T} where T}, Type{Tuple{T,T}} where T)
@test !issub(Type{Tuple{T,T,T} where T}, Type{Tuple{T,T,T}} where T)
@test isequal_type(Ref{Tuple{T, T} where Int<:T<:Int},
Ref{Tuple{S, S}} where Int<:S<:Int)
end

# level 3: UnionAll
Expand Down
8 changes: 5 additions & 3 deletions test/tuple.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,8 @@ end
@test convert(Tuple{Int, Int, Float64}, (1, 2, 3)) === (1, 2, 3.0)

@test convert(Tuple{Float64, Int, UInt8}, (1.0, 2, 0x3)) === (1.0, 2, 0x3)
@test convert(NTuple, (1.0, 2, 0x3)) === (1.0, 2, 0x3)
@test convert(Tuple{Vararg{Int}}, (1.0, 2, 0x3)) === (1, 2, 3)
@test convert(Tuple{Int, Vararg{Int}}, (1.0, 2, 0x3)) === (1, 2, 3)
@test convert(Tuple{Vararg{T}} where T<:Integer, (1.0, 2, 0x3)) === (1, 2, 0x3)
@test convert(Tuple{T, Vararg{T}} where T<:Integer, (1.0, 2, 0x3)) === (1, 2, 0x3)
@test convert(NTuple{3, Int}, (1.0, 2, 0x3)) === (1, 2, 3)
@test convert(Tuple{Int, Int, Float64}, (1.0, 2, 0x3)) === (1, 2, 3.0)

Expand All @@ -53,6 +50,11 @@ end
@test_throws MethodError convert(Tuple{Int, Int, Int}, (1, 2))
# issue #26589
@test_throws MethodError convert(NTuple{4}, (1.0,2.0,3.0,4.0,5.0))
# issue #31824
# there is no generic way to convert an arbitrary tuple to a homogeneous tuple
@test_throws MethodError convert(NTuple, (1, 1.0))
@test_throws MethodError convert(Tuple{Vararg{T}} where T<:Integer, (1.0, 2, 0x3)) === (1, 2, 0x3)
@test_throws MethodError convert(Tuple{T, Vararg{T}} where T<:Integer, (1.0, 2, 0x3)) === (1, 2, 0x3)

# PR #15516
@test Tuple{Char,Char}("za") === ('z','a')
Expand Down

0 comments on commit 9acd05e

Please sign in to comment.