Skip to content

Commit

Permalink
[vm] Rename --null-safety option to --sound-null-safety
Browse files Browse the repository at this point in the history
Deprecated option --null-safety still remains in order to allow
graceful migration.

Fixes #41853

Change-Id: Ie47b1bebc9dd6532658a60743ecb85dc7fdc108c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153660
Reviewed-by: Siva Annamalai <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
  • Loading branch information
alexmarkov authored and [email protected] committed Jul 9, 2020
1 parent 7a87766 commit 6f6b1f8
Show file tree
Hide file tree
Showing 24 changed files with 97 additions and 83 deletions.
2 changes: 1 addition & 1 deletion pkg/front_end/test/fasta/testing/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ class Run extends Step<ComponentResult, int, FastaContext> {
if (experimentalFlags[ExperimentalFlag.nonNullable]) {
args.add("--enable-experiment=non-nullable");
if (!context.weak) {
args.add("--null-safety");
args.add("--sound-null-safety");
}
}
args.add(generated.path);
Expand Down
14 changes: 9 additions & 5 deletions pkg/frontend_server/lib/frontend_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,12 @@ ArgParser argParser = ArgParser(allowTrailingOptions: true)
help: 'Include only bytecode into the output file', defaultsTo: true)
..addFlag('enable-asserts',
help: 'Whether asserts will be enabled.', defaultsTo: false)
..addFlag('sound-null-safety',
help: 'Respect the nullability of types at runtime.', defaultsTo: null)
// TODO(alexmarkov) Remove obsolete --null-safety option.
..addFlag('null-safety',
help:
'Respect the nullability of types at runtime in casts and instance checks.',
help: 'Deprecated. Please use --sound-null-safety instead.',
hide: true,
defaultsTo: null)
..addMultiOption('enable-experiment',
help: 'Comma separated list of experimental features, eg set-literals.',
Expand Down Expand Up @@ -407,6 +410,8 @@ class FrontendCompiler implements CompilerInterface {
final String platformKernelDill =
options['platform'] ?? 'platform_strong.dill';
final String packagesOption = _options['packages'];
final bool nullSafety =
_options['sound-null-safety'] ?? _options['null-safety'];
final CompilerOptions compilerOptions = CompilerOptions()
..sdkRoot = sdkRoot
..fileSystem = _fileSystem
Expand All @@ -418,8 +423,7 @@ class FrontendCompiler implements CompilerInterface {
..experimentalFlags = parseExperimentalFlags(
parseExperimentalArguments(options['enable-experiment']),
onError: (msg) => errors.add(msg))
..nnbdMode =
(options['null-safety'] == true) ? NnbdMode.Strong : NnbdMode.Weak
..nnbdMode = (nullSafety == true) ? NnbdMode.Strong : NnbdMode.Weak
..onDiagnostic = _onDiagnostic;

if (options.wasParsed('libraries-spec')) {
Expand Down Expand Up @@ -469,7 +473,7 @@ class FrontendCompiler implements CompilerInterface {
}
}

if (options['null-safety'] == null &&
if (nullSafety == null &&
compilerOptions.experimentalFlags[ExperimentalFlag.nonNullable]) {
await autoDetectNullSafetyMode(_mainSource, compilerOptions);
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/vm/bin/kernel_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ bool allowDartInternalImport = false;
// Null Safety command line options
//
// Note: The values of these constants must match the
// values of flag null_safety in ../../../../runtime/vm/flag_list.h.
// 0 - No null_safety option specified on the command line.
// 1 - '--no-null-safety' specified on the command line.
// 2 - '--null-safety' option specified on the command line.
// values of flag sound_null_safety in ../../../../runtime/vm/flag_list.h.
// 0 - No --[no-]sound-null-safety option specified on the command line.
// 1 - '--no-sound-null-safety' specified on the command line.
// 2 - '--sound-null-safety' option specified on the command line.
const int kNullSafetyOptionUnspecified = 0;
const int kNullSafetyOptionWeak = 1;
const int kNullSafetyOptionStrong = 2;
Expand Down
10 changes: 7 additions & 3 deletions pkg/vm/lib/kernel_front_end.dart
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,12 @@ void declareCompilerOptions(ArgParser args) {
help: 'The values for the environment constants (e.g. -Dkey=value).');
args.addFlag('enable-asserts',
help: 'Whether asserts will be enabled.', defaultsTo: false);
args.addFlag('sound-null-safety',
help: 'Respect the nullability of types at runtime.', defaultsTo: null);
// TODO(alexmarkov) Remove obsolete --null-safety option.
args.addFlag('null-safety',
help:
'Respect the nullability of types at runtime in casts and instance checks.',
help: 'Deprecated. Please use --sound-null-safety instead.',
hide: true,
defaultsTo: null);
args.addFlag('split-output-by-packages',
help:
Expand Down Expand Up @@ -189,7 +192,8 @@ Future<int> runCompiler(ArgResults options, String usage) async {
final bool genBytecode = options['gen-bytecode'];
final bool dropAST = options['drop-ast'];
final bool enableAsserts = options['enable-asserts'];
final bool nullSafety = options['null-safety'];
final bool nullSafety =
options['sound-null-safety'] ?? options['null-safety'];
final bool useProtobufTreeShaker = options['protobuf-tree-shaker'];
final bool useProtobufTreeShakerV2 = options['protobuf-tree-shaker-v2'];
final bool splitOutputByPackages = options['split-output-by-packages'];
Expand Down
4 changes: 2 additions & 2 deletions pkg/vm/tool/precompiler2
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ for arg in "$@"; do
PACKAGES="$arg"
;;
--enable-asserts | \
--null-safety | \
--no-null-safety | \
--sound-null-safety | \
--no-sound-null-safety | \
--enable-experiment=*)
GEN_KERNEL_OPTIONS+=("$arg")
OPTIONS+=("$arg")
Expand Down
2 changes: 1 addition & 1 deletion runtime/bin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ gen_snapshot_action("generate_snapshot_bin") {
]
args = [
"--enable-experiment=non-nullable",
"--null-safety",
"--sound-null-safety",
"--deterministic",
"--snapshot_kind=" + dart_core_snapshot_kind,
"--vm_snapshot_data=" + rebase_path(vm_snapshot_data, root_build_dir),
Expand Down
4 changes: 2 additions & 2 deletions runtime/tests/vm/dart/run_appended_aot_snapshot_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ Future<void> main(List<String> args) async {
final extraGenKernelOptions = Platform.executableArguments
.where((arg) =>
arg.startsWith('--enable-experiment=') ||
arg == '--null-safety' ||
arg == '--no-null-safety')
arg == '--sound-null-safety' ||
arg == '--no-sound-null-safety')
.toList();

{
Expand Down
4 changes: 2 additions & 2 deletions runtime/tests/vm/dart/split_aot_kernel_generation_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ Future<void> runSplitAOTKernelGenerationTest(Uri testScriptUri) async {
final extraGenKernelOptions = Platform.executableArguments
.where((arg) =>
arg.startsWith('--enable-experiment=') ||
arg == '--null-safety' ||
arg == '--no-null-safety')
arg == '--sound-null-safety' ||
arg == '--no-sound-null-safety')
.toList();

await runGenKernel('BUILD INTERMEDIATE DILL FILE', [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ runTest(String script, String output, String temp) async {
File.fromUri(Platform.script.resolve(script)).copySync(scriptInTemp);

// Do not add Platform.executableArguments into arguments to avoid passing
// --null-safety / --no-null-safety arguments.
// --sound-null-safety / --no-sound-null-safety arguments.
final result = await runBinary("RUN $script", Platform.executable, [
'--enable-experiment=non-nullable',
'--deterministic',
Expand Down
4 changes: 2 additions & 2 deletions runtime/tests/vm/dart/v8_snapshot_profile_writer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ main() async {
platformDill,
...Platform.executableArguments.where((arg) =>
arg.startsWith('--enable-experiment=') ||
arg == '--null-safety' ||
arg == '--no-null-safety'),
arg == '--sound-null-safety' ||
arg == '--no-sound-null-safety'),
'-o',
dillPath,
_thisTestPath
Expand Down
4 changes: 2 additions & 2 deletions runtime/tests/vm/dart_2/split_aot_kernel_generation_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ Future<void> runSplitAOTKernelGenerationTest(Uri testScriptUri) async {
final extraGenKernelOptions = Platform.executableArguments
.where((arg) =>
arg.startsWith('--enable-experiment=') ||
arg == '--null-safety' ||
arg == '--no-null-safety')
arg == '--sound-null-safety' ||
arg == '--no-sound-null-safety')
.toList();

await runGenKernel('BUILD INTERMEDIATE DILL FILE', [
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/benchmark_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ static Dart_NativeFunction NativeResolver(Dart_Handle name,
// Measure compile of all kernel Service(CFE) functions.
//
BENCHMARK(KernelServiceCompileAll) {
if (FLAG_null_safety == kNullSafetyOptionStrong) {
if (FLAG_sound_null_safety == kNullSafetyOptionStrong) {
// TODO(bkonyi): remove this check when we build the CFE in strong mode.
return;
}
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/clustered_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6785,14 +6785,14 @@ char* SnapshotHeaderReader::InitializeGlobalVMFlagsFromSnapshot(
#undef SET_FLAG

#if defined(DART_PRECOMPILED_RUNTIME)
if (FLAG_null_safety == kNullSafetyOptionUnspecified) {
if (FLAG_sound_null_safety == kNullSafetyOptionUnspecified) {
if (strncmp(cursor, "null-safety", end - cursor) == 0) {
FLAG_null_safety = kNullSafetyOptionStrong;
FLAG_sound_null_safety = kNullSafetyOptionStrong;
cursor = end;
continue;
}
if (strncmp(cursor, "no-null-safety", end - cursor) == 0) {
FLAG_null_safety = kNullSafetyOptionWeak;
FLAG_sound_null_safety = kNullSafetyOptionWeak;
cursor = end;
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/dart.cc
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ bool Dart::DetectNullSafety(const char* script_uri,
// generating the kernel file
// - if loading from an appJIT, based on the mode used
// when generating the snapshot.
ASSERT(FLAG_null_safety == kNullSafetyOptionUnspecified);
ASSERT(FLAG_sound_null_safety == kNullSafetyOptionUnspecified);

// If snapshot is an appJIT/AOT snapshot we will figure out the mode by
// sniffing the feature string in the snapshot.
Expand Down Expand Up @@ -1025,7 +1025,7 @@ const char* Dart::FeaturesString(Isolate* isolate,
buffer.AddString(" no-null-safety");
}
} else {
if (FLAG_null_safety == kNullSafetyOptionStrong) {
if (FLAG_sound_null_safety == kNullSafetyOptionStrong) {
buffer.AddString(" null-safety");
} else {
buffer.AddString(" no-null-safety");
Expand Down
12 changes: 6 additions & 6 deletions runtime/vm/dart_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3051,7 +3051,7 @@ DART_EXPORT Dart_Handle Dart_NewListOf(Dart_CoreType_Id element_type_id,
DARTSCOPE(Thread::Current());
if (T->isolate()->null_safety() && element_type_id != Dart_CoreType_Dynamic) {
return Api::NewError(
"Cannot use legacy types with --null-safety enabled. "
"Cannot use legacy types with --sound-null-safety enabled. "
"Use Dart_NewListOfType or Dart_NewListOfTypeFilled instead.");
}
CHECK_LENGTH(length, Array::kMaxElements);
Expand Down Expand Up @@ -5599,7 +5599,7 @@ DART_EXPORT Dart_Handle Dart_GetType(Dart_Handle library,
Dart_Handle* type_arguments) {
if (Thread::Current()->isolate()->null_safety()) {
return Api::NewError(
"Cannot use legacy types with --null-safety enabled. "
"Cannot use legacy types with --sound-null-safety enabled. "
"Use Dart_GetNullableType or Dart_GetNonNullableType instead.");
}
return GetTypeCommon(library, class_name, number_of_type_arguments,
Expand Down Expand Up @@ -6033,16 +6033,16 @@ DART_EXPORT bool Dart_DetectNullSafety(const char* script_uri,
const uint8_t* kernel_buffer,
intptr_t kernel_buffer_size) {
#if defined(DART_PRECOMPILED_RUNTIME)
ASSERT(FLAG_null_safety != kNullSafetyOptionUnspecified);
return (FLAG_null_safety == kNullSafetyOptionStrong);
ASSERT(FLAG_sound_null_safety != kNullSafetyOptionUnspecified);
return (FLAG_sound_null_safety == kNullSafetyOptionStrong);
#else
bool null_safety;
if (FLAG_null_safety == kNullSafetyOptionUnspecified) {
if (FLAG_sound_null_safety == kNullSafetyOptionUnspecified) {
null_safety = Dart::DetectNullSafety(
script_uri, snapshot_data, snapshot_instructions, kernel_buffer,
kernel_buffer_size, package_config, original_working_directory);
} else {
null_safety = (FLAG_null_safety == kNullSafetyOptionStrong);
null_safety = (FLAG_sound_null_safety == kNullSafetyOptionStrong);
}
return null_safety;
#endif // defined(DART_PRECOMPILED_RUNTIME)
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/dart_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5335,7 +5335,7 @@ TEST_CASE(DartAPI_NewListOf) {
EXPECT_STREQ("null", str);
} else {
EXPECT_ERROR(string_list,
"Cannot use legacy types with --null-safety enabled. "
"Cannot use legacy types with --sound-null-safety enabled. "
"Use Dart_NewListOfType or Dart_NewListOfTypeFilled instead.");
}

Expand All @@ -5356,7 +5356,7 @@ TEST_CASE(DartAPI_NewListOf) {
EXPECT_STREQ("null", str);
} else {
EXPECT_ERROR(int_list,
"Cannot use legacy types with --null-safety enabled. "
"Cannot use legacy types with --sound-null-safety enabled. "
"Use Dart_NewListOfType or Dart_NewListOfTypeFilled instead.");
}
}
Expand Down Expand Up @@ -6222,7 +6222,7 @@ TEST_CASE(DartAPI_TypeToNullability) {
if (Dart_IsError(type)) {
EXPECT_ERROR(
type,
"Cannot use legacy types with --null-safety enabled. "
"Cannot use legacy types with --sound-null-safety enabled. "
"Use Dart_GetNullableType or Dart_GetNonNullableType instead.");

nonNullableType = Dart_GetNonNullableType(lib, name, 0, nullptr);
Expand Down
19 changes: 12 additions & 7 deletions runtime/vm/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,20 @@ DEFINE_FLAG_HANDLER(DeterministicModeHandler,
deterministic,
"Enable deterministic mode.");

int FLAG_null_safety = kNullSafetyOptionUnspecified;
static void NullSafetyHandler(bool value) {
FLAG_null_safety = value ? kNullSafetyOptionStrong : kNullSafetyOptionWeak;
int FLAG_sound_null_safety = kNullSafetyOptionUnspecified;
static void SoundNullSafetyHandler(bool value) {
FLAG_sound_null_safety =
value ? kNullSafetyOptionStrong : kNullSafetyOptionWeak;
}

DEFINE_FLAG_HANDLER(
NullSafetyHandler,
null_safety,
"Respect the nullability of types in casts and instance checks.");
DEFINE_FLAG_HANDLER(SoundNullSafetyHandler,
sound_null_safety,
"Respect the nullability of types at runtime.");

// TODO(alexmarkov) Remove obsolete --null-safety option.
DEFINE_FLAG_HANDLER(SoundNullSafetyHandler,
null_safety,
"Respect the nullability of types at runtime.");

DEFINE_FLAG(bool,
disable_thread_pool_limit,
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class WeakTable;
constexpr int kNullSafetyOptionUnspecified = 0;
constexpr int kNullSafetyOptionWeak = 1;
constexpr int kNullSafetyOptionStrong = 2;
extern int FLAG_null_safety;
extern int FLAG_sound_null_safety;

class PendingLazyDeopt {
public:
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/kernel_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ class KernelCompilationRequest : public ValueObject {
null_safety.value.as_int32 =
(isolate != NULL) ? (isolate->null_safety() ? kNullSafetyOptionStrong
: kNullSafetyOptionWeak)
: FLAG_null_safety;
: FLAG_sound_null_safety;

intptr_t num_experimental_flags = experimental_flags->length();
Dart_CObject** experimental_flags_array =
Expand Down
10 changes: 6 additions & 4 deletions runtime/vm/kernel_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1045,15 +1045,17 @@ LibraryPtr KernelLoader::LoadLibrary(intptr_t index) {
library_helper.GetNonNullableByDefaultCompiledMode();
if (!I->null_safety() && mode == NNBDCompiledMode::kStrong) {
H.ReportError(
"Library '%s' was compiled with null safety (in strong mode) and it "
"requires --null-safety option at runtime",
"Library '%s' was compiled with sound null safety (in strong mode) and "
"it "
"requires --sound-null-safety option at runtime",
String::Handle(library.url()).ToCString());
}
if (I->null_safety() && (mode == NNBDCompiledMode::kWeak ||
mode == NNBDCompiledMode::kDisabled)) {
H.ReportError(
"Library '%s' was compiled without null safety (in weak mode) and it "
"cannot be used with --null-safety at runtime",
"Library '%s' was compiled without sound null safety (in weak mode) "
"and it "
"cannot be used with --sound-null-safety at runtime",
String::Handle(library.url()).ToCString());
}
library.set_nnbd_compiled_mode(mode);
Expand Down
3 changes: 1 addition & 2 deletions runtime/vm/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,7 @@ enum class TypeEquality {
};

// The NNBDMode reflects the opted-in status of libraries.
// Note that the weak or strong testing mode is not reflected in NNBDMode, but
// imposed globally by the value of --null-safety.
// Note that the weak or strong checking mode is not reflected in NNBDMode.
enum class NNBDMode {
// Status of the library:
kLegacyLib = 0, // Library is legacy.
Expand Down
4 changes: 2 additions & 2 deletions tests/corelib/list_removeat_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ void testModifiableList(l1) {
Expect.throwsRangeError(() => l1.removeAt(-1), "negative");
Expect.throwsRangeError(() => l1.removeAt(5), "too large");
Expect.throws(() => l1.removeAt(null),
// With --null-safety a TypeError is thrown
// With --no-null-safety an ArgumentError is thrown
// With sound null safety a TypeError is thrown.
// Without sound null safety an ArgumentError is thrown.
(e) => e is TypeError || e is ArgumentError, "is null");

Expect.equals(2, l1.removeAt(2), "l1-remove2");
Expand Down
Loading

0 comments on commit 6f6b1f8

Please sign in to comment.