[ddc] Add a covariant parameter check for optional nonnullable parameters with null initializer on lowered constructor tearoffs.
Change-Id: I77379ee43173ddbbdee25083bd641968eeeb787f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406500
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 443ef0b..c05a24d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,18 @@
- Added `Iterable.withIterator` constructor.
+### Tools
+
+#### Dart Development Compiler (dartdevc)
+
+In order to align with dart2js semantics, DDC will now throw a runtime error
+when a redirecting factory is torn off and one of its optional non-nullable
+parameters is provided no value. The implicit null passed to the factory will
+not match the non-nullable type and this will now throw.
+
+In the future this will likely be a compile-time error and will be entirely
+disallowed.
+
## 3.7.0
**Released on:** Unreleased
diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
index d9e7cde..7670f10 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -3857,7 +3857,8 @@
_emitCovarianceBoundsCheck(f.typeParameters, body);
- void initParameter(VariableDeclaration p, js_ast.Identifier jsParam) {
+ void initParameter(
+ VariableDeclaration p, js_ast.Identifier jsParam, bool isOptional) {
// When the parameter is covariant, insert the null check before the
// covariant cast to avoid a TypeError when testing equality with null.
if (name == '==') {
@@ -3870,7 +3871,19 @@
// Eliminate it when possible.
body.add(js.statement('if (# == null) return false;', [jsParam]));
}
- if (isCovariantParameter(p)) {
+ if (isCovariantParameter(p) ||
+ // TODO(52582): This should be unreachable once the CFE ensures that
+ // redirecting factories parameter types match the target constructor.
+ // Matches dart2js check semantics for redirecting factory tearoffs.
+ // If a non-nullable optional argument with a null initializer is
+ // detected, we add an additional covariant check at runtime.
+ (f.parent is Procedure &&
+ isOptional &&
+ isConstructorTearOffLowering(f.parent as Procedure) &&
+ !p.type.isPotentiallyNullable &&
+ !p.initializer!
+ .getStaticType(_staticTypeContext)
+ .isPotentiallyNonNullable)) {
var castExpr = _emitCast(jsParam, p.type);
if (!identical(castExpr, jsParam)) body.add(castExpr.toStatement());
}
@@ -3884,11 +3897,13 @@
}
}
+ var counter = 0;
for (var p in f.positionalParameters) {
var jsParam = _emitVariableRef(p);
if (_checkParameters) {
- initParameter(p, jsParam);
+ initParameter(p, jsParam, counter >= f.requiredParameterCount);
}
+ counter++;
}
for (var p in f.namedParameters) {
// Parameters will be passed using their real names, not the (possibly
@@ -3906,7 +3921,7 @@
]));
if (_checkParameters) {
- initParameter(p, jsParam);
+ initParameter(p, jsParam, !p.isRequired);
}
}
diff --git a/pkg/dev_compiler/lib/src/kernel/compiler_new.dart b/pkg/dev_compiler/lib/src/kernel/compiler_new.dart
index 9002e0a..8836869 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler_new.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler_new.dart
@@ -4256,7 +4256,8 @@
_emitCovarianceBoundsCheck(f.typeParameters, body);
- void initParameter(VariableDeclaration p, js_ast.Identifier jsParam) {
+ void initParameter(
+ VariableDeclaration p, js_ast.Identifier jsParam, bool isOptional) {
// When the parameter is covariant, insert the null check before the
// covariant cast to avoid a TypeError when testing equality with null.
if (name == '==') {
@@ -4269,7 +4270,19 @@
// Eliminate it when possible.
body.add(js.statement('if (# == null) return false;', [jsParam]));
}
- if (isCovariantParameter(p)) {
+ if (isCovariantParameter(p) ||
+ // TODO(52582): This should be unreachable once the CFE ensures that
+ // redirecting factories parameter types match the target constructor.
+ // Matches dart2js check semantics for redirecting factory tearoffs.
+ // If a non-nullable optional argument with a null initializer is
+ // detected, we add an additional covariant check at runtime.
+ (f.parent is Procedure &&
+ isOptional &&
+ isConstructorTearOffLowering(f.parent as Procedure) &&
+ !p.type.isPotentiallyNullable &&
+ !p.initializer!
+ .getStaticType(_staticTypeContext)
+ .isPotentiallyNonNullable)) {
var castExpr = _emitCast(jsParam, p.type);
if (!identical(castExpr, jsParam)) body.add(castExpr.toStatement());
}
@@ -4283,11 +4296,13 @@
}
}
+ var counter = 0;
for (var p in f.positionalParameters) {
var jsParam = _emitVariableRef(p);
if (_checkParameters) {
- initParameter(p, jsParam);
+ initParameter(p, jsParam, counter >= f.requiredParameterCount);
}
+ counter++;
}
for (var p in f.namedParameters) {
// Parameters will be passed using their real names, not the (possibly
@@ -4305,7 +4320,7 @@
]));
if (_checkParameters) {
- initParameter(p, jsParam);
+ initParameter(p, jsParam, !p.isRequired);
}
}
diff --git a/tests/web/regress/issue/52582_test.dart b/tests/web/regress/issue/52582_test.dart
new file mode 100644
index 0000000..d21f3df4
--- /dev/null
+++ b/tests/web/regress/issue/52582_test.dart
@@ -0,0 +1,18 @@
+// Copyright (c) 2025, 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.
+
+import 'package:expect/expect.dart';
+
+class A {
+ factory A.foo([int x]) = B.foo;
+}
+
+class B implements A {
+ B.foo([int? x]);
+}
+
+void main() {
+ final f = A.foo;
+ Expect.throws(() => f(), (e) => e is TypeError);
+}
