Skip to content

Commit

Permalink
Fix some vararg-related subtyping issues (#31698)
Browse files Browse the repository at this point in the history
* Fix jl_obvious_subtype with INT vararg constraint

* Fix a vararg-related non-transitivity in subtyping

* Fix another vararg subtype issue

* Take advantage of their being at most one UnionAll wrapped around a Vararg

Upon construction, we normalize `Vararg{T, N} where {T,N}` to `Vararg{T where T, N} where N`,
thus there can be at most one UnionAll wrapper around a Vararg. In subtyping we were already
assuming that there can be at most two such wrappers, so simply adjust that and add an
appropriate assertion to make sure we catch any cases where this ever goes wrong.

* Rewrite subtype_tuple to fix extra cases

* Put back the case for naked varargs

* Update test/subtype.jl

Co-Authored-By: Keno <keno@alumni.harvard.edu>

* Add test for #31805

* Fix style review comments

* Rename variable

* In person review changes

* Fix bug

* Handle integer bounds on left arguments in the environment

In subtyping proper, variables introduced on the left (i.e. forall
variables) don't have any equality constraints, because we have no
syntax for creating them. However, intersection does sometimes create
such environments, so we need to handle it in subtyping.

(Cherry-picked from 4a38e79)
  • Loading branch information
Keno committed Jul 20, 2019
1 parent 65d9ffa commit 3759bae
Show file tree
Hide file tree
Showing 5 changed files with 419 additions and 143 deletions.
11 changes: 7 additions & 4 deletions base/compiler/typelimits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,13 @@ function _limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVec
return t # easy case
elseif isa(t, DataType) && isempty(t.parameters)
return t # fast path: unparameterized are always simple
elseif isa(unwrap_unionall(t), DataType) && isa(c, Type) && c !== Union{} && c <: t
return t # t is already wider than the comparison in the type lattice
elseif is_derived_type_from_any(unwrap_unionall(t), sources, depth)
return t # t isn't something new
else
ut = unwrap_unionall(t)
if isa(ut, DataType) && ut.name !== _va_typename && isa(c, Type) && c !== Union{} && c <: t
return t # t is already wider than the comparison in the type lattice
elseif is_derived_type_from_any(ut, sources, depth)
return t # t isn't something new
end
end
# peel off (and ignore) wrappers - they contribute no useful information, so we don't need to consider their size
# first attempt to turn `c` into a type that contributes meaningful information
Expand Down
10 changes: 10 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1101,10 +1101,19 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
continue;
if (jl_is_datatype(pi))
continue;
if (jl_is_vararg_type(pi)) {
pi = jl_unwrap_vararg(pi);
if (jl_has_free_typevars(pi))
continue;
}
// normalize types equal to wrappers (prepare for wrapper_id)
jl_value_t *tw = extract_wrapper(pi);
if (tw && tw != pi && (tn != jl_type_typename || jl_typeof(pi) == jl_typeof(tw)) &&
jl_types_equal(pi, tw)) {
if (jl_is_vararg_type(iparams[i])) {
tw = jl_wrap_vararg(tw, jl_tparam1(jl_unwrap_unionall(iparams[i])));
tw = jl_rewrap_unionall(tw, iparams[i]);
}
iparams[i] = tw;
if (p) jl_gc_wb(p, tw);
}
Expand Down Expand Up @@ -1157,6 +1166,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
}
int did_normalize = 0;
jl_value_t *last2 = normalize_vararg(last);
assert(!jl_is_unionall(last2) || !jl_is_unionall(((jl_unionall_t*)last2)->body));
if (last2 != last) {
last = last2;
did_normalize = 1;
Expand Down
8 changes: 8 additions & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,14 @@ STATIC_INLINE jl_value_t *jl_unwrap_vararg(jl_value_t *v) JL_NOTSAFEPOINT
return jl_tparam0(jl_unwrap_unionall(v));
}

STATIC_INLINE size_t jl_vararg_length(jl_value_t *v) JL_NOTSAFEPOINT
{
assert(jl_is_vararg_type(v));
jl_value_t *len = jl_tparam1(jl_unwrap_unionall(v));
assert(jl_is_long(len));
return jl_unbox_long(len);
}

STATIC_INLINE jl_vararg_kind_t jl_vararg_kind(jl_value_t *v) JL_NOTSAFEPOINT
{
if (!jl_is_vararg_type(v))
Expand Down
Loading

0 comments on commit 3759bae

Please sign in to comment.