From e8906e65e72522e5e574cc772ff0c159dafb5f60 Mon Sep 17 00:00:00 2001 From: Sigmund Cherem Date: Wed, 7 Dec 2022 04:02:13 +0000 Subject: [PATCH] [3.0 alpha][dart2js] No longer infer the null-safety mode from sources. This prepares dart2js to default to sound null safety in Dart 3.0. The compiler no longer infers the default mode based on the input program, instead uses sound null safety unless '--no-sound-null-safety' is specified. On a separate change, we expect to remove the option of disabling null safety. Note: this change should also be reflected in the CHANGELOG, but we've currently not included it to redeuce merge conflicts, as we plan to land multiple 3.0-alpha changes in a short window of time Change-Id: Id87498cc5976548ec89d1f36c60674b72406950a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/270860 Reviewed-by: Nate Biggs Commit-Queue: Siva Annamalai Reviewed-by: Mayank Patke --- pkg/compiler/lib/src/options.dart | 17 ++-- pkg/compiler/lib/src/phase/load_kernel.dart | 27 +++--- .../lib/src/util/memory_compiler.dart | 33 +++++-- .../test/end_to_end/dart2js_batch_test.dart | 5 +- .../test/end_to_end/modular_loader_test.dart | 3 +- .../test/end_to_end/output_type_test.dart | 2 + .../test/end_to_end/user_crash_test.dart | 3 +- .../test/inference/inference_test_helper.dart | 4 + .../serialization/on_disk_split_test.dart | 2 + .../test/sourcemaps/minified_names_test.dart | 1 + .../test/sourcemaps/stacktrace_test.dart | 1 + .../test/sourcemaps/stepping_test.dart | 1 + .../tool/modular_test_suite_helper.dart | 6 ++ pkg/dartdev/test/commands/compile_test.dart | 1 + tools/bots/test_matrix.json | 85 +++---------------- utils/compiler/BUILD.gn | 3 + 16 files changed, 82 insertions(+), 112 deletions(-) diff --git a/pkg/compiler/lib/src/options.dart b/pkg/compiler/lib/src/options.dart index c0abb97f659..36e3deffd11 100644 --- a/pkg/compiler/lib/src/options.dart +++ b/pkg/compiler/lib/src/options.dart @@ -10,7 +10,6 @@ import 'commandline_options.dart' show Flags; import 'util/util.dart'; enum NullSafetyMode { - unspecified, unsound, sound, } @@ -552,24 +551,20 @@ class CompilerOptions implements DiagnosticOptions { /// called. bool experimentCallInstrumentation = false; - /// When null-safety is enabled, whether the compiler should emit code with - /// unsound or sound semantics. - /// - /// If unspecified, the mode must be inferred from the entrypoint. + /// Whether the compiler should emit code with unsound or sound semantics. + /// Since Dart 3.0 this is no longer inferred from sources, but defaults to + /// sound semantics. /// /// This option should rarely need to be accessed directly. Consider using /// [useLegacySubtyping] instead. - NullSafetyMode nullSafetyMode = NullSafetyMode.unspecified; + NullSafetyMode nullSafetyMode = NullSafetyMode.sound; bool _soundNullSafety = false; bool _noSoundNullSafety = false; /// Whether to use legacy subtype semantics rather than null-safe semantics. - /// This is `true` if null-safety is disabled, i.e. all code is legacy code, - /// or if unsound null-safety semantics are being used, since we do not emit - /// warnings. + /// This is `true` if unsound null-safety semantics are being used, since + /// dart2js does not emit warnings for unsound null-safety. bool get useLegacySubtyping { - assert(nullSafetyMode != NullSafetyMode.unspecified, - "Null safety mode unspecified"); return nullSafetyMode == NullSafetyMode.unsound; } diff --git a/pkg/compiler/lib/src/phase/load_kernel.dart b/pkg/compiler/lib/src/phase/load_kernel.dart index d33642fa547..b8787a81e6f 100644 --- a/pkg/compiler/lib/src/phase/load_kernel.dart +++ b/pkg/compiler/lib/src/phase/load_kernel.dart @@ -111,17 +111,6 @@ String _getPlatformFilename(CompilerOptions options, String targetName) { return "${targetName}_platform$unsoundMarker.dill"; } -void _inferNullSafetyMode(CompilerOptions options, bool isSound) { - if (options.nullSafetyMode == NullSafetyMode.unspecified) { - options.nullSafetyMode = - isSound ? NullSafetyMode.sound : NullSafetyMode.unsound; - } -} - -void _validateNullSafetyMode(CompilerOptions options) { - assert(options.nullSafetyMode != NullSafetyMode.unspecified); -} - class _LoadFromKernelResult { final ir.Component? component; final Library? entryLibrary; @@ -177,8 +166,6 @@ Future<_LoadFromKernelResult> _loadFromKernel(CompilerOptions options, throw ArgumentError("$resolvedUri was compiled with $dillMode null " "safety and is incompatible with the '$option' option"); } - _inferNullSafetyMode(options, isStrongDill); - _validateNullSafetyMode(options); // When compiling modularly, a dill for the SDK will be provided. In those // cases we ignore the implicit platform binary. @@ -269,7 +256,17 @@ Future<_LoadFromSourceResult> _loadFromSource( Uri resolvedUri = options.compilationTarget!; bool isLegacy = await fe.uriUsesLegacyLanguageVersion(resolvedUri, feOptions); - _inferNullSafetyMode(options, !isLegacy); + if (isLegacy && options.nullSafetyMode == NullSafetyMode.sound) { + reporter.reportError( + reporter.createMessage(NO_LOCATION_SPANNABLE, MessageKind.GENERIC, { + 'text': "Starting with Dart 3.0, `dart compile js` expects programs to be " + "null safe by default. Some libraries reached from " + "$resolvedUri opted-out of null safety. " + "You can temporarily compile this application using the deprecated " + "'${Flags.noSoundNullSafety}' option (which will be removed before " + "the Dart 3.0 stable release)." + })); + } sources.add(options.compilationTarget!); } @@ -298,8 +295,6 @@ Future<_LoadFromSourceResult> _loadFromSource( verbosity: verbosity); ir.Component? component = await fe.compile(initializedCompilerState, verbose, fileSystem, onDiagnostic, sources, isModularCompile); - _validateNullSafetyMode(options); - _doTransformsOnKernelLoad(component); // We have to compute canonical names on the component here to avoid missing diff --git a/pkg/compiler/lib/src/util/memory_compiler.dart b/pkg/compiler/lib/src/util/memory_compiler.dart index 2a27c4b1b0d..d2d9ae51f6f 100644 --- a/pkg/compiler/lib/src/util/memory_compiler.dart +++ b/pkg/compiler/lib/src/util/memory_compiler.dart @@ -143,18 +143,37 @@ Compiler compilerFor( // If soundNullSafety is not requested, then we prepend the opt out string to // the memory files. - if (!options.contains(Flags.soundNullSafety) && !unsafeToTouchSourceFiles) { - // Map may be immutable so copy. - sources = {}; - memorySourceFiles.forEach((k, v) => sources[k] = v); - RegExp optOutStr = RegExp(r"\/\/\s*@dart\s*=\s*2\.\d+"); + // TODO(48820): After migrating all tests we should no longer have to infer + // a mode in the memory compiler. The logic to update options and to update + // sources to opt-out should be removed. + if (!options.contains(Flags.soundNullSafety)) { + bool addUnsoundFlag = false; + if (!unsafeToTouchSourceFiles) { + // Map may be immutable so copy. + sources = {}; + memorySourceFiles.forEach((k, v) => sources[k] = v); + addUnsoundFlag = true; + } + for (var key in sources.keys) { if (sources[key] is String && key.endsWith('.dart')) { - if (!optOutStr.hasMatch(sources[key])) { - sources[key] = '// @dart=2.7\n' + sources[key]; + RegExp optOutStr = RegExp(r"\/\/\s*@dart\s*=\s*2\.(\d+)"); + final match = optOutStr.firstMatch(sources[key]); + if (match == null) { + if (!unsafeToTouchSourceFiles) { + sources[key] = '// @dart=2.7\n' + sources[key]; + } + } else { + // If the file version is prior to 2.12, we treat it as unsound + if (int.parse(match.group(1)) < 12) { + addUnsoundFlag = true; + } } } } + if (addUnsoundFlag && !options.contains(Flags.noSoundNullSafety)) { + options = [Flags.noSoundNullSafety, ...options]; + } } MemorySourceFileProvider provider; diff --git a/pkg/compiler/test/end_to_end/dart2js_batch_test.dart b/pkg/compiler/test/end_to_end/dart2js_batch_test.dart index 5d329120bf4..03c68ed86d6 100644 --- a/pkg/compiler/test/end_to_end/dart2js_batch_test.dart +++ b/pkg/compiler/test/end_to_end/dart2js_batch_test.dart @@ -67,8 +67,9 @@ Future runTests(Process process) { String outFile = path.join(tmpDir.path, 'out.js'); String outFile2 = path.join(tmpDir.path, 'out2.js'); - process.stdin.writeln('--out="$outFile" "$inFile"'); - process.stdin.writeln('--out="$outFile2" "$inFile"'); + // TODO(48820): remove null safety flag. + process.stdin.writeln('--no-sound-null-safety --out="$outFile" "$inFile"'); + process.stdin.writeln('--no-sound-null-safety --out="$outFile2" "$inFile"'); process.stdin.writeln('too many arguments'); process.stdin.writeln(r'"non existing file.dart"'); process.stdin.close(); diff --git a/pkg/compiler/test/end_to_end/modular_loader_test.dart b/pkg/compiler/test/end_to_end/modular_loader_test.dart index 33bd45b4ef0..4ab2e11b7af 100644 --- a/pkg/compiler/test/end_to_end/modular_loader_test.dart +++ b/pkg/compiler/test/end_to_end/modular_loader_test.dart @@ -40,7 +40,8 @@ main() { entryPoint: entryPoint, options: [ '--input-dill=memory:c.dill', - '--dill-dependencies=memory:a.dill,memory:b.dill' + '--dill-dependencies=memory:a.dill,memory:b.dill', + '--sound-null-safety', ], memorySourceFiles: {'a.dill': aDill, 'b.dill': bDill, 'c.dill': cDill}, diagnosticHandler: diagnostics, diff --git a/pkg/compiler/test/end_to_end/output_type_test.dart b/pkg/compiler/test/end_to_end/output_type_test.dart index 76ee848da0e..bf533389838 100644 --- a/pkg/compiler/test/end_to_end/output_type_test.dart +++ b/pkg/compiler/test/end_to_end/output_type_test.dart @@ -88,6 +88,7 @@ main() { 'pkg/compiler/test/deferred/data/deferred_helper.dart', '--out=custom.js', '--deferred-map=def/deferred.json', + '--no-sound-null-safety', '--no-csp', Flags.dumpInfo, ], [ @@ -112,6 +113,7 @@ main() { await test([ 'pkg/compiler/test/deferred/data/deferred_helper.dart', + '--no-sound-null-safety', '--csp', ...additionOptionals, ], expectedOutput); diff --git a/pkg/compiler/test/end_to_end/user_crash_test.dart b/pkg/compiler/test/end_to_end/user_crash_test.dart index e2d353ed155..51010b06cf6 100644 --- a/pkg/compiler/test/end_to_end/user_crash_test.dart +++ b/pkg/compiler/test/end_to_end/user_crash_test.dart @@ -10,6 +10,7 @@ import 'package:expect/expect.dart'; import 'package:front_end/src/fasta/messages.dart' show templateCantReadFile, messageMissingMain; import 'package:compiler/compiler_api.dart' as api; +import 'package:compiler/src/commandline_options.dart'; import 'package:compiler/src/util/memory_compiler.dart'; final EXCEPTION = 'Crash-marker'; @@ -75,7 +76,7 @@ Future run( entryPoint: entryPoint, memorySourceFiles: memorySourceFiles, diagnosticHandler: diagnostics, - unsafeToTouchSourceFiles: true); + options: [Flags.soundNullSafety]); } catch (e) { result.exceptions.add(e); } diff --git a/pkg/compiler/test/inference/inference_test_helper.dart b/pkg/compiler/test/inference/inference_test_helper.dart index 3d5d09b2d7d..92a849ef15a 100644 --- a/pkg/compiler/test/inference/inference_test_helper.dart +++ b/pkg/compiler/test/inference/inference_test_helper.dart @@ -9,6 +9,7 @@ import 'package:async_helper/async_helper.dart'; import 'package:compiler/src/closure.dart'; import 'package:compiler/src/common.dart'; import 'package:compiler/src/compiler.dart'; +import 'package:compiler/src/commandline_options.dart'; import 'package:compiler/src/elements/entities.dart'; import 'package:compiler/src/inferrer/typemasks/masks.dart'; import 'package:compiler/src/inferrer/types.dart'; @@ -33,6 +34,9 @@ runTests(List args, [int shardIndex]) { args: args, options: [stopAfterTypeInference], testedConfigs: allInternalConfigs, + perTestOptions: { + "issue48304.dart": [Flags.soundNullSafety], + }, skip: skip, shardIndex: shardIndex ?? 0, shards: shardIndex != null ? 4 : 1); diff --git a/pkg/compiler/test/serialization/on_disk_split_test.dart b/pkg/compiler/test/serialization/on_disk_split_test.dart index fdd403251f0..2dda6731883 100644 --- a/pkg/compiler/test/serialization/on_disk_split_test.dart +++ b/pkg/compiler/test/serialization/on_disk_split_test.dart @@ -23,6 +23,7 @@ main(List args) { await internalMain([ 'pkg/compiler/test/codesize/swarm/swarm.dart', Flags.writeClosedWorld, + '--no-sound-null-safety', '--out=${dillUri}', ] + commonArgs); @@ -30,6 +31,7 @@ main(List args) { '${dillUri}', Flags.readClosedWorld, Flags.writeData, + '--no-sound-null-safety', '--out=${outUri}', ] + commonArgs); diff --git a/pkg/compiler/test/sourcemaps/minified_names_test.dart b/pkg/compiler/test/sourcemaps/minified_names_test.dart index 91000fd9e14..47c0f97db27 100644 --- a/pkg/compiler/test/sourcemaps/minified_names_test.dart +++ b/pkg/compiler/test/sourcemaps/minified_names_test.dart @@ -97,6 +97,7 @@ checkExpectation(MinifiedNameTest test, bool minified) async { print('-- ${minified ? 'minified' : 'not-minified'}:'); var options = [ Flags.testMode, + Flags.noSoundNullSafety, '--libraries-spec=$sdkLibrariesSpecificationUri', if (minified) Flags.minify, ]; diff --git a/pkg/compiler/test/sourcemaps/stacktrace_test.dart b/pkg/compiler/test/sourcemaps/stacktrace_test.dart index 262c8b2ff5c..8adf30ae158 100644 --- a/pkg/compiler/test/sourcemaps/stacktrace_test.dart +++ b/pkg/compiler/test/sourcemaps/stacktrace_test.dart @@ -89,6 +89,7 @@ Future runTest(Test test, String config, '--libraries-spec=$sdkLibrariesSpecificationPath', '--packages=${Platform.packageConfig}', Flags.testMode, + Flags.noSoundNullSafety, input, ]..addAll(options); print("Compiling dart2js ${arguments.join(' ')}"); diff --git a/pkg/compiler/test/sourcemaps/stepping_test.dart b/pkg/compiler/test/sourcemaps/stepping_test.dart index 8b362b224ae..df368369355 100644 --- a/pkg/compiler/test/sourcemaps/stepping_test.dart +++ b/pkg/compiler/test/sourcemaps/stepping_test.dart @@ -75,6 +75,7 @@ Future runTest(AnnotatedCode annotatedCode, String config, '--out=$outputFile', inputFile, Flags.disableInlining, + Flags.noSoundNullSafety, '--libraries-spec=$sdkLibrariesSpecificationUri', ]; CompilationResult compilationResult = await entry.internalMain(arguments); diff --git a/pkg/compiler/tool/modular_test_suite_helper.dart b/pkg/compiler/tool/modular_test_suite_helper.dart index a1e36ac4e86..467e3caa230 100644 --- a/pkg/compiler/tool/modular_test_suite_helper.dart +++ b/pkg/compiler/tool/modular_test_suite_helper.dart @@ -127,6 +127,7 @@ abstract class CFEStep extends IOModularStep { List args = [ '--no-sound-null-safety', _kernelWorkerScript, + '--no-sound-null-safety', ...stepArguments, '--exclude-non-sources', '--multi-root', @@ -350,6 +351,7 @@ class ConcatenateDillsStep extends IOModularStep { _dart2jsScript, // TODO(sigmund): remove this dependency on libraries.json if (_options.useSdk) '--libraries-spec=$_librarySpecForSnapshot', + Flags.noSoundNullSafety, '${Flags.entryUri}=$fakeRoot${module.mainSource}', '${Flags.inputDill}=${toUri(module, dillId)}', for (String flag in flags) '--enable-experiment=$flag', @@ -411,6 +413,7 @@ class ComputeClosedWorldStep extends IOModularStep { _dart2jsScript, // TODO(sigmund): remove this dependency on libraries.json if (_options.useSdk) '--libraries-spec=$_librarySpecForSnapshot', + Flags.noSoundNullSafety, '${Flags.entryUri}=$fakeRoot${module.mainSource}', '${Flags.inputDill}=${toUri(module, fullDillId)}', for (String flag in flags) '--enable-experiment=$flag', @@ -461,6 +464,7 @@ class GlobalAnalysisStep extends IOModularStep { _dart2jsScript, // TODO(sigmund): remove this dependency on libraries.json if (_options.useSdk) '--libraries-spec=$_librarySpecForSnapshot', + Flags.noSoundNullSafety, '${Flags.entryUri}=$fakeRoot${module.mainSource}', '${Flags.inputDill}=${toUri(module, globalUpdatedDillId)}', for (String flag in flags) '--enable-experiment=$flag', @@ -515,6 +519,7 @@ class Dart2jsCodegenStep extends IOModularStep { '--packages=${sdkRoot.toFilePath()}/$packageConfigJsonPath', _dart2jsScript, if (_options.useSdk) '--libraries-spec=$_librarySpecForSnapshot', + Flags.noSoundNullSafety, '${Flags.entryUri}=$fakeRoot${module.mainSource}', '${Flags.inputDill}=${toUri(module, globalUpdatedDillId)}', for (String flag in flags) '--enable-experiment=$flag', @@ -569,6 +574,7 @@ class Dart2jsEmissionStep extends IOModularStep { '--packages=${sdkRoot.toFilePath()}/$packageConfigJsonPath', _dart2jsScript, if (_options.useSdk) '--libraries-spec=$_librarySpecForSnapshot', + Flags.noSoundNullSafety, '${Flags.entryUri}=$fakeRoot${module.mainSource}', '${Flags.inputDill}=${toUri(module, globalUpdatedDillId)}', for (String flag in flags) '${Flags.enableLanguageExperiments}=$flag', diff --git a/pkg/dartdev/test/commands/compile_test.dart b/pkg/dartdev/test/commands/compile_test.dart index 49f532c9db1..866178c9a65 100644 --- a/pkg/dartdev/test/commands/compile_test.dart +++ b/pkg/dartdev/test/commands/compile_test.dart @@ -698,6 +698,7 @@ void main() {} [ 'compile', 'js', + '--no-sound-null-safety', '-o', outFile, inFile, diff --git a/tools/bots/test_matrix.json b/tools/bots/test_matrix.json index f77cd990181..e6ffd661c4d 100644 --- a/tools/bots/test_matrix.json +++ b/tools/bots/test_matrix.json @@ -534,11 +534,6 @@ } }, "dart2js-(linux|mac|win)-chrome": { - "options": { - "use-sdk": true - } - }, - "dart2js-(linux|mac|win)-chrome-unsound": { "options": { "use-sdk": true, "dart2js-options": [ @@ -553,11 +548,6 @@ } }, "dart2js-(linux|win)-firefox": { - "options": { - "use-sdk": true - } - }, - "dart2js-(linux|win)-firefox-unsound": { "options": { "use-sdk": true, "dart2js-options": [ @@ -568,24 +558,13 @@ "dart2js-win-ie11": { "options": { "babel": "{\"presets\":[\"es2015\"]}", - "use-sdk": true - } - }, - "dart2js-win-ie11-unsound": { - "options": { - "babel": "{\"presets\":[\"es2015\"]}", + "use-sdk": true, "dart2js-options": [ "--no-sound-null-safety" - ], - "use-sdk": true + ] } }, "dart2js-win-edge": { - "options": { - "use-sdk": true - } - }, - "dart2js-win-edge-unsound": { "options": { "use-sdk": true, "dart2js-options": [ @@ -594,11 +573,6 @@ } }, "dart2js-mac-safari": { - "options": { - "use-sdk": true - } - }, - "dart2js-mac-safari-unsound": { "options": { "use-sdk": true, "dart2js-options": [ @@ -607,13 +581,6 @@ } }, "dart2js-minified-csp-(linux|mac|win)-chrome": { - "options": { - "minified": true, - "csp": true, - "use-sdk": true - } - }, - "dart2js-minified-csp-(linux|mac|win)-chrome-unsound": { "options": { "minified": true, "csp": true, @@ -630,17 +597,12 @@ "timeout": 240, "dart2js-options": [ "--merge-fragments-threshold=3", - "--simple-load-ids" + "--simple-load-ids", + "--no-sound-null-safety" ] } }, "dart2js-minified-(linux|mac|win)-d8": { - "options": { - "minified": true, - "use-sdk": true - } - }, - "dart2js-minified-(linux|mac|win)-d8-unsound": { "options": { "minified": true, "use-sdk": true, @@ -657,7 +619,8 @@ "builder-tag": "dart2js-weak", "dart2js-options": [ "--merge-fragments-threshold=3", - "--simple-load-ids" + "--simple-load-ids", + "--no-sound-null-safety" ] } }, @@ -666,7 +629,8 @@ "builder-tag": "dart2js_production", "use-sdk": true, "dart2js-options": [ - "-O3" + "-O3", + "--no-sound-null-safety" ] } }, @@ -675,7 +639,8 @@ "builder-tag": "dart2js_o0", "use-sdk": true, "dart2js-options": [ - "-O0" + "-O0", + "--no-sound-null-safety" ] } }, @@ -699,12 +664,6 @@ } }, "dart2js-hostasserts-(linux|mac|win)-(ia32|x64)-d8": { - "options": { - "host-checked": true, - "timeout": 240 - } - }, - "dart2js-hostasserts-(linux|mac|win)-(ia32|x64)-d8-unsound": { "options": { "host-checked": true, "timeout": 240, @@ -2962,13 +2921,6 @@ "pkg" ] }, - { - "name": "dart2js observatory-ui tests", - "arguments": [ - "-ndart2js-hostasserts-linux-ia32-d8-unsound", - "observatory_ui" - ] - }, { "name": "dart2js extra tests", "arguments": [ @@ -3028,14 +2980,6 @@ "pkg" ] }, - { - "name": "dart2js observatory-ui tests", - "arguments": [ - "-ndart2js-${system}-${runtime}-unsound", - "--reset-browser-configuration", - "observatory_ui" - ] - }, { "name": "dart2js extra tests", "arguments": [ @@ -3077,13 +3021,6 @@ "pkg" ] }, - { - "name": "dart2js observatory-ui tests", - "arguments": [ - "-ndart2js-minified-linux-d8-unsound", - "observatory_ui" - ] - }, { "name": "dart2js extra tests", "arguments": [ @@ -3161,7 +3098,7 @@ { "name": "dart2js observatory-ui tests", "arguments": [ - "-ndart2js-minified-csp-linux-chrome-unsound", + "-ndart2js-minified-csp-linux-chrome", "--reset-browser-configuration", "observatory_ui" ] diff --git a/utils/compiler/BUILD.gn b/utils/compiler/BUILD.gn index a9e7449b2b2..5d93b26d014 100644 --- a/utils/compiler/BUILD.gn +++ b/utils/compiler/BUILD.gn @@ -70,6 +70,9 @@ application_snapshot("dart2js") { # Specifying the platform explicitly elides running the CFE on the sdk # sources. "--platform-binaries=" + rebase_path("$root_out_dir/"), + + # TODO(48820): remove once dart2js is migrated. + "--no-sound-null-safety", rebase_path("../../pkg/compiler/lib/src/util/memory_compiler.dart"), ] }