Skip to content

Commit b524f39

Browse files
nshahancommit-bot@chromium.org
nshahan
authored and
commit-bot@chromium.org
committed
[ddc] Add option to run non-null asserts in weak mode
* Setting the nonNullAsserts flag in the bootstrapping logic will enable/disable failing when a null value is passed to a non-nullable method parameter when running with weak null safety. * Move the --null-assertions from VM options to the shared options in the test and use it to set the flag in DDC the entry point. * Configure other backends to ignore the flag. Change-Id: Ia2670514bed7fa981564e99b85d74f6bae6dd9fc Fixes: #42404 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151306 Commit-Queue: Nicholas Shahan <nshahan@google.com> Reviewed-by: Mark Zhou <markzipan@google.com> Reviewed-by: Sigmund Cherem <sigmund@google.com>
1 parent a199791 commit b524f39

File tree

7 files changed

+54
-15
lines changed

7 files changed

+54
-15
lines changed

‎pkg/dev_compiler/tool/ddb‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ void main(List<String> args) async {
4747
'DDC binary. Defaults to false.',
4848
defaultsTo: false,
4949
negatable: true)
50+
..addFlag('null-assertions',
51+
help: 'Run with assertions that values passed to non-nullable method '
52+
'parameters are not null.',
53+
defaultsTo: false,
54+
negatable: true)
5055
..addFlag('observe',
5156
help:
5257
'Run the compiler in the Dart VM with --observe. Implies --debug.',
@@ -101,6 +106,7 @@ void main(List<String> args) async {
101106
var compile = mode == 'compile' || mode == 'all';
102107
var run = mode == 'run' || mode == 'all';
103108
var verbose = options['verbose'] as bool;
109+
var nonNullAsserts = options['null-assertions'] as bool;
104110

105111
var soundNullSafety = options['sound-null-safety'] as bool;
106112
// Enable null safety either by passing the `non-nullable` experiment flag or
@@ -272,6 +278,7 @@ void main(List<String> args) async {
272278
if ($nnbd) {
273279
sdk.dart.nullSafety($soundNullSafety);
274280
sdk.dart.weakNullSafetyWarnings(!$soundNullSafety);
281+
sdk.dart.nonNullAsserts($nonNullAsserts);
275282
}
276283
sdk._debugger.registerDevtoolsFormatter();
277284
app.$libname.main();
@@ -305,6 +312,7 @@ try {
305312
if ($nnbd) {
306313
sdk.dart.nullSafety($soundNullSafety);
307314
sdk.dart.weakNullSafetyWarnings(!$soundNullSafety);
315+
sdk.dart.nonNullAsserts($nonNullAsserts);
308316
}
309317
sdk._isolate_helper.startRootIsolate(main, []);
310318
} catch(e) {
@@ -339,6 +347,7 @@ try {
339347
if ($nnbd) {
340348
dart.nullSafety($soundNullSafety);
341349
dart.weakNullSafetyWarnings(!$soundNullSafety);
350+
sdk.dart.nonNullAsserts($nonNullAsserts);
342351
}
343352
_isolate_helper.startRootIsolate(() => {}, []);
344353
main();

‎pkg/test_runner/lib/src/browser.dart‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,11 @@ bool _invalidVariableName(String keyword, {bool strictMode = true}) {
145145
/// or extension, like "math_test". [testNameAlias] is the alias of the
146146
/// test variable used for import/export (usually relative to its module root).
147147
/// [testJSDir] is the relative path to the build directory where the
148-
/// dartdevc-generated JS file is stored.
148+
/// dartdevc-generated JS file is stored. [nonNullAsserts] enables non-null
149+
/// assertions for non-nullable method parameters when running with weak null
150+
/// safety.
149151
String dartdevcHtml(String testName, String testNameAlias, String testJSDir,
150-
Compiler compiler, NnbdMode mode) {
152+
Compiler compiler, NnbdMode mode, bool nonNullAsserts) {
151153
var testId = pathToJSIdentifier(testName);
152154
var testIdAlias = pathToJSIdentifier(testNameAlias);
153155
var isKernel = compiler == Compiler.dartdevk;
@@ -234,6 +236,7 @@ requirejs(["$testName", "dart_sdk", "async_helper"],
234236
if ($isNnbd) {
235237
sdk.dart.nullSafety($isNnbdStrong);
236238
sdk.dart.weakNullSafetyWarnings(!$isNnbdStrong);
239+
sdk.dart.nonNullAsserts($nonNullAsserts);
237240
}
238241
239242
dartMainRunner(function testMainWrapper() {

‎pkg/test_runner/lib/src/compiler_configuration.dart‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,12 @@ class Dart2jsCompilerConfiguration extends Dart2xCompilerConfiguration {
419419

420420
List<String> computeCompilerArguments(
421421
TestFile testFile, List<String> vmOptions, List<String> args) {
422+
// TODO(#42403) Handle this option if dart2js supports non-nullable asserts
423+
// on non-nullable method arguments.
424+
var options = testFile.sharedOptions.toList();
425+
options.remove('--null-assertions');
422426
return [
423-
...testFile.sharedOptions,
427+
...options,
424428
..._configuration.sharedOptions,
425429
..._experimentsArgument(_configuration, testFile),
426430
...testFile.dart2jsOptions,
@@ -510,6 +514,11 @@ class DevCompilerConfiguration extends CompilerConfiguration {
510514
Command _createCommand(String inputFile, String outputFile,
511515
List<String> sharedOptions, Map<String, String> environment) {
512516
var args = <String>[];
517+
// Remove option for generating non-null assertions for non-nullable
518+
// method parameters in weak mode. DDC treats this as a runtime flag for
519+
// the bootstrapping code, instead of a compiler option.
520+
var options = sharedOptions.toList();
521+
options.remove('--null-assertions');
513522
if (!_useSdk) {
514523
// If we're testing a built SDK, DDC will find its own summary.
515524
//
@@ -524,7 +533,7 @@ class DevCompilerConfiguration extends CompilerConfiguration {
524533
.toNativePath();
525534
args.addAll(["--dart-sdk-summary", sdkSummary]);
526535
}
527-
args.addAll(sharedOptions);
536+
args.addAll(options);
528537
args.addAll(_configuration.sharedOptions);
529538

530539
args.addAll([
@@ -1179,8 +1188,12 @@ class FastaCompilerConfiguration extends CompilerConfiguration {
11791188
@override
11801189
List<String> computeCompilerArguments(
11811190
TestFile testFile, List<String> vmOptions, List<String> args) {
1191+
// Remove shared option for generating non-null assertions for non-nullable
1192+
// method parameters in weak mode. It's currently unused by the front end.
1193+
var options = testFile.sharedOptions.toList();
1194+
options.remove('--null-assertions');
11821195
var arguments = [
1183-
...testFile.sharedOptions,
1196+
...options,
11841197
..._configuration.sharedOptions,
11851198
..._experimentsArgument(_configuration, testFile),
11861199
if (_configuration.configuration.nnbdMode == NnbdMode.strong) ...[

‎pkg/test_runner/lib/src/test_suite.dart‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,8 +810,10 @@ class StandardTestSuite extends TestSuite {
810810
"${nameFromModuleRoot.directoryPath}/$nameNoExt";
811811
var jsDir =
812812
Path(compilationTempDir).relativeTo(Repository.dir).toString();
813+
var nullAssertions =
814+
testFile.sharedOptions.contains('--null-assertions');
813815
content = dartdevcHtml(nameNoExt, nameFromModuleRootNoExt, jsDir,
814-
configuration.compiler, configuration.nnbdMode);
816+
configuration.compiler, configuration.nnbdMode, nullAssertions);
815817
}
816818
}
817819

‎sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,19 @@ final _nullFailedSet = JS('!', 'new Set()');
2828
// Run-time null safety assertion per:
2929
// https://github.com/dart-lang/language/blob/master/accepted/future-releases/nnbd/feature-specification.md#automatic-debug-assertion-insertion
3030
nullFailed(String? fileUri, int? line, int? column, String? variable) {
31-
if (strictNullSafety) {
31+
if (_nonNullAsserts) {
3232
throw AssertionErrorImpl(
3333
'A null value was passed into a non-nullable parameter $variable',
3434
fileUri,
3535
line,
3636
column,
3737
'$variable != null');
38-
} else {
39-
var key = '$fileUri:$line:$column';
40-
if (!JS('!', '#.has(#)', _nullFailedSet, key)) {
41-
JS('', '#.add(#)', _nullFailedSet, key);
42-
_nullWarn(
43-
'A null value was passed into a non-nullable parameter $variable');
44-
}
38+
}
39+
var key = '$fileUri:$line:$column';
40+
if (!JS('!', '#.has(#)', _nullFailedSet, key)) {
41+
JS('', '#.add(#)', _nullFailedSet, key);
42+
_nullWarn(
43+
'A null value was passed into a non-nullable parameter $variable');
4544
}
4645
}
4746

‎sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,18 @@ void weakNullSafetyWarnings(bool showWarnings) {
4545
_weakNullSafetyWarnings = showWarnings;
4646
}
4747

48+
@notNull
49+
bool _nonNullAsserts = false;
50+
51+
/// Sets the runtime mode to insert non-null assertions on non-nullable method
52+
/// parameters.
53+
///
54+
/// When [weakNullSafetyWarnings] is also `true` the assertions will fail
55+
/// instead of printing a warning for the non-null parameters.
56+
void nonNullAsserts(bool enable) {
57+
_nonNullAsserts = enable;
58+
}
59+
4860
final metadata = JS('', 'Symbol("metadata")');
4961

5062
/// Types in dart are represented internally at runtime as follows.

‎tests/language/nnbd/null_assertions/parameter_checks_test.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
// Test for null assertions for parameters in NNBD weak mode.
66

77
// Requirements=nnbd-weak
8-
// VMOptions=--enable-asserts --null-assertions
8+
// VMOptions=--enable-asserts
9+
// SharedOptions=--null-assertions
910

1011
// Opt out of Null Safety:
1112
// @dart = 2.6

0 commit comments

Comments
 (0)