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.

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

Fix some overly optimistic fastpaths in subtyping #25796

Merged
merged 7 commits into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ jl_datatype_t *jl_new_uninitialized_datatype(void)
t->zeroinit = 0;
t->isinlinealloc = 0;
t->has_concrete_subtype = 1;
t->cached_by_hash = 0;
t->layout = NULL;
t->names = NULL;
t->types = NULL;
Expand Down
4 changes: 3 additions & 1 deletion src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ static void jl_serialize_datatype(jl_serializer_state *s, jl_datatype_t *dt) JL_
| (dt->isbitstype << 3)
| (dt->zeroinit << 4)
| (dt->isinlinealloc << 5)
| (dt->has_concrete_subtype << 6));
| (dt->has_concrete_subtype << 6)
| (dt->cached_by_hash << 7));
if (!dt->abstract) {
write_uint16(s->s, dt->ninitialized);
}
Expand Down Expand Up @@ -1174,6 +1175,7 @@ static jl_value_t *jl_deserialize_datatype(jl_serializer_state *s, int pos, jl_v
dt->zeroinit = (memflags >> 4) & 1;
dt->isinlinealloc = (memflags >> 5) & 1;
dt->has_concrete_subtype = (memflags >> 6) & 1;
dt->cached_by_hash = (memflags >> 7) & 1;
dt->types = NULL;
dt->parameters = NULL;
dt->name = NULL;
Expand Down
27 changes: 22 additions & 5 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,8 @@ static int typekey_eq(jl_datatype_t *tt, jl_value_t **key, size_t n)
// require exact same Type{T}. see e.g. issue #22842
if (jl_is_type_type(tj) || jl_is_type_type(kj))
return 0;
if (jl_is_concrete_type(tj) || jl_is_concrete_type(kj))
if ((jl_is_concrete_type(tj) || jl_is_concrete_type(kj)) &&
jl_type_equality_is_identity(tj, kj))
return 0;
if (!jl_types_equal(tj, kj))
return 0;
Expand Down Expand Up @@ -860,6 +861,17 @@ jl_datatype_t *jl_lookup_cache_type_(jl_datatype_t *type)
return (jl_datatype_t*)lookup_type(type->name, key, n);
}

int jl_type_equality_is_identity(jl_value_t *t1, jl_value_t *t2)
{
if (t1 == t2)
return 1;
if (!jl_is_datatype(t1) || !jl_is_datatype(t2))
return 0;
jl_datatype_t *dt1 = (jl_datatype_t *) t1;
jl_datatype_t *dt2 = (jl_datatype_t *) t2;

return dt1->cached_by_hash == dt2->cached_by_hash;
}

// type instantiation

Expand Down Expand Up @@ -1155,6 +1167,7 @@ void jl_precompute_memoized_dt(jl_datatype_t *dt, int cacheable)
if (dt->name == jl_type_typename)
cacheable = 0; // the cache for Type ignores parameter normalization, so it can't be used as a regular hash
dt->hash = typekey_hash(dt->name, jl_svec_data(dt->parameters), l, cacheable);
dt->cached_by_hash = cacheable ? (typekey_hash(dt->name, jl_svec_data(dt->parameters), l, 0) != 0) : (dt->hash != 0);
}

static void check_datatype_parameters(jl_typename_t *tn, jl_value_t **params, size_t np)
Expand Down Expand Up @@ -1880,7 +1893,7 @@ void jl_init_types(void) JL_GC_DISABLED
jl_datatype_type->name->wrapper = (jl_value_t*)jl_datatype_type;
jl_datatype_type->super = (jl_datatype_t*)jl_type_type;
jl_datatype_type->parameters = jl_emptysvec;
jl_datatype_type->name->names = jl_perm_symsvec(19,
jl_datatype_type->name->names = jl_perm_symsvec(20,
"name",
"super",
"parameters",
Expand All @@ -1899,8 +1912,9 @@ void jl_init_types(void) JL_GC_DISABLED
"isbitstype",
"zeroinit",
"isinlinealloc",
"has_concrete_subtype");
jl_datatype_type->types = jl_svec(19,
"has_concrete_subtype",
"cached_by_hash");
jl_datatype_type->types = jl_svec(20,
jl_typename_type,
jl_datatype_type,
jl_simplevector_type,
Expand All @@ -1909,7 +1923,7 @@ void jl_init_types(void) JL_GC_DISABLED
jl_any_type, jl_any_type, jl_any_type, jl_any_type, // properties
jl_any_type, jl_any_type, jl_any_type, jl_any_type,
jl_any_type, jl_any_type, jl_any_type, jl_any_type,
jl_any_type);
jl_any_type, jl_any_type);
jl_datatype_type->abstract = 0;
// NOTE: types are not actually mutable, but we want to ensure they are heap-allocated with stable addresses
jl_datatype_type->mutabl = 1;
Expand Down Expand Up @@ -2019,10 +2033,12 @@ void jl_init_types(void) JL_GC_DISABLED
jl_anytuple_type->isconcretetype = 0;
jl_anytuple_type->layout = NULL;
jl_anytuple_type->size = 0;
jl_anytuple_type->cached_by_hash = 0;

jl_tvar_t *tttvar = tvar("T");
((jl_datatype_t*)jl_type_type)->parameters = jl_svec(1, tttvar);
((jl_datatype_t*)jl_type_type)->hasfreetypevars = 1;
((jl_datatype_t*)jl_type_type)->cached_by_hash = 0;
jl_type_typename->wrapper = jl_new_struct(jl_unionall_type, tttvar, (jl_value_t*)jl_type_type);
jl_type_type = (jl_unionall_t*)jl_type_typename->wrapper;

Expand Down Expand Up @@ -2482,6 +2498,7 @@ void jl_init_types(void) JL_GC_DISABLED
jl_svecset(jl_datatype_type->types, 16, jl_bool_type);
jl_svecset(jl_datatype_type->types, 17, jl_bool_type);
jl_svecset(jl_datatype_type->types, 18, jl_bool_type);
jl_svecset(jl_datatype_type->types, 19, jl_bool_type);
jl_svecset(jl_typename_type->types, 1, jl_module_type);
jl_svecset(jl_typename_type->types, 6, jl_long_type);
jl_svecset(jl_typename_type->types, 3, jl_type_type);
Expand Down
2 changes: 2 additions & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ typedef struct _jl_datatype_t {
uint8_t zeroinit; // if one or more fields requires zero-initialization
uint8_t isinlinealloc; // if this is allocated inline
uint8_t has_concrete_subtype; // If clear, no value will have this datatype
uint8_t cached_by_hash; // stored in hash-based set cache (instead of linear cache)
} jl_datatype_t;

typedef struct {
Expand Down Expand Up @@ -1216,6 +1217,7 @@ JL_DLLEXPORT int jl_egal(jl_value_t *a JL_MAYBE_UNROOTED, jl_value_t *b JL_MAYBE
JL_DLLEXPORT uintptr_t jl_object_id(jl_value_t *v) JL_NOTSAFEPOINT;

// type predicates and basic operations
JL_DLLEXPORT int jl_type_equality_is_identity(jl_value_t *t1, jl_value_t *t2) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_has_free_typevars(jl_value_t *v) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_has_typevar(jl_value_t *t, jl_tvar_t *v) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_has_typevar_from_unionall(jl_value_t *t, jl_unionall_t *ua);
Expand Down
17 changes: 8 additions & 9 deletions src/subtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ static int obviously_unequal(jl_value_t *a, jl_value_t *b)
if (ad->name != bd->name)
return 1;
int istuple = (ad->name == jl_tuple_typename);
if (jl_is_concrete_type(a) || jl_is_concrete_type(b)) {
if ((jl_is_concrete_type(a) || jl_is_concrete_type(b)) &&
jl_type_equality_is_identity(a, b)) {
if (!istuple && ad->name != jl_type_typename) // HACK: can't properly normalize Tuple{Float64} == Tuple{<:Float64} like types or Type{T} types
return 1;
}
Expand Down Expand Up @@ -313,13 +314,11 @@ static int obviously_disjoint(jl_value_t *a, jl_value_t *b, int specificity)
return 0;
if (specificity && a == (jl_value_t*)jl_typeofbottom_type)
return 0;
// TODO: this would be a nice fast-path to have, unfortuanately,
// datatype allocation fails to correctly hash-cons them
// and the subtyping tests include tests for this case
//if (jl_is_concrete_type(a) && jl_is_concrete_type(b) &&
// (((jl_datatype_t*)a)->name != jl_tuple_typename ||
// ((jl_datatype_t*)b)->name != jl_tuple_typename))
// return 1;
if (jl_is_concrete_type(a) && jl_is_concrete_type(b) &&
jl_type_equality_is_identity(a, b) &&
(((jl_datatype_t*)a)->name != jl_tuple_typename ||
((jl_datatype_t*)b)->name != jl_tuple_typename))
return 1;
if (jl_is_unionall(a)) a = jl_unwrap_unionall(a);
if (jl_is_unionall(b)) b = jl_unwrap_unionall(b);
if (jl_is_datatype(a) && jl_is_datatype(b)) {
Expand Down Expand Up @@ -2115,7 +2114,7 @@ JL_DLLEXPORT int jl_isa(jl_value_t *x, jl_value_t *t)
return 0;
}
}
if (jl_is_concrete_type(t))
if (jl_is_concrete_type(t) && jl_type_equality_is_identity(jl_typeof(x), t))
return 0;
return jl_subtype(jl_typeof(x), t);
}
Expand Down
11 changes: 11 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7239,6 +7239,17 @@ struct AVL35416{K,V}
end
@test AVL35416(Node35416{AVL35416{Integer,AbstractString},Int,String}()) isa AVL35416{Integer,AbstractString}

# issue #31696
foo31696(x::Int8, y::Int8) = 1
foo31696(x::T, y::T) where {T <: Int8} = 2
@test length(methods(foo31696)) == 1
let T1 = Tuple{Int8}, T2 = Tuple{T} where T<:Int8, a = T1[(1,)], b = T2[(1,)]
b .= a
@test b[1] == (1,)
a .= b
@test a[1] == (1,)
end

# issue #36104
module M36104
struct T36104
Expand Down
27 changes: 15 additions & 12 deletions test/subtype.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,18 @@ let (t, e) = intersection_env(Tuple{Union{Int,Int8}}, Tuple{T} where T)
@test e[1] isa TypeVar
end

# issue #25430
@test Vector{Tuple{Any}}() isa Vector{Tuple{>:Int}}
@test Vector{Tuple{>:Int}}() isa Vector{Tuple{Any}}
@test Vector{Tuple{Any}} == Vector{Tuple{>:Int}}
@test Vector{Vector{Tuple{Any}}} == Vector{Vector{Tuple{>:Int}}}
f25430(t::Vector{Tuple{Any}}) = true
g25430(t::Vector{Tuple{>:Int}}) = true
@test f25430(Vector{Tuple{>:Int}}())
@test g25430(Vector{Tuple{Any}}())
@testintersect(Vector{Tuple{>:Int}}, Vector{Tuple{Any}}, Vector{Tuple{Any}})
@testintersect(Vector{Vector{Tuple{>:Int}}}, Vector{Vector{Tuple{Any}}}, Vector{Vector{Tuple{Any}}})

# issue #24521
g24521(::T, ::T) where {T} = T
@test_throws MethodError g24521(Tuple{Any}, Tuple{T} where T)
Expand Down Expand Up @@ -1627,18 +1639,9 @@ end
@testintersect(Tuple{Type{<:AbstractVector{T}}, Int} where T,
Tuple{Type{Vector}, Any},
Union{})
#@testintersect(Tuple{Type{<:AbstractVector{T}}, Int} where T,
# Tuple{Type{Vector{T} where Int<:T<:Int}, Any},
# Tuple{Type{Vector{Int}}, Int})
@test_broken isequal(_type_intersect(Tuple{Type{<:AbstractVector{T}}, Int} where T,
Tuple{Type{Vector{T} where Int<:T<:Int}, Any}),
Tuple{Type{Vector{Int}}, Int})
@test isequal_type(_type_intersect(Tuple{Type{<:AbstractVector{T}}, Int} where T,
Tuple{Type{Vector{T} where Int<:T<:Int}, Any}),
Tuple{Type{Vector{Int}}, Int})
@test isequal_type(_type_intersect(Tuple{Type{Vector{T} where Int<:T<:Int}, Any},
Tuple{Type{<:AbstractVector{T}}, Int} where T),
Tuple{Type{Vector{Int}}, Int})
@testintersect(Tuple{Type{<:AbstractVector{T}}, Int} where T,
Tuple{Type{Vector{T} where Int<:T<:Int}, Any},
Tuple{Type{Vector{Int}}, Int})
let X = LinearAlgebra.Symmetric{T, S} where S<:(AbstractArray{U, 2} where U<:T) where T,
Y = Union{LinearAlgebra.Hermitian{T, S} where S<:(AbstractArray{U, 2} where U<:T) where T,
LinearAlgebra.Symmetric{T, S} where S<:(AbstractArray{U, 2} where U<:T) where T}
Expand Down