Skip to content

Commit

Permalink
[VM/nnbd] Remove propagation of library mode to runtime after spec ch…
Browse files Browse the repository at this point in the history
…ange.

On 1/27/20, the nnbd specification changed weak and strong mode instance checks
to make them behave uniformly across legacy and opted-in libraries.
Therefore, it is not necessary anymore to propagate the library mode in
generated code to the runtime.

Change-Id: I42d3ddc6e9a921899aeac21be6374c7893a6d27c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138111
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
  • Loading branch information
crelier authored and commit-bot@chromium.org committed Mar 3, 2020
1 parent 5cd6b30 commit 5701c4f
Show file tree
Hide file tree
Showing 55 changed files with 372 additions and 731 deletions.
5 changes: 0 additions & 5 deletions pkg/vm/lib/bytecode/dbc.dart
Expand Up @@ -674,8 +674,3 @@ const int argumentsLimit = 1 << 8;
// Base class for exceptions thrown when certain limit of bytecode
// format is exceeded.
abstract class BytecodeLimitExceededException {}

// Constants specifying the NNBD mode of libraries.
// Must be kept in sync with enum class NNBDMode in the VM.
const int kLegacy = 0;
const int kOptedIn = 1;
12 changes: 2 additions & 10 deletions pkg/vm/lib/bytecode/gen_bytecode.dart
Expand Up @@ -1272,11 +1272,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
}
}

void _genPushNnbdMode() {
asm.emitPushInt(
staticTypeContext.isNonNullableByDefault ? kOptedIn : kLegacy);
}

Constant _getConstant(Expression expr) {
if (expr is ConstantExpression) {
return expr.constant;
Expand Down Expand Up @@ -1688,11 +1683,10 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
asm.emitPushNull(); // Function type arguments.
}
asm.emitPushConstant(cp.addType(type));
_genPushNnbdMode();
final argDesc = objectTable.getArgDescHandle(5);
final argDesc = objectTable.getArgDescHandle(4);
final cpIndex =
cp.addInterfaceCall(InvocationKind.method, objectInstanceOf, argDesc);
asm.emitInterfaceCall(cpIndex, 5);
asm.emitInterfaceCall(cpIndex, 4);
}

void start(Member node, bool hasCode) {
Expand Down Expand Up @@ -2449,7 +2443,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
final DartType bound = (forwardingTypeParameterBounds != null)
? forwardingTypeParameterBounds[typeParam]
: typeParam.bound;
// TODO(regis): Revisit and take nnbdMode into consideration.
final DartType type = new TypeParameterType(typeParam, Nullability.legacy);
_genPushInstantiatorAndFunctionTypeArguments([type, bound]);
asm.emitPushConstant(cp.addType(type));
Expand Down Expand Up @@ -2478,7 +2471,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
_genPushInstantiatorAndFunctionTypeArguments([type]);
asm.emitPushConstant(
name != null ? cp.addName(name) : cp.addString(message));
// TODO(regis): Revisit and take nnbd mode into consideration.
bool isIntOk = typeEnvironment.isSubtypeOf(
typeEnvironment.coreTypes.intLegacyRawType,
type,
Expand Down
14 changes: 7 additions & 7 deletions pkg/vm/lib/bytecode/object_table.dart
Expand Up @@ -763,7 +763,7 @@ class _NeverTypeHandle extends _TypeHandle {
other is _NeverTypeHandle && this.nullability == other.nullability;

@override
// TODO(regis): Print nullability, only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() => 'Never';
}

Expand Down Expand Up @@ -803,7 +803,7 @@ class _SimpleTypeHandle extends _TypeHandle {
this.nullability == other.nullability;

@override
// TODO(regis): Print nullability, only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() => '$class_';
}

Expand Down Expand Up @@ -857,7 +857,7 @@ class _TypeParameterHandle extends _TypeHandle {
this.nullability == other.nullability;

@override
// TODO(regis): Print nullability, only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() => '$parent::TypeParam/$indexInParent';
}

Expand Down Expand Up @@ -903,7 +903,7 @@ class _GenericTypeHandle extends _TypeHandle {
this.nullability == other.nullability;

@override
// TODO(regis): Print nullability, only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() => '$class_ $typeArgs';
}

Expand Down Expand Up @@ -956,7 +956,7 @@ class _RecursiveGenericTypeHandle extends _TypeHandle {
this.nullability == other.nullability;

@override
// TODO(regis): Print nullability, only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() => '(recursive #$id) $class_ $typeArgs';
}

Expand Down Expand Up @@ -1009,7 +1009,7 @@ class NameAndType {
this.type == other.type;

@override
// TODO(regis): Print nullability, always or only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() => '$type ${name.name}';
}

Expand Down Expand Up @@ -1152,7 +1152,7 @@ class _FunctionTypeHandle extends _TypeHandle {
this.returnType == other.returnType;

@override
// TODO(regis): Print nullability, only if nnbd experiment is enabled?
// TODO(regis): Print nullability.
String toString() {
StringBuffer sb = new StringBuffer();
sb.write('FunctionType');
Expand Down
21 changes: 8 additions & 13 deletions pkg/vm/testcases/bytecode/type_ops.dart.expect
Expand Up @@ -31,8 +31,7 @@ L1:
PushNull
PushNull
PushConstant CP#6
PushInt 0
InterfaceCall CP#7, 5
InterfaceCall CP#7, 4
JumpIfFalse L2
PushConstant CP#9
DirectCall CP#4, 1
Expand All @@ -54,7 +53,7 @@ ConstantPool {
[4] = DirectCall 'dart:core::print', ArgDesc num-args 1, num-type-args 0, names []
[5] = Reserved
[6] = Type #lib::C < dart:core::String, dart:core::int, dart:core::Object, dynamic >
[7] = InterfaceCall 'dart:core::Object::_instanceOf', ArgDesc num-args 5, num-type-args 0, names []
[7] = InterfaceCall 'dart:core::Object::_instanceOf', ArgDesc num-args 4, num-type-args 0, names []
[8] = Reserved
[9] = ObjectRef '12'
[10] = Type #lib::A < dart:core::int >
Expand Down Expand Up @@ -222,8 +221,7 @@ Bytecode {
LoadTypeArgumentsField CP#0
PushNull
PushConstant CP#1
PushInt 0
InterfaceCall CP#2, 5
InterfaceCall CP#2, 4
JumpIfFalse L1
PushConstant CP#4
DirectCall CP#5, 1
Expand All @@ -234,8 +232,7 @@ L1:
LoadTypeArgumentsField CP#0
PushNull
PushConstant CP#7
PushInt 0
InterfaceCall CP#2, 5
InterfaceCall CP#2, 4
JumpIfFalse L2
PushConstant CP#8
DirectCall CP#5, 1
Expand All @@ -257,7 +254,7 @@ L2:
ConstantPool {
[0] = TypeArgumentsField #lib::D
[1] = Type #lib::A < #lib::D::TypeParam/0 >
[2] = InterfaceCall 'dart:core::Object::_instanceOf', ArgDesc num-args 5, num-type-args 0, names []
[2] = InterfaceCall 'dart:core::Object::_instanceOf', ArgDesc num-args 4, num-type-args 0, names []
[3] = Reserved
[4] = ObjectRef '21'
[5] = DirectCall 'dart:core::print', ArgDesc num-args 1, num-type-args 0, names []
Expand Down Expand Up @@ -285,8 +282,7 @@ Bytecode {
PushNull
Push r0
PushConstant CP#0
PushInt 0
InterfaceCall CP#1, 5
InterfaceCall CP#1, 4
JumpIfFalse L1
PushConstant CP#3
DirectCall CP#4, 1
Expand All @@ -297,8 +293,7 @@ L1:
LoadTypeArgumentsField CP#6
Push r0
PushConstant CP#7
PushInt 0
InterfaceCall CP#1, 5
InterfaceCall CP#1, 4
JumpIfFalse L2
PushConstant CP#8
DirectCall CP#4, 1
Expand All @@ -316,7 +311,7 @@ L2:
}
ConstantPool {
[0] = Type #lib::A < #lib::D::foo3::TypeParam/0 >
[1] = InterfaceCall 'dart:core::Object::_instanceOf', ArgDesc num-args 5, num-type-args 0, names []
[1] = InterfaceCall 'dart:core::Object::_instanceOf', ArgDesc num-args 4, num-type-args 0, names []
[2] = Reserved
[3] = ObjectRef '31'
[4] = DirectCall 'dart:core::print', ArgDesc num-args 1, num-type-args 0, names []
Expand Down
1 change: 0 additions & 1 deletion runtime/bin/entrypoints_verification_test_extension.cc
Expand Up @@ -61,7 +61,6 @@ void FailClosurizeConstructor(const char* name, Dart_Handle result) {
}

void TestFields(Dart_Handle target) {
// TODO(regis): Fix these tests, as a function type is not nullable.
FAIL("fld0", Dart_GetField(target, Dart_NewStringFromCString("fld0")));
FAIL("fld0",
Dart_SetField(target, Dart_NewStringFromCString("fld0"), Dart_Null()));
Expand Down
3 changes: 1 addition & 2 deletions runtime/lib/ffi.cc
Expand Up @@ -278,8 +278,7 @@ DEFINE_NATIVE_ENTRY(Ffi_storePointer, 0, 3) {

auto& new_value_type =
AbstractType::Handle(zone, new_value.GetType(Heap::kNew));
if (!new_value_type.IsSubtypeOf(NNBDMode::kLegacyLib, pointer_type_arg,
Heap::kNew)) {
if (!new_value_type.IsSubtypeOf(pointer_type_arg, Heap::kNew)) {
const String& error = String::Handle(String::NewFormatted(
"New value (%s) is not a subtype of '%s'.",
String::Handle(new_value_type.UserVisibleName()).ToCString(),
Expand Down
15 changes: 7 additions & 8 deletions runtime/lib/mirrors.cc
Expand Up @@ -713,8 +713,8 @@ static RawAbstractType* InstantiateType(const AbstractType& type,
instantiator_type_args = instantiator.arguments();
}
AbstractType& result = AbstractType::Handle(type.InstantiateFrom(
NNBDMode::kLegacyLib, instantiator_type_args,
Object::null_type_arguments(), kAllFree, NULL, Heap::kOld));
instantiator_type_args, Object::null_type_arguments(), kAllFree, NULL,
Heap::kOld));
ASSERT(result.IsFinalized());
return result.Canonicalize();
}
Expand Down Expand Up @@ -1483,8 +1483,8 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 0, 5) {
// type arguments of the type reflected by the class mirror.
ASSERT(redirect_type.IsInstantiated(kFunctions));
redirect_type ^= redirect_type.InstantiateFrom(
NNBDMode::kLegacyLib, type_arguments, Object::null_type_arguments(),
kNoneFree, NULL, Heap::kOld);
type_arguments, Object::null_type_arguments(), kNoneFree, NULL,
Heap::kOld);
redirect_type ^= redirect_type.Canonicalize();
}

Expand Down Expand Up @@ -1513,8 +1513,7 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 0, 5) {
ArgumentsDescriptor::New(kTypeArgsLen, args.Length(), arg_names));

ArgumentsDescriptor args_descriptor(args_descriptor_array);
if (!redirected_constructor.AreValidArguments(NNBDMode::kLegacyLib,
args_descriptor, NULL)) {
if (!redirected_constructor.AreValidArguments(args_descriptor, NULL)) {
external_constructor_name = redirected_constructor.name();
ThrowNoSuchMethod(AbstractType::Handle(klass.RareType()),
external_constructor_name, explicit_args, arg_names,
Expand All @@ -1524,7 +1523,7 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 0, 5) {
}
const Object& type_error =
Object::Handle(redirected_constructor.DoArgumentTypesMatch(
NNBDMode::kLegacyLib, args, args_descriptor, type_arguments));
args, args_descriptor, type_arguments));
if (!type_error.IsNull()) {
Exceptions::PropagateError(Error::Cast(type_error));
UNREACHABLE();
Expand Down Expand Up @@ -1764,7 +1763,7 @@ DEFINE_NATIVE_ENTRY(VariableMirror_type, 0, 2) {
DEFINE_NATIVE_ENTRY(TypeMirror_subtypeTest, 0, 2) {
GET_NON_NULL_NATIVE_ARGUMENT(AbstractType, a, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(AbstractType, b, arguments->NativeArgAt(1));
return Bool::Get(a.IsSubtypeOf(NNBDMode::kLegacyLib, b, Heap::kNew)).raw();
return Bool::Get(a.IsSubtypeOf(b, Heap::kNew)).raw();
}

#endif // !DART_PRECOMPILED_RUNTIME
Expand Down
26 changes: 6 additions & 20 deletions runtime/lib/object.cc
Expand Up @@ -133,7 +133,7 @@ DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 0, 2) {
.raw();
}

DEFINE_NATIVE_ENTRY(Object_instanceOf, 0, 5) {
DEFINE_NATIVE_ENTRY(Object_instanceOf, 0, 4) {
const Instance& instance =
Instance::CheckedHandle(zone, arguments->NativeArgAt(0));
const TypeArguments& instantiator_type_arguments =
Expand All @@ -142,11 +142,9 @@ DEFINE_NATIVE_ENTRY(Object_instanceOf, 0, 5) {
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(2));
const AbstractType& type =
AbstractType::CheckedHandle(zone, arguments->NativeArgAt(3));
const NNBDMode nnbd_mode = static_cast<NNBDMode>(
Smi::CheckedHandle(zone, arguments->NativeArgAt(4)).Value());
ASSERT(type.IsFinalized());
const bool is_instance_of = instance.IsInstanceOf(
nnbd_mode, type, instantiator_type_arguments, function_type_arguments);
type, instantiator_type_arguments, function_type_arguments);
if (FLAG_trace_type_checks) {
const char* result_str = is_instance_of ? "true" : "false";
OS::PrintErr("Native Object.instanceOf: result %s\n", result_str);
Expand All @@ -169,18 +167,8 @@ DEFINE_NATIVE_ENTRY(Object_simpleInstanceOf, 0, 2) {
AbstractType::CheckedHandle(zone, arguments->NativeArgAt(1));
ASSERT(type.IsFinalized());
ASSERT(type.IsInstantiated());
// If the instance is not null, the result of _simpleInstanceOf does not
// depend on the nnbd mode, because we only check against a rare type,
// i.e. a class. However, we need to determine the correct nnbd mode when
// the instance is null.
// Since type literals are not imported, a legacy type indicates that the
// call originated in a legacy library. Note that the type test against a
// non-legacy type (even in a legacy library) such as dynamic, void, or Null
// yield the same result independently of the mode used.
NNBDMode mode =
type.IsLegacy() ? NNBDMode::kLegacyLib : NNBDMode::kOptedInLib;
const bool is_instance_of = instance.IsInstanceOf(
mode, type, Object::null_type_arguments(), Object::null_type_arguments());
type, Object::null_type_arguments(), Object::null_type_arguments());
return Bool::Get(is_instance_of).raw();
}

Expand Down Expand Up @@ -247,8 +235,8 @@ static bool ExtractInterfaceTypeArgs(Zone* zone,
if (!cur_interface_type_args.IsNull() &&
!cur_interface_type_args.IsInstantiated()) {
cur_interface_type_args = cur_interface_type_args.InstantiateFrom(
cur_cls.nnbd_mode(), instance_type_args,
Object::null_type_arguments(), kNoneFree, NULL, Heap::kNew);
instance_type_args, Object::null_type_arguments(), kNoneFree, NULL,
Heap::kNew);
}
if (ExtractInterfaceTypeArgs(zone, cur_interface_cls,
cur_interface_type_args, interface_cls,
Expand Down Expand Up @@ -401,10 +389,8 @@ DEFINE_NATIVE_ENTRY(Internal_boundsCheckForPartialInstantiation, 0, 2) {
ASSERT(!supertype.IsNull());

// The supertype may not be instantiated.
// TODO(regis): What is the correct nnbd mode to use here?
if (!AbstractType::InstantiateAndTestSubtype(
NNBDMode::kLegacyLib, &subtype, &supertype, instantiator_type_args,
function_type_args)) {
&subtype, &supertype, instantiator_type_args, function_type_args)) {
// Throw a dynamic type error.
TokenPosition location;
{
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/bootstrap_natives.h
Expand Up @@ -22,7 +22,7 @@ namespace dart {
V(Object_toString, 1) \
V(Object_runtimeType, 1) \
V(Object_haveSameRuntimeType, 2) \
V(Object_instanceOf, 5) \
V(Object_instanceOf, 4) \
V(Object_simpleInstanceOf, 2) \
V(Function_apply, 2) \
V(Closure_equals, 2) \
Expand Down
20 changes: 9 additions & 11 deletions runtime/vm/class_finalizer.cc
Expand Up @@ -390,16 +390,14 @@ void ClassFinalizer::CheckRecursiveType(const Class& cls,
!pending_arguments.IsSubvectorInstantiated(first_type_param,
num_type_params)) {
const TypeArguments& instantiated_arguments = TypeArguments::Handle(
zone, arguments.InstantiateFrom(NNBDMode::kLegacyLib,
Object::null_type_arguments(),
zone, arguments.InstantiateFrom(Object::null_type_arguments(),
Object::null_type_arguments(),
kNoneFree, NULL, Heap::kNew));
const TypeArguments& instantiated_pending_arguments =
TypeArguments::Handle(
zone, pending_arguments.InstantiateFrom(
NNBDMode::kLegacyLib, Object::null_type_arguments(),
Object::null_type_arguments(), kNoneFree, NULL,
Heap::kNew));
TypeArguments::Handle(zone, pending_arguments.InstantiateFrom(
Object::null_type_arguments(),
Object::null_type_arguments(),
kNoneFree, NULL, Heap::kNew));
// By using TypeEquality::kInSubtypeTest, we throw a wider net than
// using canonical or syntactical equality and may reject more
// problematic declarations.
Expand Down Expand Up @@ -625,8 +623,8 @@ void ClassFinalizer::FinalizeTypeArguments(const Class& cls,
continue;
}
super_type_arg = super_type_arg.InstantiateFrom(
cls.nnbd_mode(), arguments, Object::null_type_arguments(),
kNoneFree, instantiation_trail, Heap::kOld);
arguments, Object::null_type_arguments(), kNoneFree,
instantiation_trail, Heap::kOld);
if (super_type_arg.IsBeingFinalized()) {
// The super_type_arg was instantiated from a type being finalized.
// We need to finish finalizing its type arguments.
Expand Down Expand Up @@ -783,8 +781,8 @@ RawAbstractType* ClassFinalizer::FinalizeType(const Class& cls,
const TypeArguments& instantiator_type_arguments =
TypeArguments::Handle(zone, fun_type.arguments());
signature = signature.InstantiateSignatureFrom(
scope_class.nnbd_mode(), instantiator_type_arguments,
Object::null_type_arguments(), kNoneFree, Heap::kOld);
instantiator_type_arguments, Object::null_type_arguments(),
kNoneFree, Heap::kOld);
// Note that if instantiator_type_arguments contains type parameters,
// as in F<K>, the signature is still uninstantiated (the typedef type
// parameters were substituted in the signature with typedef type
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/aot/aot_call_specializer.cc
Expand Up @@ -1310,7 +1310,7 @@ void AotCallSpecializer::TryReplaceWithDispatchTableCall(
const AbstractType& target_type =
AbstractType::Handle(Class::Handle(interface_target.Owner()).RareType());
const bool receiver_can_be_smi =
CompileType::Smi().IsAssignableTo(NNBDMode::kLegacyLib, target_type);
CompileType::Smi().IsAssignableTo(target_type);
auto load_cid = new (Z) LoadClassIdInstr(receiver->CopyWithType(Z), kUntagged,
receiver_can_be_smi);
InsertBefore(call, load_cid, call->env(), FlowGraph::kValue);
Expand Down

0 comments on commit 5701c4f

Please sign in to comment.