From 55cc495451e4ecff6fb74ba7b0ee692fada95caa Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Tue, 25 Feb 2020 20:05:39 +0000 Subject: [PATCH] [vm/nnbd] Rename --strong-non-nullable-type-checks to --null-safety and pass it to CFE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: https://github.com/dart-lang/sdk/issues/38845 Change-Id: I9baeb4a384a6b2de79df9f97e82c127ae4f611c0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137120 Commit-Queue: Alexander Markov Reviewed-by: Régis Crelier Reviewed-by: Liam Appelbe --- pkg/vm/bin/kernel_service.dart | 17 +++++++++++++++-- runtime/vm/compiler/backend/type_propagator.cc | 2 +- runtime/vm/flag_list.h | 4 ++-- runtime/vm/isolate_reload.cc | 2 +- runtime/vm/kernel_isolate.cc | 10 +++++----- runtime/vm/object.cc | 16 +++++++--------- runtime/vm/object.h | 2 +- runtime/vm/runtime_entry.cc | 2 +- runtime/vm/type_testing_stubs.cc | 8 +++----- tools/bots/test_matrix.json | 6 +++--- 10 files changed, 39 insertions(+), 30 deletions(-) diff --git a/pkg/vm/bin/kernel_service.dart b/pkg/vm/bin/kernel_service.dart index 99141fa3509..fe0e4da7665 100644 --- a/pkg/vm/bin/kernel_service.dart +++ b/pkg/vm/bin/kernel_service.dart @@ -80,6 +80,7 @@ abstract class Compiler { final Uri platformKernelPath; final bool suppressWarnings; final bool enableAsserts; + final bool nullSafety; final List experimentalFlags; final bool bytecode; final String packageConfig; @@ -96,6 +97,7 @@ abstract class Compiler { Compiler(this.isolateId, this.fileSystem, this.platformKernelPath, {this.suppressWarnings: false, this.enableAsserts: false, + this.nullSafety: false, this.experimentalFlags: null, this.bytecode: false, this.supportCodeCoverage: false, @@ -135,6 +137,7 @@ abstract class Compiler { onError: (msg) => errors.add(msg)) ..environmentDefines = new EnvironmentMap() ..enableAsserts = enableAsserts + ..nnbdMode = nullSafety ? NnbdMode.Strong : NnbdMode.Weak ..onDiagnostic = (DiagnosticMessage message) { bool printMessage; switch (message.severity) { @@ -281,12 +284,14 @@ class IncrementalCompilerWrapper extends Compiler { int isolateId, FileSystem fileSystem, Uri platformKernelPath, {bool suppressWarnings: false, bool enableAsserts: false, + bool nullSafety: false, List experimentalFlags: null, bool bytecode: false, String packageConfig: null}) : super(isolateId, fileSystem, platformKernelPath, suppressWarnings: suppressWarnings, enableAsserts: enableAsserts, + nullSafety: nullSafety, experimentalFlags: experimentalFlags, bytecode: bytecode, supportHotReload: true, @@ -312,6 +317,7 @@ class IncrementalCompilerWrapper extends Compiler { isolateId, fileSystem, platformKernelPath, suppressWarnings: suppressWarnings, enableAsserts: enableAsserts, + nullSafety: nullSafety, experimentalFlags: experimentalFlags, bytecode: bytecode, packageConfig: packageConfig); @@ -342,12 +348,14 @@ class SingleShotCompilerWrapper extends Compiler { {this.requireMain: false, bool suppressWarnings: false, bool enableAsserts: false, + bool nullSafety: false, List experimentalFlags: null, bool bytecode: false, String packageConfig: null}) : super(isolateId, fileSystem, platformKernelPath, suppressWarnings: suppressWarnings, enableAsserts: enableAsserts, + nullSafety: nullSafety, experimentalFlags: experimentalFlags, bytecode: bytecode, packageConfig: packageConfig); @@ -380,6 +388,7 @@ Future lookupOrBuildNewIncrementalCompiler(int isolateId, List sourceFiles, Uri platformKernelPath, List platformKernel, {bool suppressWarnings: false, bool enableAsserts: false, + bool nullSafety: false, List experimentalFlags: null, bool bytecode: false, String packageConfig: null, @@ -411,6 +420,7 @@ Future lookupOrBuildNewIncrementalCompiler(int isolateId, isolateId, fileSystem, platformKernelPath, suppressWarnings: suppressWarnings, enableAsserts: enableAsserts, + nullSafety: nullSafety, experimentalFlags: experimentalFlags, bytecode: bytecode, packageConfig: packageConfig); @@ -613,7 +623,8 @@ Future _processLoadRequest(request) async { final String inputFileUri = request[2]; final Uri script = inputFileUri != null ? Uri.base.resolve(inputFileUri) : null; - bool incremental = request[4]; + final bool incremental = request[4]; + final bool nullSafety = request[5]; final int isolateId = request[6]; final List sourceFiles = request[7]; final bool suppressWarnings = request[8]; @@ -680,6 +691,7 @@ Future _processLoadRequest(request) async { isolateId, sourceFiles, platformKernelPath, platformKernel, suppressWarnings: suppressWarnings, enableAsserts: enableAsserts, + nullSafety: nullSafety, experimentalFlags: experimentalFlags, bytecode: bytecode, packageConfig: packageConfig, @@ -693,6 +705,7 @@ Future _processLoadRequest(request) async { requireMain: false, suppressWarnings: suppressWarnings, enableAsserts: enableAsserts, + nullSafety: nullSafety, experimentalFlags: experimentalFlags, bytecode: bytecode, packageConfig: packageConfig); @@ -841,7 +854,7 @@ Future trainInternal( scriptUri, platformKernelPath, false /* incremental */, - true /* strong */, + false /* null safety */, 1 /* isolateId chosen randomly */, [] /* source files */, false /* suppress warnings */, diff --git a/runtime/vm/compiler/backend/type_propagator.cc b/runtime/vm/compiler/backend/type_propagator.cc index 523b0391247..fb8bab5efd3 100644 --- a/runtime/vm/compiler/backend/type_propagator.cc +++ b/runtime/vm/compiler/backend/type_propagator.cc @@ -826,7 +826,7 @@ bool CompileType::IsAssignableTo(NNBDMode mode, const AbstractType& other) { const AbstractType& compile_type = *ToAbstractType(); if (compile_type.IsNullType()) { - if (!FLAG_strong_non_nullable_type_checks) { + if (!FLAG_null_safety) { // In weak mode, 'null' is assignable to any type. return true; } diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h index 87d1f0a4b1f..68a9220a651 100644 --- a/runtime/vm/flag_list.h +++ b/runtime/vm/flag_list.h @@ -189,8 +189,8 @@ constexpr bool kDartUseBackgroundCompilation = true; P(reorder_basic_blocks, bool, true, "Reorder basic blocks") \ C(stress_async_stacks, false, false, bool, false, \ "Stress test async stack traces") \ - P(strong_non_nullable_type_checks, bool, false, \ - "Enable strong non-nullable type checking mode.") \ + P(null_safety, bool, false, \ + "Respect the nullability of types in casts and instance checks.") \ P(use_table_dispatch, bool, true, "Enable dispatch table based calls.") \ P(enable_isolate_groups, bool, false, "Enable isolate group support.") \ P(show_invisible_frames, bool, false, \ diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 5d973a14bc8..2bc33ff8f05 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -2074,7 +2074,7 @@ class FieldInvalidator { DART_FORCE_INLINE void CheckValueType(const Instance& value, const Field& field) { - if (!FLAG_strong_non_nullable_type_checks && value.IsNull()) { + if (!FLAG_null_safety && value.IsNull()) { return; } type_ = field.type(); diff --git a/runtime/vm/kernel_isolate.cc b/runtime/vm/kernel_isolate.cc index c813c94aad2..46e3efce326 100644 --- a/runtime/vm/kernel_isolate.cc +++ b/runtime/vm/kernel_isolate.cc @@ -613,10 +613,6 @@ class KernelCompilationRequest : public ValueObject { dart_incremental.type = Dart_CObject_kBool; dart_incremental.value.as_bool = incremental_compile; - Dart_CObject dart_strong; - dart_strong.type = Dart_CObject_kBool; - dart_strong.value.as_bool = true; - // TODO(aam): Assert that isolate exists once we move CompileAndReadScript // compilation logic out of CreateIsolateAndSetupHelper and into // IsolateSetupHelper in main.cc. @@ -644,6 +640,10 @@ class KernelCompilationRequest : public ValueObject { enable_asserts.value.as_bool = isolate != NULL ? isolate->asserts() : FLAG_enable_asserts; + Dart_CObject null_safety; + null_safety.type = Dart_CObject_kBool; + null_safety.value.as_bool = FLAG_null_safety; + intptr_t num_experimental_flags = experimental_flags->length(); Dart_CObject** experimental_flags_array = new Dart_CObject*[num_experimental_flags]; @@ -702,7 +702,7 @@ class KernelCompilationRequest : public ValueObject { &uri, &dart_platform_kernel, &dart_incremental, - &dart_strong, + &null_safety, &isolate_id, &files, &suppress_warnings, diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index f855b972660..2f0bf4370da 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -17484,7 +17484,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(FLAG_strong_non_nullable_type_checks || !IsNull()); + ASSERT(FLAG_null_safety || !IsNull()); // In strong mode, compute NNBD_SUBTYPE(runtimeType, other). // In weak mode, compute LEGACY_SUBTYPE(runtimeType, other). return RuntimeTypeIsSubtypeOf(mode, other, other_instantiator_type_arguments, @@ -17543,7 +17543,7 @@ bool Instance::RuntimeTypeIsSubtypeOf( return true; } // In weak testing mode, Null type is a subtype of any type. - if (IsNull() && !FLAG_strong_non_nullable_type_checks) { + if (IsNull() && !FLAG_null_safety) { return true; } Thread* thread = Thread::Current(); @@ -17609,7 +17609,7 @@ bool Instance::RuntimeTypeIsSubtypeOf( return false; } if (IsNull()) { - ASSERT(FLAG_strong_non_nullable_type_checks); + ASSERT(FLAG_null_safety); if (instantiated_other.IsNullType()) { return true; } @@ -18340,7 +18340,7 @@ bool AbstractType::IsSubtypeOf(NNBDMode mode, // Only Never? remains, which maps to Null regardless of weak/strong mode. if (IsNullType() || IsNeverType()) { // In weak mode, Null is a bottom type. - if (!FLAG_strong_non_nullable_type_checks) { + if (!FLAG_null_safety) { return true; } const AbstractType& unwrapped_other = @@ -18420,8 +18420,7 @@ bool AbstractType::IsSubtypeOf(NNBDMode mode, } return false; } - if (FLAG_strong_non_nullable_type_checks && IsNullable() && - other.IsNonNullable()) { + if (FLAG_null_safety && IsNullable() && other.IsNonNullable()) { return false; } return Class::IsSubtypeOf( @@ -18789,8 +18788,7 @@ bool Type::IsEquivalent(const Instance& other, Nullability this_type_nullability = nullability(); Nullability other_type_nullability = other_type.nullability(); if (kind == TypeEquality::kInSubtypeTest) { - if (FLAG_strong_non_nullable_type_checks && - this_type_nullability == Nullability::kNullable && + if (FLAG_null_safety && this_type_nullability == Nullability::kNullable && other_type_nullability == Nullability::kNonNullable) { return false; } @@ -19509,7 +19507,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 (FLAG_strong_non_nullable_type_checks && + if (FLAG_null_safety && this_type_param_nullability == Nullability::kNullable && (other_type_param_nullability == Nullability::kNonNullable || other_type_param_nullability == Nullability::kUndetermined)) { diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 89a21fe6b32..1758971e694 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -892,7 +892,7 @@ enum class TypeEquality { // tests. The mode reflects the opted-in status of the library performing type // reification and/or subtype tests. // Note that the weak or strong testing mode is not reflected in NNBDMode, but -// imposed globally by the value of FLAG_strong_non_nullable_type_checks. +// imposed globally by the value of FLAG_null_safety. enum class NNBDMode { // Status of the library: kLegacyLib = 0, // Library is legacy. diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 627ea1c8627..aacb48ef1da 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -821,7 +821,7 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 8) { ASSERT(!dst_type.IsDynamicType()); // No need to check assignment. // A null instance is already detected and allowed in inlined code, unless // strong checking is enabled. - ASSERT(!src_instance.IsNull() || FLAG_strong_non_nullable_type_checks); + ASSERT(!src_instance.IsNull() || FLAG_null_safety); const bool is_instance_of = src_instance.IsAssignableTo( nnbd_mode, dst_type, instantiator_type_arguments, function_type_arguments); diff --git a/runtime/vm/type_testing_stubs.cc b/runtime/vm/type_testing_stubs.cc index 825f9f90880..bbe8828a63b 100644 --- a/runtime/vm/type_testing_stubs.cc +++ b/runtime/vm/type_testing_stubs.cc @@ -105,17 +105,15 @@ RawCode* TypeTestingStubGenerator::DefaultCodeForType( } if (cid == kDynamicCid || cid == kVoidCid || - (cid == kInstanceCid && - (!FLAG_strong_non_nullable_type_checks || !type.IsNonNullable()))) { + (cid == kInstanceCid && (!FLAG_null_safety || !type.IsNonNullable()))) { return StubCode::TopTypeTypeTest().raw(); } if (type.IsType() || type.IsTypeParameter()) { // TODO(dartbug.com/38845): Add support for specialized TTS for // nullable and non-nullable types in NNBD strong mode. - const bool should_specialize = - !FLAG_precompiled_mode && lazy_specialize && - (type.IsLegacy() || !FLAG_strong_non_nullable_type_checks); + const bool should_specialize = !FLAG_precompiled_mode && lazy_specialize && + (type.IsLegacy() || !FLAG_null_safety); return should_specialize ? StubCode::LazySpecializeTypeTest().raw() : StubCode::DefaultTypeTest().raw(); } diff --git a/tools/bots/test_matrix.json b/tools/bots/test_matrix.json index cc1c0f5365a..7ed633c320a 100644 --- a/tools/bots/test_matrix.json +++ b/tools/bots/test_matrix.json @@ -717,7 +717,7 @@ "--null-safety" ], "vm-options": [ - "--strong-non-nullable-type-checks" + "--null-safety" ] } }, @@ -730,7 +730,7 @@ "--null-safety" ], "vm-options": [ - "--strong-non-nullable-type-checks" + "--null-safety" ] } }, @@ -744,7 +744,7 @@ "--null-safety" ], "vm-options": [ - "--strong-non-nullable-type-checks" + "--null-safety" ] } },