Skip to content

Commit

Permalink
[vm] Cleanup legacy differences between 'is' and 'as' checks
Browse files Browse the repository at this point in the history 
With sound null safety both 'is' and 'as' type checks use subtyping
without any extra differences.

Replace Instance::IsAssignableTo with IsInstanceOf (which is now
the same as RuntimeTypeIsSubtypeOf).

Remove Instance::NullIsInstanceOf as it is no longer used.

Replace CompileType::IsAssignableTo and IsInstanceOf with IsSubtypeOf.

TEST=ci

Change-Id: I3a071d114c5f86d990255416b244961d506257a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401861
Commit-Queue: Alexander Markov <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>
  • Loading branch information
@alexmarkov
alexmarkov authored and Commit Queue committed Dec 19, 2024
1 parent da257d2 commit 5fd11d3
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 139 deletions.
10 changes: 1 addition & 9 deletions runtime/vm/compiler/backend/compile_type.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,6 @@ class CompileType : public ZoneAllocated {
// Return true if this type is a subtype of the given type.
bool IsSubtypeOf(const AbstractType& other);

// Return true if value of this type is assignable to a location of the
// given type.
bool IsAssignableTo(const AbstractType& other);

// Return true if value of this type always passes 'is' test
// against given type.
bool IsInstanceOf(const AbstractType& other);

// Return the non-nullable version of this type.
CompileType CopyNonNullable() {
if (IsNull()) {
Expand Down Expand Up @@ -278,7 +270,7 @@ class CompileType : public ZoneAllocated {

// Returns true if a value of this CompileType can contain a Smi.
// Note that this is not the same as calling
// CompileType::Smi().IsAssignableTo(this) - because this compile type
// CompileType::Smi().IsSubtypeOf(this) - because this compile type
// can be uninstantiated.
bool CanBeSmi();

Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/constant_propagator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ void ConstantPropagator::VisitAssertAssignable(AssertAssignableInstr* instr) {
if (dst_type.IsAbstractType()) {
// We are ignoring the instantiator and instantiator_type_arguments, but
// still monotonic and safe.
if (instr->value()->Type()->IsAssignableTo(AbstractType::Cast(dst_type))) {
if (instr->value()->Type()->IsSubtypeOf(AbstractType::Cast(dst_type))) {
SetValue(instr, value);
return;
}
Expand Down
1 change: 0 additions & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2714,7 +2714,6 @@ void FlowGraphCompiler::GenerateInstanceOf(const InstructionSource& source,
AbstractType::Handle(type.UnwrapFutureOr());
if (!unwrapped_type.IsTypeParameter() || unwrapped_type.IsNullable()) {
// Only nullable type parameter remains nullable after instantiation.
// See NullIsInstanceOf().
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
__ BranchIf(EQUAL,
unwrapped_type.IsNullable() ? &is_instance : &is_not_instance);
Expand Down
7 changes: 3 additions & 4 deletions runtime/vm/compiler/backend/il.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3025,8 +3025,7 @@ Definition* AssertAssignableInstr::Canonicalize(FlowGraph* flow_graph) {
const auto& abs_type = AbstractType::Cast(dst_type()->BoundConstant());

if (abs_type.IsTopTypeForSubtyping() ||
(FLAG_eliminate_type_checks &&
value()->Type()->IsAssignableTo(abs_type))) {
(FLAG_eliminate_type_checks && value()->Type()->IsSubtypeOf(abs_type))) {
return value()->definition();
}
if (abs_type.IsInstantiated()) {
Expand Down Expand Up @@ -3111,7 +3110,7 @@ Definition* AssertAssignableInstr::Canonicalize(FlowGraph* flow_graph) {

if (new_dst_type.IsTopTypeForSubtyping() ||
(FLAG_eliminate_type_checks &&
value()->Type()->IsAssignableTo(new_dst_type))) {
value()->Type()->IsSubtypeOf(new_dst_type))) {
return value()->definition();
}
}
Expand Down Expand Up @@ -5148,7 +5147,7 @@ bool InstanceCallBaseInstr::CanReceiverBeSmiBasedOnInterfaceTarget(
// it would compute correctly whether or not receiver can be a smi.
const AbstractType& target_type = AbstractType::Handle(
zone, Class::Handle(zone, interface_target().Owner()).RareType());
if (!CompileType::Smi().IsAssignableTo(target_type)) {
if (!CompileType::Smi().IsSubtypeOf(target_type)) {
return false;
}
}
Expand Down
27 changes: 1 addition & 26 deletions runtime/vm/compiler/backend/type_propagator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -865,18 +865,6 @@ bool CompileType::IsSubtypeOf(const AbstractType& other) {
if (other.IsTopTypeForSubtyping()) {
return true;
}

if (IsNone()) {
return false;
}

return ToAbstractType()->IsSubtypeOf(other, Heap::kOld);
}

bool CompileType::IsAssignableTo(const AbstractType& other) {
if (other.IsTopTypeForSubtyping()) {
return true;
}
// If we allow comparisons against an uninstantiated type, then we can
// end up incorrectly optimizing away AssertAssignables where the incoming
// value and outgoing value have CompileTypes that would return true to the
Expand All @@ -899,19 +887,6 @@ bool CompileType::IsAssignableTo(const AbstractType& other) {
return ToAbstractType()->IsSubtypeOf(other, Heap::kOld);
}

bool CompileType::IsInstanceOf(const AbstractType& other) {
if (other.IsTopTypeForInstanceOf()) {
return true;
}
if (IsNone() || !other.IsInstantiated()) {
return false;
}
if (is_nullable() && !other.IsNullable()) {
return false;
}
return ToAbstractType()->IsSubtypeOf(other, Heap::kOld);
}

bool CompileType::Specialize(GrowableArray<intptr_t>* class_ids) {
ToNullableCid();
if (cid_ != kDynamicCid) {
Expand Down Expand Up @@ -939,7 +914,7 @@ bool CompileType::Specialize(GrowableArray<intptr_t>* class_ids) {
// bounds.
static bool CanPotentiallyBeSmi(const AbstractType& type, bool recurse) {
if (type.IsInstantiated()) {
return CompileType::Smi().IsAssignableTo(type);
return CompileType::Smi().IsSubtypeOf(type);
} else if (type.IsTypeParameter()) {
// For type parameters look at their bounds (if recurse allows us).
const auto& param = TypeParameter::Cast(type);
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/call_specializer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ bool CallSpecializer::TryOptimizeInstanceOfUsingStaticTypes(
}

Value* left_value = call->Receiver();
if (left_value->Type()->IsInstanceOf(type)) {
if (left_value->Type()->IsSubtypeOf(type)) {
ConstantInstr* replacement = flow_graph()->GetConstant(Bool::True());
call->ReplaceUsesWith(replacement);
ASSERT(current_iterator()->Current() == call);
Expand Down Expand Up @@ -1507,7 +1507,7 @@ void TypedDataSpecializer::TryInlineCall(TemplateDartCall<0>* call) {

auto& type_class = Class::Handle(zone_);
for (auto& variant : typed_data_variants_) {
if (!receiver_type->IsAssignableTo(variant.array_type)) {
if (!receiver_type->IsSubtypeOf(variant.array_type)) {
continue;
}

Expand Down Expand Up @@ -1535,7 +1535,7 @@ void TypedDataSpecializer::TryInlineCall(TemplateDartCall<0>* call) {
type_class = variant.array_type.type_class();
ReplaceWithIndexGet(call, variant.array_cid);
} else {
if (!value_type->IsAssignableTo(variant.element_type)) {
if (!value_type->IsSubtypeOf(variant.element_type)) {
return;
}
type_class = variant.array_type.type_class();
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/isolate_reload.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2386,8 +2386,8 @@ class FieldInvalidator {
}

instance_ ^= value.ptr();
if (instance_.IsAssignableTo(type, instantiator_type_arguments_,
function_type_arguments_)) {
if (instance_.IsInstanceOf(type, instantiator_type_arguments_,
function_type_arguments_)) {
// Do not add record instances to cache as they don't have a valid
// key (type of a record depends on types of all its fields).
if (cid != kRecordCid) {
Expand Down
81 changes: 2 additions & 79 deletions runtime/vm/object.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10011,12 +10011,8 @@ ObjectPtr Function::DoArgumentTypesMatch(
const TypeArguments& function_type_args) -> bool {
// If the argument type is the top type, no need to check.
if (type.IsTopTypeForSubtyping()) return true;
if (argument.IsNull()) {
return Instance::NullIsAssignableTo(type, instantiator_type_args,
function_type_args);
}
return argument.IsAssignableTo(type, instantiator_type_args,
function_type_args);
return argument.IsInstanceOf(type, instantiator_type_args,
function_type_args);
};

// Check types of the provided arguments against the expected parameter types.
Expand Down Expand Up @@ -21342,88 +21338,15 @@ void Instance::SetTypeArguments(const TypeArguments& value) const {
SetFieldAtOffset(field_offset, value);
}

/*
Specification of instance checks (e is T) and casts (e as T), where e evaluates
to a value v and v has runtime type S:

Instance checks (e is T) in weak checking mode in a legacy or opted-in library:
If v == null and T is a legacy type
return LEGACY_SUBTYPE(T, Null) || LEGACY_SUBTYPE(Object, T)
If v == null and T is not a legacy type, return NNBD_SUBTYPE(Null, T)
Otherwise return LEGACY_SUBTYPE(S, T)

Instance checks (e is T) in strong checking mode in a legacy or opted-in lib:
If v == null and T is a legacy type
return LEGACY_SUBTYPE(T, Null) || LEGACY_SUBTYPE(Object, T)
Otherwise return NNBD_SUBTYPE(S, T)

Casts (e as T) in weak checking mode in a legacy or opted-in library:
If LEGACY_SUBTYPE(S, T) then e as T evaluates to v.
Otherwise a TypeError is thrown.

Casts (e as T) in strong checking mode in a legacy or opted-in library:
If NNBD_SUBTYPE(S, T) then e as T evaluates to v.
Otherwise a TypeError is thrown.
*/

bool Instance::IsInstanceOf(
const AbstractType& other,
const TypeArguments& other_instantiator_type_arguments,
const TypeArguments& other_function_type_arguments) const {
ASSERT(!other.IsDynamicType());
if (IsNull()) {
return Instance::NullIsInstanceOf(other, other_instantiator_type_arguments,
other_function_type_arguments);
}
// In strong mode, compute NNBD_SUBTYPE(runtimeType, other).
// In weak mode, compute LEGACY_SUBTYPE(runtimeType, other).
return RuntimeTypeIsSubtypeOf(other, other_instantiator_type_arguments,
other_function_type_arguments);
}

bool Instance::IsAssignableTo(
const AbstractType& other,
const TypeArguments& other_instantiator_type_arguments,
const TypeArguments& other_function_type_arguments) const {
ASSERT(!other.IsDynamicType());
// In strong mode, compute NNBD_SUBTYPE(runtimeType, other).
// In weak mode, compute LEGACY_SUBTYPE(runtimeType, other).
return RuntimeTypeIsSubtypeOf(other, other_instantiator_type_arguments,
other_function_type_arguments);
}

// If 'other' type (once instantiated) is a legacy type:
// return LEGACY_SUBTYPE(other, Null) || LEGACY_SUBTYPE(Object, other).
// Otherwise return NNBD_SUBTYPE(Null, T).
// Ignore value of strong flag value.
bool Instance::NullIsInstanceOf(
const AbstractType& other,
const TypeArguments& other_instantiator_type_arguments,
const TypeArguments& other_function_type_arguments) {
ASSERT(other.IsFinalized());
if (other.IsNullable()) {
// This case includes top types (void, dynamic, Object?).
// The uninstantiated nullable type will remain nullable after
// instantiation.
return true;
}
if (other.IsFutureOrType()) {
const auto& type = AbstractType::Handle(other.UnwrapFutureOr());
return NullIsInstanceOf(type, other_instantiator_type_arguments,
other_function_type_arguments);
}
// No need to instantiate type, unless it is a type parameter.
// Note that a typeref cannot refer to a type parameter.
if (other.IsTypeParameter()) {
auto& type = AbstractType::Handle(other.InstantiateFrom(
other_instantiator_type_arguments, other_function_type_arguments,
kAllFree, Heap::kOld));
return Instance::NullIsInstanceOf(type, Object::null_type_arguments(),
Object::null_type_arguments());
}
return false;
}

// Must be kept in sync with GenerateNullIsAssignableToType in
// stub_code_compiler.cc if any changes are made.
bool Instance::NullIsAssignableTo(const AbstractType& other) {
Expand Down
12 changes: 0 additions & 12 deletions runtime/vm/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8348,12 +8348,6 @@ class Instance : public Object {
const TypeArguments& other_instantiator_type_arguments,
const TypeArguments& other_function_type_arguments) const;

// Check if this instance is assignable to the given other type.
// The type argument vectors are used to instantiate the other type if needed.
bool IsAssignableTo(const AbstractType& other,
const TypeArguments& other_instantiator_type_arguments,
const TypeArguments& other_function_type_arguments) const;

// Return true if the null instance can be assigned to a variable of [other]
// type. Return false if null cannot be assigned or we cannot tell (if
// [other] is a type parameter in NNBD strong mode). Only used for checks at
Expand Down Expand Up @@ -8477,12 +8471,6 @@ class Instance : public Object {
bool RuntimeTypeIsSubtypeOfFutureOr(Zone* zone,
const AbstractType& other) const;

// Return true if the null instance is an instance of other type.
static bool NullIsInstanceOf(
const AbstractType& other,
const TypeArguments& other_instantiator_type_arguments,
const TypeArguments& other_function_type_arguments);

CompressedObjectPtr* FieldAddrAtOffset(intptr_t offset) const {
ASSERT(IsValidFieldOffset(offset));
return reinterpret_cast<CompressedObjectPtr*>(raw_value() - kHeapObjectTag +
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/runtime_entry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1359,7 +1359,7 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) {
// This is guaranteed on the calling side.
ASSERT(!dst_type.IsDynamicType());

const bool is_instance_of = src_instance.IsAssignableTo(
const bool is_instance_of = src_instance.IsInstanceOf(
dst_type, instantiator_type_arguments, function_type_arguments);

if (FLAG_trace_type_checks) {
Expand Down Expand Up @@ -1433,7 +1433,7 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) {
if (result.IsError()) {
Exceptions::PropagateError(Error::Cast(result));
}
// IsAssignableTo returned false, so we should have thrown a type
// IsInstanceOf returned false, so we should have thrown a type
// error in DoArgumentsTypesMatch.
UNREACHABLE();
}
Expand Down

0 comments on commit 5fd11d3

Please sign in to comment.