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 May 9, 2019
1 parent decb802 commit 9e4e80c
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 19 deletions.
16 changes: 12 additions & 4 deletions base/essentials.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,18 @@ convert(::Type{T}, x::T) where {T<:AtLeast1} = x
convert(::Type{T}, x::AtLeast1) where {T<:AtLeast1} =
(convert(tuple_type_head(T), x[1]), convert(tuple_type_tail(T), tail(x))...)

# converting to Vararg tuple types
convert(::Type{Tuple{Vararg{V}}}, x::Tuple{Vararg{V}}) where {V} = x
convert(T::Type{Tuple{Vararg{V}}}, x::Tuple) where {V} =
(convert(tuple_type_head(T), x[1]), convert(T, tail(x))...)
# converting to other tuple types, including Vararg tuple types
_bad_tuple_conversion(T, x) = (@_noinline_meta; throw(MethodError(convert, (T, x))))
function convert(::Type{T}, x::AtLeast1) where {T<:Tuple}
if x isa T
return x
end
y = (convert(tuple_type_head(T), x[1]), convert(tuple_type_tail(T), tail(x))...)
if !(y isa T)
_bad_tuple_conversion(T, x)
end
return y
end

# used for splatting in `new`
convert_prefix(::Type{Tuple{}}, x::Tuple) = x
Expand Down
89 changes: 81 additions & 8 deletions src/subtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ typedef struct jl_varbinding_t {
int8_t occurs_inv; // occurs in invariant position
int8_t occurs_cov; // # of occurrences in covariant position
int8_t concrete; // 1 if another variable has a constraint forcing this one to be concrete
int8_t upper_bounded; // var upper bound has been constrained
// in covariant position, we need to try constraining a variable in different ways:
// 0 - unconstrained
// 1 - less than
Expand Down Expand Up @@ -145,7 +146,7 @@ static void save_env(jl_stenv_t *e, jl_value_t **root, jl_savedenv_t *se)
v = v->prev;
}
*root = (jl_value_t*)jl_alloc_svec(len*3);
se->buf = (int8_t*)(len ? malloc(len*2) : NULL);
se->buf = (int8_t*)(len ? malloc(len*3) : NULL);
#ifdef __clang_analyzer__
if (len)
memset(se->buf, 0, len*2);
Expand All @@ -157,6 +158,7 @@ static void save_env(jl_stenv_t *e, jl_value_t **root, jl_savedenv_t *se)
jl_svecset(*root, i++, (jl_value_t*)v->innervars);
se->buf[j++] = v->occurs_inv;
se->buf[j++] = v->occurs_cov;
se->buf[j++] = v->upper_bounded;
v = v->prev;
}
se->rdepth = e->Runions.depth;
Expand All @@ -176,13 +178,46 @@ static void restore_env(jl_stenv_t *e, jl_value_t *root, jl_savedenv_t *se) JL_N
assert(se->buf);
v->occurs_inv = se->buf[j++];
v->occurs_cov = se->buf[j++];
v->upper_bounded = se->buf[j++];
v = v->prev;
}
e->Runions.depth = se->rdepth;
if (e->envout && e->envidx < e->envsz)
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++];
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++;
j++;
v = v->prev;
}
}

// type utilities

// quickly test that two types are identical
Expand Down Expand Up @@ -528,8 +563,10 @@ static int var_lt(jl_tvar_t *b, jl_value_t *a, jl_stenv_t *e, int param)
record_var_occurrence(bb, e, param);
if (!bb->right) // check ∀b . b<:a
return subtype_left_var(bb->ub, a, e);
if (bb->ub == a)
if (bb->ub == a) {
bb->upper_bounded = 1;
return 1;
}
if (!((bb->lb == jl_bottom_type && !jl_is_type(a) && !jl_is_typevar(a)) || subtype_ccheck(bb->lb, a, e)))
return 0;
// for this to work we need to compute issub(left,right) before issub(right,left),
Expand All @@ -542,6 +579,7 @@ static int var_lt(jl_tvar_t *b, jl_value_t *a, jl_stenv_t *e, int param)
else {
bb->ub = simple_meet(bb->ub, a);
}
bb->upper_bounded = 1;
assert(bb->ub != (jl_value_t*)b);
if (jl_is_typevar(a)) {
jl_varbinding_t *aa = lookup(e, (jl_tvar_t*)a);
Expand Down Expand Up @@ -657,7 +695,7 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8
}
btemp = btemp->prev;
}
jl_varbinding_t vb = { u->var, u->var->lb, u->var->ub, R, NULL, 0, 0, 0, 0, e->invdepth, 0, NULL, e->vars };
jl_varbinding_t vb = { u->var, u->var->lb, u->var->ub, R, NULL, 0, 0, 0, 0, 0, e->invdepth, 0, NULL, e->vars };
JL_GC_PUSH4(&u, &vb.lb, &vb.ub, &vb.innervars);
e->vars = &vb;
int ans;
Expand All @@ -671,7 +709,9 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8
// fill variable values into `envout` up to `envsz`
if (e->envidx < e->envsz) {
jl_value_t *val;
if (!vb.occurs_inv && vb.lb != jl_bottom_type)
if (vb.lb == vb.ub && vb.upper_bounded)
val = vb.lb;
else if (!vb.occurs_inv && vb.lb != jl_bottom_type)
val = is_leaf_bound(vb.lb) ? vb.lb : (jl_value_t*)jl_new_typevar(u->var->name, jl_bottom_type, vb.lb);
else if (vb.lb == vb.ub)
val = vb.lb;
Expand Down Expand Up @@ -720,7 +760,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 +771,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 +862,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 @@ -1069,6 +1120,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 @@ -1098,11 +1155,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 Expand Up @@ -1966,7 +2039,7 @@ static jl_value_t *intersect_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_
{
jl_value_t *res=NULL, *res2=NULL, *save=NULL, *save2=NULL;
jl_savedenv_t se, se2;
jl_varbinding_t vb = { u->var, u->var->lb, u->var->ub, R, NULL, 0, 0, 0, 0, e->invdepth, 0, NULL, e->vars };
jl_varbinding_t vb = { u->var, u->var->lb, u->var->ub, R, NULL, 0, 0, 0, 0, 0, e->invdepth, 0, NULL, e->vars };
JL_GC_PUSH6(&res, &save2, &vb.lb, &vb.ub, &save, &vb.innervars);
save_env(e, &save, &se);
res = intersect_unionall_(t, u, e, R, param, &vb);
Expand Down
4 changes: 0 additions & 4 deletions test/ambiguous.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,6 @@ end
@test_broken need_to_handle_undef_sparam == Set()
pop!(need_to_handle_undef_sparam, which(Core.Compiler._cat, Tuple{Any, AbstractArray}))
pop!(need_to_handle_undef_sparam, first(methods(Core.Compiler.same_names)))
pop!(need_to_handle_undef_sparam, which(Core.Compiler.convert, Tuple{Type{Tuple{Vararg{Int}}}, Tuple{}}))
pop!(need_to_handle_undef_sparam, which(Core.Compiler.convert, Tuple{Type{Tuple{Vararg{Int}}}, Tuple{Int8}}))
@test need_to_handle_undef_sparam == Set()
end
let need_to_handle_undef_sparam =
Expand All @@ -292,8 +290,6 @@ end
pop!(need_to_handle_undef_sparam, which(Base.oneunit, Tuple{Type{Union{Missing, T}} where T}))
pop!(need_to_handle_undef_sparam, which(Base.convert, (Type{Union{Some{T}, Nothing}} where T, Some)))
pop!(need_to_handle_undef_sparam, which(Base.convert, (Type{Union{T, Nothing}} where T, Some)))
pop!(need_to_handle_undef_sparam, which(Base.convert, Tuple{Type{Tuple{Vararg{Int}}}, Tuple{}}))
pop!(need_to_handle_undef_sparam, which(Base.convert, Tuple{Type{Tuple{Vararg{Int}}}, Tuple{Int8}}))
pop!(need_to_handle_undef_sparam, which(Base.convert, Tuple{Type{Union{Nothing,T}},Union{Nothing,T}} where T))
pop!(need_to_handle_undef_sparam, which(Base.convert, Tuple{Type{Union{Missing,T}},Union{Missing,T}} where T))
pop!(need_to_handle_undef_sparam, which(Base.convert, Tuple{Type{Union{Missing,Nothing,T}},Union{Missing,Nothing,T}} where T))
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
14 changes: 11 additions & 3 deletions test/tuple.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ 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{Real}}, (1.0, 2, 0x3)) === (1.0, 2, 0x3)
@test convert(Tuple{Vararg{Integer}}, (1.0, 2, 0x3)) === (1, 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 +52,15 @@ 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))) convert(NTuple, (1, 1.0))
let T = Tuple{Vararg{T}} where T<:Integer, v = (1.0, 2, 0x3)
@test_throws MethodError(convert, (T, (2, 0x3))) convert(T, v)
end
let T = Tuple{T, Vararg{T}} where T<:Integer, v = (1.0, 2, 0x3)
@test_throws MethodError(convert, (Tuple{Vararg{T}} where T<:Integer, (2, 0x3))) convert(T, v)
end

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

0 comments on commit 9e4e80c

Please sign in to comment.