Skip to content

Commit 5c99205

Browse files
fishythefishcommit-bot@chromium.org
fishythefish
authored and
[email protected]
committed
[dart2js] Support required named parameters in weak NNBD function types.
We cannot simply discard the `required` modifier in weak mode function types since function types that differ in the placement of `required` cannot compare equal. Instead, we do that during subtype checks. We continue to ignore `required` in the actual calling convention in weak mode. Change-Id: I7dbb28550095c635f65592f78e495e8e4e8d7026 Fixes: #42608 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153386 Reviewed-by: Mark Zhou <[email protected]> Reviewed-by: Stephen Adams <[email protected]> Commit-Queue: Mayank Patke <[email protected]>
1 parent a1f5100 commit 5c99205

7 files changed

Lines changed: 27 additions & 21 deletions

File tree

‎pkg/compiler/lib/src/elements/types.dart‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,21 +2114,25 @@ abstract class DartTypes {
21142114
String sName = sNamed[sIndex++];
21152115
int comparison = sName.compareTo(tName);
21162116
if (comparison > 0) return false;
2117-
bool sIsRequired = sRequiredNamed.contains(sName);
2117+
bool sIsRequired =
2118+
!useLegacySubtyping && sRequiredNamed.contains(sName);
21182119
if (comparison < 0) {
21192120
if (sIsRequired) return false;
21202121
continue;
21212122
}
2122-
bool tIsRequired = tRequiredNamed.contains(tName);
2123+
bool tIsRequired =
2124+
!useLegacySubtyping && tRequiredNamed.contains(tName);
21232125
if (sIsRequired && !tIsRequired) return false;
21242126
if (!_isSubtype(
21252127
tNamedTypes[tIndex], sNamedTypes[sIndex - 1], env))
21262128
return false;
21272129
break;
21282130
}
21292131
}
2130-
while (sIndex < sNamedLength) {
2131-
if (sRequiredNamed.contains(sNamed[sIndex++])) return false;
2132+
if (!useLegacySubtyping) {
2133+
while (sIndex < sNamedLength) {
2134+
if (sRequiredNamed.contains(sNamed[sIndex++])) return false;
2135+
}
21322136
}
21332137
return true;
21342138
} finally {

‎pkg/compiler/lib/src/ir/visitors.dart‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,10 @@ class DartTypeConverter extends ir.DartTypeVisitor<DartType> {
130130
.skip(node.requiredParameterCount)
131131
.toList()),
132132
node.namedParameters.map((n) => n.name).toList(),
133-
_options.useLegacySubtyping
134-
? const <String>{}
135-
: node.namedParameters
136-
.where((n) => n.isRequired)
137-
.map((n) => n.name)
138-
.toSet(),
133+
node.namedParameters
134+
.where((n) => n.isRequired)
135+
.map((n) => n.name)
136+
.toSet(),
139137
node.namedParameters.map((n) => visitType(n.type)).toList(),
140138
typeVariables ?? const <FunctionTypeVariable>[]);
141139
DartType type = _convertNullability(functionType, node.nullability);

‎pkg/compiler/lib/src/js_model/element_map_impl.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
917917
for (ir.VariableDeclaration variable in sortedNamedParameters) {
918918
namedParameters.add(variable.name);
919919
namedParameterTypes.add(getParameterType(variable));
920-
if (variable.isRequired && !options.useLegacySubtyping) {
920+
if (variable.isRequired) {
921921
requiredNamedParameters.add(variable.name);
922922
}
923923
}

‎pkg/compiler/lib/src/kernel/element_map_impl.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ class KernelToElementMapImpl implements KernelToElementMap, IrToElementMap {
539539
for (ir.VariableDeclaration variable in sortedNamedParameters) {
540540
namedParameters.add(variable.name);
541541
namedParameterTypes.add(getParameterType(variable));
542-
if (variable.isRequired && !options.useLegacySubtyping) {
542+
if (variable.isRequired) {
543543
requiredNamedParameters.add(variable.name);
544544
}
545545
}

‎sdk/lib/_internal/js_runtime/lib/rti.dart‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2913,22 +2913,26 @@ bool _isFunctionSubtype(
29132913
String sName = _Utils.asString(_Utils.arrayAt(sNamed, sIndex));
29142914
sIndex += 3;
29152915
if (_Utils.stringLessThan(tName, sName)) return false;
2916-
bool sIsRequired = _Utils.asBool(_Utils.arrayAt(sNamed, sIndex - 2));
2916+
bool sIsRequired = !JS_GET_FLAG('LEGACY') &&
2917+
_Utils.asBool(_Utils.arrayAt(sNamed, sIndex - 2));
29172918
if (_Utils.stringLessThan(sName, tName)) {
29182919
if (sIsRequired) return false;
29192920
continue;
29202921
}
2921-
bool tIsRequired = _Utils.asBool(_Utils.arrayAt(tNamed, tIndex + 1));
2922+
bool tIsRequired = !JS_GET_FLAG('LEGACY') &&
2923+
_Utils.asBool(_Utils.arrayAt(tNamed, tIndex + 1));
29222924
if (sIsRequired && !tIsRequired) return false;
29232925
Rti sType = _Utils.asRti(_Utils.arrayAt(sNamed, sIndex - 1));
29242926
Rti tType = _Utils.asRti(_Utils.arrayAt(tNamed, tIndex + 2));
29252927
if (!_isSubtype(universe, tType, tEnv, sType, sEnv)) return false;
29262928
break;
29272929
}
29282930
}
2929-
while (sIndex < sNamedLength) {
2930-
if (_Utils.asBool(_Utils.arrayAt(sNamed, sIndex + 1))) return false;
2931-
sIndex += 3;
2931+
if (!JS_GET_FLAG('LEGACY')) {
2932+
while (sIndex < sNamedLength) {
2933+
if (_Utils.asBool(_Utils.arrayAt(sNamed, sIndex + 1))) return false;
2934+
sIndex += 3;
2935+
}
29322936
}
29332937
return true;
29342938
}

‎tests/dart2js/internal/rti/required_named_parameters_test.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ main() {
5151

5252
// Subtype may not redeclare optional parameters as required
5353
rti2 = rti.testingUniverseEval(universe, "@(A,{a!A,b!A,c!A})");
54-
Expect.isFalse(rti.testingIsSubtype(universe, rti2, rti1));
54+
Expect.equals(isWeakMode, rti.testingIsSubtype(universe, rti2, rti1));
5555

5656
// Subtype may not declare new required named parameters
5757
rti2 = rti.testingUniverseEval(universe, "@(A,{a!A,b:A,c!A,d!A})");
58-
Expect.isFalse(rti.testingIsSubtype(universe, rti2, rti1));
58+
Expect.equals(isWeakMode, rti.testingIsSubtype(universe, rti2, rti1));
5959

6060
// Rti.toString() appears as expected
6161
Expect.equals('(B, {required B a, B b, required B c}) => dynamic',

‎tests/dart2js_2/internal/rti/required_named_parameters_test.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ main() {
5151

5252
// Subtype may not redeclare optional parameters as required
5353
rti2 = rti.testingUniverseEval(universe, "@(A,{a!A,b!A,c!A})");
54-
Expect.isFalse(rti.testingIsSubtype(universe, rti2, rti1));
54+
Expect.equals(isWeakMode, rti.testingIsSubtype(universe, rti2, rti1));
5555

5656
// Subtype may not declare new required named parameters
5757
rti2 = rti.testingUniverseEval(universe, "@(A,{a!A,b:A,c!A,d!A})");
58-
Expect.isFalse(rti.testingIsSubtype(universe, rti2, rti1));
58+
Expect.equals(isWeakMode, rti.testingIsSubtype(universe, rti2, rti1));
5959

6060
// Rti.toString() appears as expected
6161
Expect.equals('(B, {required B a, B b, required B c}) => dynamic',

0 commit comments

Comments
 (0)