dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Revert "[ddc] Add non-null assertions when setting fields"

Browse Source 
This reverts commit 27099c121c.

Reason for revert: Breaks google3 (b/246251728)
Original change's description:
> [ddc] Add non-null assertions when setting fields
>
> Fixes: https://github.com/dart-lang/sdk/issues/49918
> Change-Id: I6dddda878afa504bebebb00a80855bac636f8efd
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/258220
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Commit-Queue: Nicholas Shahan <nshahan@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I5e3fb1c151a85e11025135b8fc95c65192b33791
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/258780
Commit-Queue: Emmanuel Pellereau <emmanuelp@google.com>
Reviewed-by: Emmanuel Pellereau <emmanuelp@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
This commit is contained in:
Emmanuel Pellereau
2022-09-12 11:21:06 +00:00
committed by Commit Bot
parent 96d52d4e35
commit 727e792e77
4 changed files with 39 additions and 171 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/dev_compiler/lib/src/kernel/compiler.dart
+38 -62
View File
@@ -2170,21 +2170,18 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
var jsGetter = js_ast.Method(name, getter, isGetter: true)
..sourceInformation = _nodeStart(field);
var body = <js_ast.Statement>[];
var value = _emitIdentifier('value');
if (_requiresExtraNullCheck(field.setterType, field.annotations)) {
body.add(
_nullSafetyParameterCheck(value, field.location, field.name.text));
}
var args = field.isFinal
? [js_ast.Super(), name, value]
: [
js_ast.This(),
virtualFieldSymbol,
if (isCovariantField(field)) _emitCast(value, field.type) else value
];
body.add(js.call('#[#] = #', args).toStatement());
var jsSetter = js_ast.Method(name, js_ast.Fun([value], js_ast.Block(body)),
? [js_ast.Super(), name]
: [js_ast.This(), virtualFieldSymbol];
js_ast.Expression value = _emitIdentifier('value');
if (!field.isFinal && isCovariantField(field)) {
value = _emitCast(value, field.type);
}
args.add(value);
var jsSetter = js_ast.Method(
name, js.fun('function(value) { #[#] = #; }', args),
isSetter: true)
..sourceInformation = _nodeStart(field);
@@ -2371,17 +2368,10 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
member.fileOffset,
member.name.text.length));
if (!member.isFinal && !member.isConst) {
var body = <js_ast.Statement>[];
var value = _emitIdentifier('value');
if (_requiresExtraNullCheck(member.setterType, member.annotations)) {
body.add(_nullSafetyParameterCheck(
value, member.location, member.name.text));
}
// Even when no null check is present a dummy setter is still required
// to indicate writeable.
// TODO(jmesserly): currently uses a dummy setter to indicate
// writable.
accessors.add(js_ast.Method(
access, js_ast.Fun([value], js_ast.Block(body)),
access, js.call('function(_) {}') as js_ast.Fun,
isSetter: true));
}
} else if (member is Procedure) {
@@ -3618,42 +3608,6 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
bool _mustBeNonNullable(DartType type) =>
type.nullability == Nullability.nonNullable;
/// Returns `true` when an additional null check is needed because of the
/// null safety mode, and the provided [type] and [annotations].
bool _requiresExtraNullCheck(DartType type, List<Expression> annotations) =>
!_options.soundNullSafety &&
_mustBeNonNullable(type) &&
!_annotatedNotNull(annotations);
/// Returns a null check for [value] that if fails produces an error message
/// containing the [location] and [name] of the original value being checked.
///
/// This is used to generate checks for non-nullable parameters when running
/// with weak null safety. The checks can be silent, warn, or throw, depending
/// on the flags set in the SDK at runtime.
js_ast.Statement _nullSafetyParameterCheck(
js_ast.Identifier value, Location? location, String? name) {
// TODO(nshahan): Remove when weak mode null safety assertions are no longer
// supported.
// The check on `field.setterType` is per:
// https://github.com/dart-lang/language/blob/master/accepted/2.12/nnbd/feature-specification.md#automatic-debug-assertion-insertion
var condition = js.call('# == null', [value]);
// Offsets are not available for compiler-generated variables
// Get the best available location even if the offset is missing.
// https://github.com/dart-lang/sdk/issues/34942
return js.statement(' if (#) #;', [
condition,
runtimeCall('nullFailed(#, #, #, #)', [
location != null
? _cacheUri(location.file.toString())
: js_ast.LiteralNull(),
js.number(location?.line ?? -1),
js.number(location?.column ?? -1),
js.escapedString('$name')
])
]);
}
/// Emits argument initializers, which handles optional/named args, as well
/// as generic type checks needed due to our covariance.
List<js_ast.Statement> _emitArgumentInitializers(
@@ -3684,8 +3638,30 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
if (_annotatedNullCheck(p.annotations)) {
body.add(_nullParameterCheck(jsParam));
} else if (_requiresExtraNullCheck(p.type, p.annotations)) {
body.add(_nullSafetyParameterCheck(jsParam, p.location, p.name));
} else if (!_options.soundNullSafety &&
_mustBeNonNullable(p.type) &&
!_annotatedNotNull(p.annotations)) {
// TODO(vsm): Remove if / when CFE does this:
// https://github.com/dart-lang/sdk/issues/40597
// The check on `p.type` is per:
// https://github.com/dart-lang/language/blob/master/accepted/future-releases/nnbd/feature-specification.md#automatic-debug-assertion-insertion
var condition = js.call('# == null', [jsParam]);
// Offsets are not available for compiler-generated variables
// Get the best available location even if the offset is missing.
// https://github.com/dart-lang/sdk/issues/34942
var location = p.location;
var check = js.statement(' if (#) #;', [
condition,
runtimeCall('nullFailed(#, #, #, #)', [
location != null
? _cacheUri(location.file.toString())
: js_ast.LiteralNull(),
js.number(location?.line ?? -1),
js.number(location?.column ?? -1),
js.escapedString('${p.name}')
])
]);
body.add(check);
}
}
sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart
+1 -1
View File
@@ -65,7 +65,7 @@ String _nullFailedMessage(variableName) =>
'A null value was passed into a non-nullable parameter: $variableName.';
// Run-time null safety assertion per:
// https://github.com/dart-lang/language/blob/master/accepted/2.12/nnbd/feature-specification.md#automatic-debug-assertion-insertion
// 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 (_nonNullAsserts) {
throw AssertionErrorImpl(_nullFailedMessage(variable), fileUri, line,
tests/language/nnbd/null_assertions/parameter_checks_fields_and_setters_test.dart
-81
View File
@@ -1,81 +0,0 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// Test for null assertions for parameters in NNBD weak mode.
// Requirements=nnbd-weak
// VMOptions=--enable-asserts
// dart2jsOptions=--enable-asserts -DcheckString=false
// SharedOptions=--null-assertions
// Opt out of Null Safety:
// @dart = 2.6
import "package:expect/expect.dart";
import 'parameter_checks_opted_in.dart';
bool Function(Object) asserted(String name) {
if (const bool.fromEnvironment('checkString', defaultValue: true)) {
return (e) => e is AssertionError && e.toString().contains("$name != null");
} else {
return (e) => e is AssertionError;
}
}
class C extends A {
// Overrides the getters but not the setters.
@override
int get getterSetterPair => 999;
@override
int get field => 999;
}
main() {
Expect.throws(() {
topLevelField = null;
}, asserted("topLevelField"));
Expect.throws(() {
topLevelGetterSetterPair = null;
}, asserted("i"));
Expect.throws(() {
topLevelSetterOnly = null;
}, asserted("s"));
var a = A();
Expect.throws(() {
a.getterSetterPair = null;
}, asserted("i"));
Expect.throws(() {
a.setterOnly = null;
}, asserted("s"));
Expect.throws(() {
a.field = null;
}, asserted("field"));
Expect.throws(() {
A.staticGetterSetterPair = null;
}, asserted("i"));
Expect.throws(() {
A.staticSetterOnly = null;
}, asserted("s"));
Expect.throws(() {
A.staticField = null;
}, asserted("staticField"));
var b = B();
Expect.throws(() {
b.getterSetterPair = null;
}, asserted("i"));
Expect.throws(() {
b.field = null;
}, asserted("field"));
var c = C();
Expect.throws(() {
c.getterSetterPair = null;
}, asserted("i"));
Expect.throws(() {
c.field = null;
}, asserted("field"));
}
tests/language/nnbd/null_assertions/parameter_checks_opted_in.dart
-27
View File
@@ -6,11 +6,6 @@
import 'dart:async' show FutureOr;
bool topLevelField = false;
int get topLevelGetterSetterPair => 0;
set topLevelGetterSetterPair(int i) => null;
set topLevelSetterOnly(String s) => null;
foo1(int a) {}
foo2(int a, [int b = 1, String c = '']) {}
foo3({int a = 0, required int b}) {}
@@ -22,25 +17,3 @@ foo6a<T extends FutureOr<S>, S extends U, U extends int?>(T a) {}
foo6b<T extends FutureOr<S>, S extends U, U extends int>(T a) {}
void Function(int) bar() => (int x) {};
class A {
int get getterSetterPair => 0;
set getterSetterPair(int i) => null;
set setterOnly(String s) => null;
int field = 0;
static bool staticField = false;
static int get staticGetterSetterPair => 0;
static set staticGetterSetterPair(int i) => null;
static set staticSetterOnly(String s) => null;
void instanceMethod(String s) => print(s);
static void staticMethod(String s) => print(s);
}
class B extends A {
// Overrides the getters but not the setters.
@override
int get getterSetterPair => 999;
@override
int get field => 999;
}