Skip to content

Commit b628d00

Browse files
rakudramaCommit Bot
authored and
Commit Bot
committed
[dart2js] Correct evaluation order of named arguments in redirecting/super initializers
Minor refactoring to share building the map from argument names to compiled instructions. Fixes #47047 Change-Id: I20b8d37673bd4cb18aa8b2091af79e40ec498370 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/223381 Reviewed-by: Mayank Patke <[email protected]> Commit-Queue: Stephen Adams <[email protected]>
1 parent 0868f39 commit b628d00

File tree

1 file changed

+23
-40
lines changed

1 file changed

+23
-40
lines changed

pkg/compiler/lib/src/ssa/builder_kernel.dart

Lines changed: 23 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,35 +1074,18 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
10741074
arguments.positional[positionalIndex++].accept(this);
10751075
builtArguments.add(pop());
10761076
} else {
1077-
ConstantValue constantValue = _elementMap.getConstantValue(
1078-
member, parameter.initializer,
1079-
implicitNull: true);
1080-
assert(
1081-
constantValue != null,
1082-
failedAt(_elementMap.getMethod(function.parent),
1083-
'No constant computed for $parameter'));
1084-
builtArguments.add(graph.addConstant(constantValue, closedWorld));
1077+
builtArguments.add(_defaultValueForParameter(member, parameter));
10851078
}
10861079
});
1080+
// Evaluate named arguments in given order.
1081+
Map<String, HInstruction> namedArguments = _visitNamedArguments(arguments);
1082+
// And add them to `builtArguments` in calling-convention order.
10871083
function.namedParameters.toList()
10881084
..sort(namedOrdering)
10891085
..forEach((ir.VariableDeclaration parameter) {
1090-
var correspondingNamed = arguments.named.firstWhere(
1091-
(named) => named.name == parameter.name,
1092-
orElse: () => null);
1093-
if (correspondingNamed != null) {
1094-
correspondingNamed.value.accept(this);
1095-
builtArguments.add(pop());
1096-
} else {
1097-
ConstantValue constantValue = _elementMap.getConstantValue(
1098-
member, parameter.initializer,
1099-
implicitNull: true);
1100-
assert(
1101-
constantValue != null,
1102-
failedAt(_elementMap.getMethod(function.parent),
1103-
'No constant computed for $parameter'));
1104-
builtArguments.add(graph.addConstant(constantValue, closedWorld));
1105-
}
1086+
var argument = namedArguments[parameter.name];
1087+
argument ??= _defaultValueForParameter(member, parameter);
1088+
builtArguments.add(argument);
11061089
});
11071090

11081091
return builtArguments;
@@ -3586,7 +3569,8 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
35863569
}
35873570
}
35883571

3589-
/// Extracts the list of instructions for the positional subset of arguments.
3572+
/// Generate instructions to evaluate the positional arguments in source
3573+
/// order.
35903574
List<HInstruction> _visitPositionalArguments(ir.Arguments arguments) {
35913575
List<HInstruction> result = [];
35923576
for (ir.Expression argument in arguments.positional) {
@@ -3596,6 +3580,17 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
35963580
return result;
35973581
}
35983582

3583+
/// Generate instructions to evaluate the named arguments in source order.
3584+
/// Returns a fresh map from parameter name to evaluated argument.
3585+
Map<String, HInstruction> _visitNamedArguments(ir.Arguments arguments) {
3586+
Map<String, HInstruction> values = {};
3587+
for (ir.NamedExpression argument in arguments.named) {
3588+
argument.value.accept(this);
3589+
values[argument.name] = pop();
3590+
}
3591+
return values;
3592+
}
3593+
35993594
/// Builds the list of instructions for the expressions in the arguments to a
36003595
/// dynamic target (member function). Dynamic targets use stubs to add
36013596
/// defaulted arguments, so (unlike static targets) we do not add the default
@@ -3606,11 +3601,7 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
36063601
List<HInstruction> values = _visitPositionalArguments(arguments);
36073602

36083603
if (arguments.named.isNotEmpty) {
3609-
Map<String, HInstruction> namedValues = {};
3610-
for (ir.NamedExpression argument in arguments.named) {
3611-
argument.value.accept(this);
3612-
namedValues[argument.name] = pop();
3613-
}
3604+
Map<String, HInstruction> namedValues = _visitNamedArguments(arguments);
36143605
for (String name in selector.callStructure.getOrderedNamedArguments()) {
36153606
values.add(namedValues[name]);
36163607
}
@@ -3638,11 +3629,7 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
36383629
if (function is ConstructorEntity && function.isFactoryConstructor) {
36393630
// TODO(sra): Have a "CompiledArguments" structure to just update with
36403631
// what values we have rather than creating a map and de-populating it.
3641-
Map<String, HInstruction> namedValues = {};
3642-
for (ir.NamedExpression argument in arguments.named) {
3643-
argument.value.accept(this);
3644-
namedValues[argument.name] = pop();
3645-
}
3632+
Map<String, HInstruction> namedValues = _visitNamedArguments(arguments);
36463633

36473634
// Visit named arguments in parameter-position order, selecting provided
36483635
// or default value.
@@ -3719,11 +3706,7 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
37193706
}
37203707

37213708
if (parameterStructure.namedParameters.isNotEmpty) {
3722-
Map<String, HInstruction> namedValues = {};
3723-
for (ir.NamedExpression argument in arguments.named) {
3724-
argument.value.accept(this);
3725-
namedValues[argument.name] = pop();
3726-
}
3709+
Map<String, HInstruction> namedValues = _visitNamedArguments(arguments);
37273710

37283711
// Visit named arguments in parameter-position order, selecting provided
37293712
// or default value.

0 commit comments

Comments
 (0)