[vm/nnbd] Add --strict_null_safety_checks option

This option enables NNBD strong mode type checks and required parameter
checks even in NNBD weak mode. It is useful for testing partially
migrated code which cannot use sound null safety yet.

TEST=runtime/tests/vm/dart/strict_null_safety_checks_in_weak_mode_test.dart

Change-Id: I899e83d260fbed5b0fc5c1dcf0e42831d021bbe8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169963
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>

runtime/tests/vm/dart/strict_null_safety_checks_in_weak_mode_test.dart

@@ -0,0 +1,152 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This test verifies that with --strict_null_safety_checks runtime casts and
+// required parameters have strong mode semantics in weak mode.
+// Derived from tests/language/nnbd/subtyping/type_casts_strong_test.dart
+// and tests/language/nnbd/required_named_parameters/required_named_args_strong_test.dart.
+
+// VMOptions=--strict_null_safety_checks --optimization_counter_threshold=10 --deterministic
+// Requirements=nnbd-weak
+
+import 'package:expect/expect.dart';
+
+class C {}
+
+class W<T> {
+  @pragma('vm:never-inline')
+  asT(arg) => arg as T;
+
+  @pragma('vm:never-inline')
+  asNullableT(arg) => arg as T?;
+
+  @pragma('vm:never-inline')
+  asXT(arg) => arg as X<T>;
+
+  @pragma('vm:never-inline')
+  asNullableXT(arg) => arg as X<T>?;
+
+  @pragma('vm:never-inline')
+  asXNullableT(arg) => arg as X<T?>;
+}
+
+class X<T> {}
+
+class Y {}
+
+class Z extends Y {}
+
+testCasts() {
+  // Testing 'arg as T', T = Y
+  final wy = new W<Y>();
+  wy.asT(new Y());
+  wy.asT(new Z());
+  Expect.throwsTypeError(() {
+    wy.asT(null);
+  });
+  Expect.throwsTypeError(() {
+    wy.asT(new C());
+  });
+
+  // Testing 'arg as T?', T = Y
+  wy.asNullableT(new Y());
+  wy.asNullableT(new Z());
+  wy.asNullableT(null);
+  Expect.throwsTypeError(() {
+    wy.asNullableT(new C());
+  });
+
+  // Testing 'arg as X<T>', T = Y
+  wy.asXT(new X<Y>());
+  wy.asXT(new X<Z>());
+  Expect.throwsTypeError(() {
+    wy.asXT(null);
+  });
+  Expect.throwsTypeError(() {
+    wy.asXT(new X<dynamic>());
+  });
+  Expect.throwsTypeError(() {
+    wy.asXT(new X<Y?>());
+  });
+
+  // Testing 'arg as X<T>?', T = Y
+  wy.asNullableXT(new X<Y>());
+  wy.asNullableXT(new X<Z>());
+  wy.asNullableXT(null);
+  Expect.throwsTypeError(() {
+    wy.asNullableXT(new X<dynamic>());
+  });
+  Expect.throwsTypeError(() {
+    wy.asNullableXT(new X<Y?>());
+  });
+
+  // Testing 'arg as X<T?>', T = Y
+  wy.asXNullableT(new X<Y>());
+  wy.asXNullableT(new X<Z>());
+  wy.asXNullableT(new X<Y?>());
+  Expect.throwsTypeError(() {
+    wy.asXNullableT(null);
+  });
+  Expect.throwsTypeError(() {
+    wy.asXNullableT(new X<dynamic>());
+  });
+
+  // Testing 'arg as X<T>', T = Y?
+  final wny = new W<Y?>();
+  wny.asXT(new X<Y>());
+  wny.asXT(new X<Z>());
+  wny.asXT(new X<Y?>());
+  Expect.throwsTypeError(() {
+    wny.asXT(null);
+  });
+  Expect.throwsTypeError(() {
+    wny.asXT(new X<dynamic>());
+  });
+}
+
+func(String q0, {bool p3 = false, required int p1, required String p2}) => "";
+
+testRequiredParameters() {
+  dynamic f = func;
+
+  // Invalid: Subtype may not redeclare optional parameters as required.
+  Expect.throwsTypeError(() {
+    Function(
+      String p0, {
+      required int p1,
+      String p2,
+    }) t2 = f;
+  });
+
+  // Invalid: Subtype may not declare new required named parameters.
+  Expect.throwsTypeError(() {
+    Function(
+      String p0, {
+      required int p1,
+    }) t3 = f;
+  });
+
+  // Invalid: Invocation with explicit null required named argument.
+  Expect.throwsTypeError(() {
+    f("", p1: null, p2: null);
+  });
+  Expect.throwsTypeError(() {
+    Function.apply(f, [""], {#p1: null, #p2: null});
+  });
+
+  // Invalid: Invocation that omits a required named argument.
+  Expect.throwsNoSuchMethodError(() {
+    f("", p1: 100);
+  });
+  Expect.throwsNoSuchMethodError(() {
+    Function.apply(f, [""], {#p1: 100});
+  });
+}
+
+main() {
+  for (int i = 0; i < 20; ++i) {
+    testCasts();
+    testRequiredParameters();
+  }
+}

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -2414,7 +2414,8 @@ SubtypeTestCachePtr FlowGraphCompiler::GenerateSubtype1TestCacheLookup(
   // Check immediate superclass equality. If type_class is Object, then testing
   // supertype may yield a wrong result for Null in NNBD strong mode (because
   // Null also extends Object).
-  if (!type_class.IsObjectClass() || !Isolate::Current()->null_safety()) {
+  if (!type_class.IsObjectClass() ||
+      !Isolate::Current()->use_strict_null_safety_checks()) {
     // We don't use TypeTestABI::kScratchReg for the first scratch register as
     // it is not defined on IA32. Instead, we use the subtype test cache
     // register, as it is clobbered by the subtype test cache stub call anyway.
@@ -2785,7 +2786,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
   if (dst_type.IsNull()) {
     __ Comment("AssertAssignable for runtime type");
     // kDstTypeReg should already contain the destination type.
-    const bool null_safety = isolate()->null_safety();
+    const bool null_safety = isolate()->use_strict_null_safety_checks();
     GenerateStubCall(token_pos,
                      StubCode::GetTypeIsTopTypeForSubtyping(null_safety),
                      PcDescriptorsLayout::kOther, locs, deopt_id);
@@ -2906,7 +2907,8 @@ void FlowGraphCompiler::GenerateCallerChecksForAssertAssignable(
 
   if (dst_type.IsObjectType()) {
     // Special case: non-nullable Object.
-    ASSERT(dst_type.IsNonNullable() && isolate()->null_safety());
+    ASSERT(dst_type.IsNonNullable() &&
+           isolate()->use_strict_null_safety_checks());
     __ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
     __ BranchIf(NOT_EQUAL, done);
     // Fall back to type testing stub in caller to throw the exception.
@@ -2928,7 +2930,8 @@ void FlowGraphCompiler::GenerateCallerChecksForAssertAssignable(
     // Special case: Instantiate the type parameter on the caller side, invoking
     // the TTS of the corresponding type parameter in the caller.
     const TypeParameter& type_param = TypeParameter::Cast(dst_type);
-    if (isolate()->null_safety() && !type_param.IsNonNullable()) {
+    if (isolate()->use_strict_null_safety_checks() &&
+        !type_param.IsNonNullable()) {
       // If the type parameter is nullable when running in strong mode, we need
       // to handle null before calling the TTS because the type parameter may be
       // instantiated with a non-nullable type, where the TTS rejects null.

runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc

@@ -358,7 +358,8 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
   if (dst_type.IsNull()) {
     __ Comment("AssertAssignable for runtime type");
     // kDstTypeReg should already contain the destination type.
-    const bool null_safety = Isolate::Current()->null_safety();
+    const bool null_safety =
+        Isolate::Current()->use_strict_null_safety_checks();
     GenerateStubCall(token_pos,
                      StubCode::GetTypeIsTopTypeForSubtyping(null_safety),
                      PcDescriptorsLayout::kOther, locs, deopt_id);

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -2102,7 +2102,7 @@ Fragment FlowGraphBuilder::TestClosureFunctionNamedParameterRequired(
     Fragment set,
     Fragment not_set) {
   // Required named arguments only exist if null_safety is enabled.
-  if (!I->null_safety()) return not_set;
+  if (!I->use_strict_null_safety_checks()) return not_set;
 
   Fragment check_required;
   // First, we convert the index to be in terms of the number of optional
@@ -2277,7 +2277,7 @@ Fragment FlowGraphBuilder::BuildClosureCallNamedArgumentsCheck(
   // required named arguments.
   if (info.descriptor.NamedCount() == 0) {
     // No work to do if there are no possible required named parameters.
-    if (!I->null_safety()) {
+    if (!I->use_strict_null_safety_checks()) {
       return Fragment();
     }
     // If the below changes, we can no longer assume that flag slots existing

runtime/vm/compiler/frontend/prologue_builder.cc

@@ -306,7 +306,8 @@ Fragment PrologueBuilder::BuildOptionalParameterHandling(
   } else {
     ASSERT(num_opt_named_params > 0);
 
-    bool null_safety = Isolate::Current()->null_safety();
+    bool check_required_params =
+        Isolate::Current()->use_strict_null_safety_checks();
     const intptr_t first_name_offset =
         compiler::target::ArgumentsDescriptor::first_named_entry_offset() -
         compiler::target::Array::data_offset();
@@ -371,8 +372,8 @@ Fragment PrologueBuilder::BuildOptionalParameterHandling(
         good += Drop();
       }
 
-      const bool required =
-          null_safety && function_.IsRequiredAt(opt_param_position[i]);
+      const bool required = check_required_params &&
+                            function_.IsRequiredAt(opt_param_position[i]);
 
       // If this function cannot be invoked dynamically and this is a required
       // named argument, then we can just add this fragment directly without

runtime/vm/flag_list.h

@@ -109,6 +109,9 @@ constexpr bool kDartUseBackgroundCompilation = true;
   R(enable_asserts, false, bool, false, "Enable assert statements.")           \
   R(null_assertions, false, bool, false,                                       \
     "Enable null assertions for parameters.")                                  \
+  R(strict_null_safety_checks, false, bool, false,                             \
+    "Enable strict type checks for non-nullable types and required "           \
+    "parameters.")                                                             \
   P(enable_kernel_expression_compilation, bool, true,                          \
     "Compile expressions with the Kernel front-end.")                          \
   P(enable_mirrors, bool, true,                                                \

runtime/vm/isolate.h

@@ -1337,6 +1337,10 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
     isolate_flags_ = NullSafetyBit::update(null_safety, isolate_flags_);
   }
 
+  bool use_strict_null_safety_checks() const {
+    return null_safety() || FLAG_strict_null_safety_checks;
+  }
+
   bool has_attempted_stepping() const {
     return HasAttemptedSteppingBit::decode(isolate_flags_);
   }

runtime/vm/object.cc

@@ -5083,7 +5083,7 @@ bool Class::IsSubtypeOf(const Class& cls,
   Zone* zone = thread->zone();
   Isolate* isolate = thread->isolate();
   // Nullability of left and right hand sides is verified in strong mode only.
-  const bool verified_nullability = !isolate->null_safety() ||
+  const bool verified_nullability = !isolate->use_strict_null_safety_checks() ||
                                     nullability != Nullability::kNullable ||
                                     !other.IsNonNullable();
 
@@ -7854,7 +7854,7 @@ bool Function::AreValidArguments(const ArgumentsDescriptor& args_desc,
       return false;
     }
   }
-  if (isolate->null_safety()) {
+  if (isolate->use_strict_null_safety_checks()) {
     // Verify that all required named parameters are filled.
     for (intptr_t j = num_parameters - NumOptionalNamedParameters();
          j < num_parameters; j++) {
@@ -8503,7 +8503,7 @@ bool Function::IsSubtypeOf(const Function& other, Heap::Space space) const {
       return false;
     }
   }
-  if (isolate->null_safety()) {
+  if (isolate->use_strict_null_safety_checks()) {
     // Check that for each required named parameter in this function, there's a
     // corresponding required named parameter in the other function.
     String& param_name = other_param_name;
@@ -18266,7 +18266,7 @@ bool Instance::IsAssignableTo(
   // In weak mode type casts, whether in legacy or opted-in libraries, the null
   // instance is detected and handled in inlined code and therefore cannot be
   // encountered here as a Dart null receiver.
-  ASSERT(Isolate::Current()->null_safety() || !IsNull());
+  ASSERT(Isolate::Current()->use_strict_null_safety_checks() || !IsNull());
   // In strong mode, compute NNBD_SUBTYPE(runtimeType, other).
   // In weak mode, compute LEGACY_SUBTYPE(runtimeType, other).
   return RuntimeTypeIsSubtypeOf(other, other_instantiator_type_arguments,
@@ -18317,7 +18317,7 @@ bool Instance::NullIsAssignableTo(const AbstractType& other) {
   Zone* zone = thread->zone();
 
   // In weak mode, Null is a bottom type (according to LEGACY_SUBTYPE).
-  if (!isolate->null_safety()) {
+  if (!isolate->use_strict_null_safety_checks()) {
     return true;
   }
   // "Left Null" rule: null is assignable when destination type is either
@@ -18348,7 +18348,7 @@ bool Instance::RuntimeTypeIsSubtypeOf(
   Isolate* isolate = thread->isolate();
   Zone* zone = thread->zone();
   // In weak testing mode, Null type is a subtype of any type.
-  if (IsNull() && !isolate->null_safety()) {
+  if (IsNull() && !isolate->use_strict_null_safety_checks()) {
     return true;
   }
   const Class& cls = Class::Handle(zone, clazz());
@@ -18413,7 +18413,7 @@ bool Instance::RuntimeTypeIsSubtypeOf(
     return false;
   }
   if (IsNull()) {
-    ASSERT(isolate->null_safety());
+    ASSERT(isolate->use_strict_null_safety_checks());
     if (instantiated_other.IsNullType()) {
       return true;
     }
@@ -19139,7 +19139,8 @@ bool AbstractType::IsTopTypeForSubtyping() const {
   if (cid == kInstanceCid) {  // Object type.
     // NNBD weak mode uses LEGACY_SUBTYPE for assignability / 'as' tests,
     // and non-nullable Object is a top type according to LEGACY_SUBTYPE.
-    return !IsNonNullable() || !Isolate::Current()->null_safety();
+    return !IsNonNullable() ||
+           !Isolate::Current()->use_strict_null_safety_checks();
   }
   if (cid == kFutureOrCid) {
     // FutureOr<T> where T is a top type behaves as a top type.
@@ -19298,7 +19299,8 @@ bool AbstractType::IsSubtypeOf(const AbstractType& other,
   const bool other_is_dart_function_type = other.IsDartFunctionType();
   if (other_is_dart_function_type || other.IsFunctionType()) {
     if (IsFunctionType()) {
-      if (isolate->null_safety() && IsNullable() && other.IsNonNullable()) {
+      if (isolate->use_strict_null_safety_checks() && IsNullable() &&
+          other.IsNonNullable()) {
         return false;
       }
       if (other_is_dart_function_type) {
@@ -19707,7 +19709,7 @@ bool Type::IsEquivalent(const Instance& other,
   Isolate* isolate = thread->isolate();
   Zone* zone = thread->zone();
   if (kind == TypeEquality::kInSubtypeTest) {
-    if (isolate->null_safety() &&
+    if (isolate->use_strict_null_safety_checks() &&
         this_type_nullability == Nullability::kNullable &&
         other_type_nullability == Nullability::kNonNullable) {
       return false;
@@ -20475,7 +20477,7 @@ bool TypeParameter::IsEquivalent(const Instance& other,
   Nullability this_type_param_nullability = nullability();
   Nullability other_type_param_nullability = other_type_param.nullability();
   if (kind == TypeEquality::kInSubtypeTest) {
-    if (Isolate::Current()->null_safety() &&
+    if (Isolate::Current()->use_strict_null_safety_checks() &&
         (this_type_param_nullability == Nullability::kNullable) &&
         (other_type_param_nullability == Nullability::kNonNullable)) {
       return false;

runtime/vm/runtime_entry.cc

@@ -808,7 +808,7 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) {
 
   // These are guaranteed on the calling side.
   ASSERT(!dst_type.IsDynamicType());
-  ASSERT(!src_instance.IsNull() || isolate->null_safety());
+  ASSERT(!src_instance.IsNull() || isolate->use_strict_null_safety_checks());
 
   const bool is_instance_of = src_instance.IsAssignableTo(
       dst_type, instantiator_type_arguments, function_type_arguments);

runtime/vm/type_testing_stubs.cc

@@ -95,8 +95,9 @@ CodePtr TypeTestingStubGenerator::DefaultCodeForType(
   auto isolate = Isolate::Current();
 
   if (type.IsTypeRef()) {
-    return isolate->null_safety() ? StubCode::DefaultTypeTest().raw()
-                                  : StubCode::DefaultNullableTypeTest().raw();
+    return isolate->use_strict_null_safety_checks()
+               ? StubCode::DefaultTypeTest().raw()
+               : StubCode::DefaultNullableTypeTest().raw();
   }
 
   // During bootstrapping we have no access to stubs yet, so we'll just return
@@ -281,7 +282,8 @@ void TypeTestingStubGenerator::BuildOptimizedTypeTestStubFastCases(
     __ Bind(&continue_checking);
 
   } else if (type.IsObjectType()) {
-    ASSERT(type.IsNonNullable() && Isolate::Current()->null_safety());
+    ASSERT(type.IsNonNullable() &&
+           Isolate::Current()->use_strict_null_safety_checks());
     compiler::Label continue_checking;
     __ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
     __ BranchIf(EQUAL, &continue_checking);
@@ -493,8 +495,8 @@ void TypeTestingStubGenerator::BuildOptimizedTypeArgumentValueCheck(
     // Weak NNBD mode uses LEGACY_SUBTYPE which ignores nullability.
     // We don't need to check nullability of LHS for nullable and legacy RHS
     // ("Right Legacy", "Right Nullable" rules).
-    if (Isolate::Current()->null_safety() && !type_arg.IsNullable() &&
-        !type_arg.IsLegacy()) {
+    if (Isolate::Current()->use_strict_null_safety_checks() &&
+        !type_arg.IsNullable() && !type_arg.IsLegacy()) {
       // Nullable type is not a subtype of non-nullable type.
       // TODO(dartbug.com/40736): Allocate a register for instance type argument
       // and avoid reloading it.