Skip to content

Commit

Permalink
[ddc] Add option to run non-null asserts in weak mode
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
nshahan authored and commit-bot@chromium.org committed Jul 7, 2020
1 parent a199791 commit b524f39
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 15 deletions.
9 changes: 9 additions & 0 deletions pkg/dev_compiler/tool/ddb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ void main(List<String> args) async {
'DDC binary. Defaults to false.',
defaultsTo: false,
negatable: true)
..addFlag('null-assertions',
help: 'Run with assertions that values passed to non-nullable method '
'parameters are not null.',
defaultsTo: false,
negatable: true)
..addFlag('observe',
help:
'Run the compiler in the Dart VM with --observe. Implies --debug.',
Expand Down Expand Up @@ -101,6 +106,7 @@ void main(List<String> args) async {
var compile = mode == 'compile' || mode == 'all';
var run = mode == 'run' || mode == 'all';
var verbose = options['verbose'] as bool;
var nonNullAsserts = options['null-assertions'] as bool;

var soundNullSafety = options['sound-null-safety'] as bool;
// Enable null safety either by passing the `non-nullable` experiment flag or
Expand Down Expand Up @@ -272,6 +278,7 @@ void main(List<String> args) async {
if ($nnbd) {
sdk.dart.nullSafety($soundNullSafety);
sdk.dart.weakNullSafetyWarnings(!$soundNullSafety);
sdk.dart.nonNullAsserts($nonNullAsserts);
}
sdk._debugger.registerDevtoolsFormatter();
app.$libname.main();
Expand Down Expand Up @@ -305,6 +312,7 @@ try {
if ($nnbd) {
sdk.dart.nullSafety($soundNullSafety);
sdk.dart.weakNullSafetyWarnings(!$soundNullSafety);
sdk.dart.nonNullAsserts($nonNullAsserts);
}
sdk._isolate_helper.startRootIsolate(main, []);
} catch(e) {
Expand Down Expand Up @@ -339,6 +347,7 @@ try {
if ($nnbd) {
dart.nullSafety($soundNullSafety);
dart.weakNullSafetyWarnings(!$soundNullSafety);
sdk.dart.nonNullAsserts($nonNullAsserts);
}
_isolate_helper.startRootIsolate(() => {}, []);
main();
Expand Down
7 changes: 5 additions & 2 deletions pkg/test_runner/lib/src/browser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,11 @@ bool _invalidVariableName(String keyword, {bool strictMode = true}) {
/// or extension, like "math_test". [testNameAlias] is the alias of the
/// test variable used for import/export (usually relative to its module root).
/// [testJSDir] is the relative path to the build directory where the
/// dartdevc-generated JS file is stored.
/// dartdevc-generated JS file is stored. [nonNullAsserts] enables non-null
/// assertions for non-nullable method parameters when running with weak null
/// safety.
String dartdevcHtml(String testName, String testNameAlias, String testJSDir,
Compiler compiler, NnbdMode mode) {
Compiler compiler, NnbdMode mode, bool nonNullAsserts) {
var testId = pathToJSIdentifier(testName);
var testIdAlias = pathToJSIdentifier(testNameAlias);
var isKernel = compiler == Compiler.dartdevk;
Expand Down Expand Up @@ -234,6 +236,7 @@ requirejs(["$testName", "dart_sdk", "async_helper"],
if ($isNnbd) {
sdk.dart.nullSafety($isNnbdStrong);
sdk.dart.weakNullSafetyWarnings(!$isNnbdStrong);
sdk.dart.nonNullAsserts($nonNullAsserts);
}
dartMainRunner(function testMainWrapper() {
Expand Down
19 changes: 16 additions & 3 deletions pkg/test_runner/lib/src/compiler_configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,12 @@ class Dart2jsCompilerConfiguration extends Dart2xCompilerConfiguration {

List<String> computeCompilerArguments(
TestFile testFile, List<String> vmOptions, List<String> args) {
// TODO(#42403) Handle this option if dart2js supports non-nullable asserts
// on non-nullable method arguments.
var options = testFile.sharedOptions.toList();
options.remove('--null-assertions');
return [
...testFile.sharedOptions,
...options,
..._configuration.sharedOptions,
..._experimentsArgument(_configuration, testFile),
...testFile.dart2jsOptions,
Expand Down Expand Up @@ -510,6 +514,11 @@ class DevCompilerConfiguration extends CompilerConfiguration {
Command _createCommand(String inputFile, String outputFile,
List<String> sharedOptions, Map<String, String> environment) {
var args = <String>[];
// Remove option for generating non-null assertions for non-nullable
// method parameters in weak mode. DDC treats this as a runtime flag for
// the bootstrapping code, instead of a compiler option.
var options = sharedOptions.toList();
options.remove('--null-assertions');
if (!_useSdk) {
// If we're testing a built SDK, DDC will find its own summary.
//
Expand All @@ -524,7 +533,7 @@ class DevCompilerConfiguration extends CompilerConfiguration {
.toNativePath();
args.addAll(["--dart-sdk-summary", sdkSummary]);
}
args.addAll(sharedOptions);
args.addAll(options);
args.addAll(_configuration.sharedOptions);

args.addAll([
Expand Down Expand Up @@ -1179,8 +1188,12 @@ class FastaCompilerConfiguration extends CompilerConfiguration {
@override
List<String> computeCompilerArguments(
TestFile testFile, List<String> vmOptions, List<String> args) {
// Remove shared option for generating non-null assertions for non-nullable
// method parameters in weak mode. It's currently unused by the front end.
var options = testFile.sharedOptions.toList();
options.remove('--null-assertions');
var arguments = [
...testFile.sharedOptions,
...options,
..._configuration.sharedOptions,
..._experimentsArgument(_configuration, testFile),
if (_configuration.configuration.nnbdMode == NnbdMode.strong) ...[
Expand Down
4 changes: 3 additions & 1 deletion pkg/test_runner/lib/src/test_suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -810,8 +810,10 @@ class StandardTestSuite extends TestSuite {
"${nameFromModuleRoot.directoryPath}/$nameNoExt";
var jsDir =
Path(compilationTempDir).relativeTo(Repository.dir).toString();
var nullAssertions =
testFile.sharedOptions.contains('--null-assertions');
content = dartdevcHtml(nameNoExt, nameFromModuleRootNoExt, jsDir,
configuration.compiler, configuration.nnbdMode);
configuration.compiler, configuration.nnbdMode, nullAssertions);
}
}

Expand Down
15 changes: 7 additions & 8 deletions sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,19 @@ final _nullFailedSet = JS('!', 'new Set()');
// Run-time null safety assertion per:
// https://github.com/dart-lang/language/blob/master/accepted/future-releases/nnbd/feature-specification.md#automatic-debug-assertion-insertion
nullFailed(String? fileUri, int? line, int? column, String? variable) {
if (strictNullSafety) {
if (_nonNullAsserts) {
throw AssertionErrorImpl(
'A null value was passed into a non-nullable parameter $variable',
fileUri,
line,
column,
'$variable != null');
} else {
var key = '$fileUri:$line:$column';
if (!JS('!', '#.has(#)', _nullFailedSet, key)) {
JS('', '#.add(#)', _nullFailedSet, key);
_nullWarn(
'A null value was passed into a non-nullable parameter $variable');
}
}
var key = '$fileUri:$line:$column';
if (!JS('!', '#.has(#)', _nullFailedSet, key)) {
JS('', '#.add(#)', _nullFailedSet, key);
_nullWarn(
'A null value was passed into a non-nullable parameter $variable');
}
}

Expand Down
12 changes: 12 additions & 0 deletions sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ void weakNullSafetyWarnings(bool showWarnings) {
_weakNullSafetyWarnings = showWarnings;
}

@notNull
bool _nonNullAsserts = false;

/// Sets the runtime mode to insert non-null assertions on non-nullable method
/// parameters.
///
/// When [weakNullSafetyWarnings] is also `true` the assertions will fail
/// instead of printing a warning for the non-null parameters.
void nonNullAsserts(bool enable) {
_nonNullAsserts = enable;
}

final metadata = JS('', 'Symbol("metadata")');

/// Types in dart are represented internally at runtime as follows.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
// Test for null assertions for parameters in NNBD weak mode.

// Requirements=nnbd-weak
// VMOptions=--enable-asserts --null-assertions
// VMOptions=--enable-asserts
// SharedOptions=--null-assertions

// Opt out of Null Safety:
// @dart = 2.6
Expand Down

0 comments on commit b524f39

Please sign in to comment.