Skip to content

Commit f7a9017

Browse files
creliercommit-bot@chromium.org
authored andcommitted
Reland "[VM/runtime] Fix type tests with LHS FutureOr (fixes flutter#38906)."
This is a reland of 3ef5270 Original change's description: > [VM/runtime] Fix type tests with LHS FutureOr (fixes flutter#38906). > > Fixes tests/language_2/subtyping_static/future_or_subtype_test > Also simplifies code making use of TypeAtNullSafe(). > > Change-Id: I045a030b2ece9d6980f7a9ba785948f0c10f697b > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135625 > Commit-Queue: Régis Crelier <regis@google.com> > Reviewed-by: Alexander Markov <alexmarkov@google.com> Change-Id: Ie3254f6aca3d47a00aead0ac34f3a6b9f301d8dc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136680 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Régis Crelier <regis@google.com>
1 parent 355d443 commit f7a9017

File tree

10 files changed

+124
-83
lines changed

10 files changed

+124
-83
lines changed

runtime/vm/compiler/aot/precompiler.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ void Precompiler::DoCompileAll() {
419419
Class& null_class = Class::Handle(Z);
420420
Function& null_function = Function::Handle(Z);
421421
Field& null_field = Field::Handle(Z);
422-
I->object_store()->set_future_class(null_class);
423422
I->object_store()->set_pragma_class(null_class);
424423
I->object_store()->set_pragma_name(null_field);
425424
I->object_store()->set_pragma_options(null_field);

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,8 +368,7 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
368368
// If instance is Smi, check directly.
369369
const Class& smi_class = Class::Handle(zone(), Smi::Class());
370370
if (Class::IsSubtypeOf(NNBDMode::kLegacyLib, smi_class,
371-
Object::null_type_arguments(), type_class,
372-
Object::null_type_arguments(), Heap::kOld)) {
371+
Object::null_type_arguments(), type, Heap::kOld)) {
373372
// Fast case for type = int/num/top-type.
374373
__ b(is_instance_lbl, EQ);
375374
} else {

runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,7 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
353353
// If instance is Smi, check directly.
354354
const Class& smi_class = Class::Handle(zone(), Smi::Class());
355355
if (Class::IsSubtypeOf(NNBDMode::kLegacyLib, smi_class,
356-
Object::null_type_arguments(), type_class,
357-
Object::null_type_arguments(), Heap::kOld)) {
356+
Object::null_type_arguments(), type, Heap::kOld)) {
358357
// Fast case for type = int/num/top-type.
359358
__ BranchIfSmi(kInstanceReg, is_instance_lbl);
360359
} else {

runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,7 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
344344
// If instance is Smi, check directly.
345345
const Class& smi_class = Class::Handle(zone(), Smi::Class());
346346
if (Class::IsSubtypeOf(NNBDMode::kLegacyLib, smi_class,
347-
Object::null_type_arguments(), type_class,
348-
Object::null_type_arguments(), Heap::kOld)) {
347+
Object::null_type_arguments(), type, Heap::kOld)) {
349348
// Fast case for type = int/num/top-type.
350349
__ j(ZERO, is_instance_lbl);
351350
} else {

runtime/vm/compiler/backend/flow_graph_compiler_x64.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,7 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
369369
// If instance is Smi, check directly.
370370
const Class& smi_class = Class::Handle(zone(), Smi::Class());
371371
if (Class::IsSubtypeOf(NNBDMode::kLegacyLib, smi_class,
372-
Object::null_type_arguments(), type_class,
373-
Object::null_type_arguments(), Heap::kOld)) {
372+
Object::null_type_arguments(), type, Heap::kOld)) {
374373
// Fast case for type = int/num/top-type.
375374
__ j(ZERO, is_instance_lbl);
376375
} else {

runtime/vm/compiler/call_specializer.cc

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,8 +1112,7 @@ RawBool* CallSpecializer::InstanceOfAsBool(
11121112
(unwrapped_type.IsLegacy() && unwrapped_type.IsNeverType());
11131113
} else {
11141114
is_subtype = Class::IsSubtypeOf(mode, cls, Object::null_type_arguments(),
1115-
type_class, Object::null_type_arguments(),
1116-
Heap::kOld);
1115+
type, Heap::kOld);
11171116
}
11181117
results->Add(cls.id());
11191118
results->Add(static_cast<intptr_t>(is_subtype));
@@ -1460,11 +1459,9 @@ bool CallSpecializer::SpecializeTestCidsForNumericTypes(
14601459
const ClassTable& class_table = *Isolate::Current()->class_table();
14611460
if ((*results)[0] != kSmiCid) {
14621461
const Class& smi_class = Class::Handle(class_table.At(kSmiCid));
1463-
const Class& type_class = Class::Handle(type.type_class());
1464-
// When testing '42 is type', the nullability of type is irrelevant.
1465-
const bool smi_is_subtype = Class::IsSubtypeOf(
1466-
NNBDMode::kLegacyLib, smi_class, Object::null_type_arguments(),
1467-
type_class, Object::null_type_arguments(), Heap::kOld);
1462+
const bool smi_is_subtype =
1463+
Class::IsSubtypeOf(NNBDMode::kLegacyLib, smi_class,
1464+
Object::null_type_arguments(), type, Heap::kOld);
14681465
results->Add((*results)[results->length() - 2]);
14691466
results->Add((*results)[results->length() - 2]);
14701467
for (intptr_t i = results->length() - 3; i > 1; --i) {

runtime/vm/dart_api_impl.cc

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,22 @@ class CheckFunctionTypesVisitor : public ObjectVisitor {
146146

147147
static RawInstance* GetListInstance(Zone* zone, const Object& obj) {
148148
if (obj.IsInstance()) {
149-
const Library& core_lib = Library::Handle(zone, Library::CoreLibrary());
150-
const Class& list_class =
151-
Class::Handle(zone, core_lib.LookupClass(Symbols::List()));
152-
ASSERT(!list_class.IsNull());
149+
ObjectStore* object_store = Isolate::Current()->object_store();
150+
Type& list_rare_type =
151+
Type::Handle(zone, object_store->non_nullable_list_rare_type());
152+
if (list_rare_type.IsNull()) {
153+
const Library& core_lib = Library::Handle(zone, Library::CoreLibrary());
154+
const Class& list_class =
155+
Class::Handle(zone, core_lib.LookupClass(Symbols::List()));
156+
ASSERT(!list_class.IsNull());
157+
list_rare_type ^= list_class.RareType();
158+
object_store->set_non_nullable_list_rare_type(list_rare_type);
159+
}
153160
const Instance& instance = Instance::Cast(obj);
154161
const Class& obj_class = Class::Handle(zone, obj.clazz());
155162
if (Class::IsSubtypeOf(NNBDMode::kLegacyLib, obj_class,
156-
Object::null_type_arguments(), list_class,
157-
Object::null_type_arguments(), Heap::kNew)) {
163+
Object::null_type_arguments(), list_rare_type,
164+
Heap::kNew)) {
158165
return instance.raw();
159166
}
160167
}
@@ -163,15 +170,22 @@ static RawInstance* GetListInstance(Zone* zone, const Object& obj) {
163170

164171
static RawInstance* GetMapInstance(Zone* zone, const Object& obj) {
165172
if (obj.IsInstance()) {
166-
const Library& core_lib = Library::Handle(zone, Library::CoreLibrary());
167-
const Class& map_class =
168-
Class::Handle(core_lib.LookupClass(Symbols::Map()));
169-
ASSERT(!map_class.IsNull());
173+
ObjectStore* object_store = Isolate::Current()->object_store();
174+
Type& map_rare_type =
175+
Type::Handle(zone, object_store->non_nullable_map_rare_type());
176+
if (map_rare_type.IsNull()) {
177+
const Library& core_lib = Library::Handle(zone, Library::CoreLibrary());
178+
const Class& map_class =
179+
Class::Handle(zone, core_lib.LookupClass(Symbols::Map()));
180+
ASSERT(!map_class.IsNull());
181+
map_rare_type ^= map_class.RareType();
182+
object_store->set_non_nullable_map_rare_type(map_rare_type);
183+
}
170184
const Instance& instance = Instance::Cast(obj);
171185
const Class& obj_class = Class::Handle(zone, obj.clazz());
172186
if (Class::IsSubtypeOf(NNBDMode::kLegacyLib, obj_class,
173-
Object::null_type_arguments(), map_class,
174-
Object::null_type_arguments(), Heap::kNew)) {
187+
Object::null_type_arguments(), map_rare_type,
188+
Heap::kNew)) {
175189
return instance.raw();
176190
}
177191
}
@@ -2212,16 +2226,21 @@ DART_EXPORT bool Dart_IsByteBuffer(Dart_Handle handle) {
22122226
DART_EXPORT bool Dart_IsFuture(Dart_Handle handle) {
22132227
DARTSCOPE(Thread::Current());
22142228
API_TIMELINE_DURATION(T);
2215-
Isolate* I = T->isolate();
22162229
const Object& obj = Object::Handle(Z, Api::UnwrapHandle(handle));
22172230
if (obj.IsInstance()) {
2218-
const Class& future_class =
2219-
Class::Handle(I->object_store()->future_class());
2220-
ASSERT(!future_class.IsNull());
2231+
ObjectStore* object_store = T->isolate()->object_store();
2232+
Type& future_rare_type =
2233+
Type::Handle(Z, object_store->non_nullable_future_rare_type());
2234+
if (future_rare_type.IsNull()) {
2235+
const Class& future_class = Class::Handle(object_store->future_class());
2236+
ASSERT(!future_class.IsNull());
2237+
future_rare_type ^= future_class.RareType();
2238+
object_store->set_non_nullable_future_rare_type(future_rare_type);
2239+
}
22212240
const Class& obj_class = Class::Handle(Z, obj.clazz());
2222-
bool is_future = Class::IsSubtypeOf(
2223-
NNBDMode::kLegacyLib, obj_class, Object::null_type_arguments(),
2224-
future_class, Object::null_type_arguments(), Heap::kNew);
2241+
bool is_future = Class::IsSubtypeOf(NNBDMode::kLegacyLib, obj_class,
2242+
Object::null_type_arguments(),
2243+
future_rare_type, Heap::kNew);
22252244
return is_future;
22262245
}
22272246
return false;

runtime/vm/object.cc

Lines changed: 72 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4858,56 +4858,94 @@ bool Class::IsFutureClass() const {
48584858
(library() == Library::AsyncLibrary());
48594859
}
48604860

4861-
// Checks if type S is a subtype of type T, assuming that S is non-nullable.
4862-
// Type S is specified by class 'cls' parameterized with 'type_arguments', and
4863-
// type T by class 'other' parameterized with 'other_type_arguments'.
4861+
// Checks if type T0 is a subtype of type T1, assuming that T0 is non-nullable.
4862+
// Type T0 is specified by class 'cls' parameterized with 'type_arguments', and
4863+
// type T1 is specified by 'other' and must have a type class.
48644864
// This class and class 'other' do not need to be finalized, however, they must
48654865
// be resolved as well as their interfaces.
48664866
bool Class::IsSubtypeOf(NNBDMode mode,
48674867
const Class& cls,
48684868
const TypeArguments& type_arguments,
4869-
const Class& other,
4870-
const TypeArguments& other_type_arguments,
4869+
const AbstractType& other,
48714870
Heap::Space space) {
4872-
// This function does not support Null, dynamic, or void as type S.
4873-
ASSERT(!cls.IsNullClass() && !cls.IsDynamicClass() && !cls.IsVoidClass());
4874-
// Since we assume that S is non-nullable, the nullability of T is irrelevant.
4875-
if (other.IsDynamicClass() || other.IsVoidClass() || other.IsObjectClass()) {
4871+
// This function does not support Null, Never, dynamic, or void as type T0.
4872+
classid_t this_cid = cls.id();
4873+
ASSERT(this_cid != kNullCid && this_cid != kNeverCid &&
4874+
this_cid != kDynamicCid && this_cid != kVoidCid);
4875+
// Type T1 must have a type class (e.g. not a type parameter).
4876+
ASSERT(other.HasTypeClass());
4877+
const classid_t other_cid = other.type_class_id();
4878+
// Since T0 is assumed non-nullable, the nullability of T1 is irrelevant.
4879+
if (other_cid == kDynamicCid || other_cid == kVoidCid ||
4880+
other_cid == kObjectCid) {
48764881
return true;
48774882
}
4878-
// Use the 'this_class' object as if it was the receiver of this method, but
4879-
// instead of recursing, reset it to the super class and loop.
48804883
Thread* thread = Thread::Current();
48814884
Zone* zone = thread->zone();
4885+
const Class& other_class = Class::Handle(zone, other.type_class());
4886+
const TypeArguments& other_type_arguments =
4887+
TypeArguments::Handle(zone, other.arguments());
4888+
// Use the 'this_class' object as if it was the receiver of this method, but
4889+
// instead of recursing, reset it to the super class and loop.
48824890
Class& this_class = Class::Handle(zone, cls.raw());
48834891
while (true) {
4884-
// Apply additional subtyping rules if 'other' is 'FutureOr'.
4885-
if (other.IsFutureOrClass()) {
4886-
if (other_type_arguments.IsNull()) {
4887-
return true;
4892+
// Apply additional subtyping rules if T0 or T1 are 'FutureOr'.
4893+
4894+
// Left FutureOr:
4895+
// if T0 is FutureOr<S0> then:
4896+
// T0 <: T1 iff Future<S0> <: T1 and S0 <: T1
4897+
if (this_cid == kFutureOrCid) {
4898+
// Check Future<S0> <: T1.
4899+
ObjectStore* object_store = Isolate::Current()->object_store();
4900+
const Class& future_class =
4901+
Class::Handle(zone, object_store->future_class());
4902+
ASSERT(!future_class.IsNull() && future_class.NumTypeParameters() == 1 &&
4903+
this_class.NumTypeParameters() == 1);
4904+
ASSERT(type_arguments.IsNull() || type_arguments.Length() >= 1);
4905+
if (Class::IsSubtypeOf(mode, future_class, type_arguments, other,
4906+
space)) {
4907+
// Check S0 <: T1.
4908+
const AbstractType& type_arg =
4909+
AbstractType::Handle(zone, type_arguments.TypeAtNullSafe(0));
4910+
if (type_arg.IsSubtypeOf(mode, other, space)) {
4911+
return true;
4912+
}
48884913
}
4914+
}
4915+
4916+
// Right FutureOr:
4917+
// if T1 is FutureOr<S1> then:
4918+
// T0 <: T1 iff any of the following hold:
4919+
// either T0 <: Future<S1>
4920+
// or T0 <: S1
4921+
// or T0 is X0 and X0 has bound S0 and S0 <: T1 (checked elsewhere)
4922+
if (other_cid == kFutureOrCid) {
48894923
const AbstractType& other_type_arg =
4890-
AbstractType::Handle(zone, other_type_arguments.TypeAt(0));
4924+
AbstractType::Handle(zone, other_type_arguments.TypeAtNullSafe(0));
4925+
// Check if S1 is a top type.
48914926
if (other_type_arg.IsTopType()) {
48924927
return true;
48934928
}
4894-
if (!type_arguments.IsNull() && this_class.IsFutureClass()) {
4929+
// Check T0 <: Future<S1> when T0 is Future<S0>.
4930+
if (this_class.IsFutureClass()) {
48954931
const AbstractType& type_arg =
4896-
AbstractType::Handle(zone, type_arguments.TypeAt(0));
4932+
AbstractType::Handle(zone, type_arguments.TypeAtNullSafe(0));
4933+
// If T0 is Future<S0>, then T0 <: Future<S1>, iff S0 <: S1.
48974934
if (type_arg.IsSubtypeOf(mode, other_type_arg, space)) {
48984935
return true;
48994936
}
49004937
}
4938+
// Check T0 <: Future<S1> when T0 is FutureOr<S0> is already done.
4939+
// Check T0 <: S1.
49014940
if (other_type_arg.HasTypeClass() &&
4902-
Class::IsSubtypeOf(
4903-
mode, this_class, type_arguments,
4904-
Class::Handle(zone, other_type_arg.type_class()),
4905-
TypeArguments::Handle(zone, other_type_arg.arguments()), space)) {
4941+
Class::IsSubtypeOf(mode, this_class, type_arguments, other_type_arg,
4942+
space)) {
49064943
return true;
49074944
}
49084945
}
4946+
49094947
// Check for reflexivity.
4910-
if (this_class.raw() == other.raw()) {
4948+
if (this_class.raw() == other_class.raw()) {
49114949
const intptr_t num_type_params = this_class.NumTypeParameters();
49124950
if (num_type_params == 0) {
49134951
return true;
@@ -4961,7 +4999,7 @@ bool Class::IsSubtypeOf(NNBDMode mode,
49614999
continue;
49625000
}
49635001
if (Class::IsSubtypeOf(mode, interface_class, interface_args, other,
4964-
other_type_arguments, space)) {
5002+
space)) {
49655003
return true;
49665004
}
49675005
}
@@ -4970,6 +5008,7 @@ bool Class::IsSubtypeOf(NNBDMode mode,
49705008
if (this_class.IsNull()) {
49715009
return false;
49725010
}
5011+
this_cid = this_class.id();
49735012
}
49745013
UNREACHABLE();
49755014
return false;
@@ -17569,35 +17608,28 @@ bool Instance::RuntimeTypeIsSubtypeOf(
1756917608
}
1757017609
// RuntimeType of non-null instance is non-nullable, so there is no need to
1757117610
// check nullability of other type.
17572-
return Class::IsSubtypeOf(
17573-
mode, cls, type_arguments,
17574-
Class::Handle(zone, instantiated_other.type_class()),
17575-
TypeArguments::Handle(zone, instantiated_other.arguments()), Heap::kOld);
17611+
return Class::IsSubtypeOf(mode, cls, type_arguments, instantiated_other,
17612+
Heap::kOld);
1757617613
}
1757717614

1757817615
bool Instance::IsFutureOrInstanceOf(Zone* zone,
1757917616
NNBDMode mode,
1758017617
const AbstractType& other) const {
1758117618
if (other.IsType() && other.IsFutureOrType()) {
17582-
if (other.arguments() == TypeArguments::null()) {
17583-
return true;
17584-
}
1758517619
const TypeArguments& other_type_arguments =
1758617620
TypeArguments::Handle(zone, other.arguments());
1758717621
const AbstractType& other_type_arg =
17588-
AbstractType::Handle(zone, other_type_arguments.TypeAt(0));
17622+
AbstractType::Handle(zone, other_type_arguments.TypeAtNullSafe(0));
1758917623
if (other_type_arg.IsTopType()) {
1759017624
return true;
1759117625
}
1759217626
if (Class::Handle(zone, clazz()).IsFutureClass()) {
1759317627
const TypeArguments& type_arguments =
1759417628
TypeArguments::Handle(zone, GetTypeArguments());
17595-
if (!type_arguments.IsNull()) {
17596-
const AbstractType& type_arg =
17597-
AbstractType::Handle(zone, type_arguments.TypeAt(0));
17598-
if (type_arg.IsSubtypeOf(mode, other_type_arg, Heap::kOld)) {
17599-
return true;
17600-
}
17629+
const AbstractType& type_arg =
17630+
AbstractType::Handle(zone, type_arguments.TypeAtNullSafe(0));
17631+
if (type_arg.IsSubtypeOf(mode, other_type_arg, Heap::kOld)) {
17632+
return true;
1760117633
}
1760217634
}
1760317635
// Retry RuntimeTypeIsSubtypeOf after unwrapping type arg of FutureOr.
@@ -18396,26 +18428,22 @@ bool AbstractType::IsSubtypeOf(NNBDMode mode,
1839618428
return false;
1839718429
}
1839818430
return Class::IsSubtypeOf(
18399-
mode, type_cls, TypeArguments::Handle(zone, arguments()), other_type_cls,
18400-
TypeArguments::Handle(zone, other.arguments()), space);
18431+
mode, type_cls, TypeArguments::Handle(zone, arguments()), other, space);
1840118432
}
1840218433

1840318434
bool AbstractType::IsSubtypeOfFutureOr(Zone* zone,
1840418435
NNBDMode mode,
1840518436
const AbstractType& other,
1840618437
Heap::Space space) const {
1840718438
if (other.IsType() && other.IsFutureOrType()) {
18408-
if (other.arguments() == TypeArguments::null()) {
18409-
return true;
18410-
}
1841118439
// This function is only called with a receiver that is either a function
1841218440
// type or an uninstantiated type parameter, therefore, it cannot be of
1841318441
// class Future and we can spare the check.
1841418442
ASSERT(IsFunctionType() || IsTypeParameter());
1841518443
const TypeArguments& other_type_arguments =
1841618444
TypeArguments::Handle(zone, other.arguments());
1841718445
const AbstractType& other_type_arg =
18418-
AbstractType::Handle(zone, other_type_arguments.TypeAt(0));
18446+
AbstractType::Handle(zone, other_type_arguments.TypeAtNullSafe(0));
1841918447
if (other_type_arg.IsTopType()) {
1842018448
return true;
1842118449
}

runtime/vm/object.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,13 +1226,12 @@ class Class : public Object {
12261226
cls->ptr()->library_->ptr()->flags_);
12271227
}
12281228

1229-
// Returns true if the type specified by cls and type_arguments is a
1230-
// subtype of the type specified by other class and other_type_arguments.
1229+
// Returns true if the type specified by cls and type_arguments is a subtype
1230+
// of the other type.
12311231
static bool IsSubtypeOf(NNBDMode mode,
12321232
const Class& cls,
12331233
const TypeArguments& type_arguments,
1234-
const Class& other,
1235-
const TypeArguments& other_type_arguments,
1234+
const AbstractType& other,
12361235
Heap::Space space);
12371236

12381237
// Check if this is the top level class.

runtime/vm/object_store.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ class ObjectPointerVisitor;
7373
RW(Type, string_type) \
7474
RW(Type, legacy_string_type) \
7575
RW(Type, non_nullable_string_type) \
76+
RW(Type, non_nullable_list_rare_type) /* maybe be null, lazily built */ \
77+
RW(Type, non_nullable_map_rare_type) /* maybe be null, lazily built */ \
78+
RW(Type, non_nullable_future_rare_type) /* maybe be null, lazily built */ \
7679
RW(TypeArguments, type_argument_int) \
7780
RW(TypeArguments, type_argument_legacy_int) \
7881
RW(TypeArguments, type_argument_non_nullable_int) \

0 commit comments

Comments
 (0)