Skip to content

Commit 92bf76d

Browse files
sstricklCommit Queue
authored and
Commit Queue
committed
[pkg/vm] Handle switch statements in unreachable code eliminator.
Namely, if the tested expression for a switch statement is constant, then we can remove any constant cases where the constants differ, and if all but a single case is removed, we can replace the switch with the case body. If constant functions are not enabled, then getters annotated with @pragma("vm:platform-const") are still evaluated with the constant function evaluation machinery, but only those and no others (including any functions called within an annotated getter). This way, functions can be annotated with @pragma("vm:platform-const") without having to rewrite them to be a single returned expression. TEST=pkg/vm/test/transformations/unreachable_code_elimination pkg/vm/test/transformations/vm_constant_evaluator Issue: #50473 Issue: #31969 Change-Id: Ie290d2f1f469326238d66c3d9631f8e696685ff0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332760 Commit-Queue: Tess Strickland <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]> Reviewed-by: Alexander Markov <[email protected]>
1 parent 2b36aff commit 92bf76d

12 files changed

+1120
-156
lines changed

pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2631,6 +2631,11 @@ class ConstantEvaluator implements ExpressionVisitor<Constant> {
26312631

26322632
/// Execute a function body using the [StatementConstantEvaluator].
26332633
Constant executeBody(Statement statement) {
2634+
if (!enableConstFunctions && !inExtensionTypeConstConstructor) {
2635+
throw new UnsupportedError("Statement evaluation is only supported when "
2636+
"in extension type const constructors or when the const functions "
2637+
"feature is enabled.");
2638+
}
26342639
StatementConstantEvaluator statementEvaluator =
26352640
new StatementConstantEvaluator(this);
26362641
ExecutionStatus status = statement.accept(statementEvaluator);
@@ -2657,6 +2662,11 @@ class ConstantEvaluator implements ExpressionVisitor<Constant> {
26572662
/// Returns [null] on success and an error-"constant" on failure, as such the
26582663
/// return value should be checked.
26592664
AbortConstant? executeConstructorBody(Constructor constructor) {
2665+
if (!enableConstFunctions && !inExtensionTypeConstConstructor) {
2666+
throw new UnsupportedError("Statement evaluation is only supported when "
2667+
"in extension type const constructors or when the const functions "
2668+
"feature is enabled.");
2669+
}
26602670
final Statement body = constructor.function.body!;
26612671
StatementConstantEvaluator statementEvaluator =
26622672
new StatementConstantEvaluator(this);
@@ -5477,14 +5487,7 @@ class ConstantEvaluator implements ExpressionVisitor<Constant> {
54775487
class StatementConstantEvaluator implements StatementVisitor<ExecutionStatus> {
54785488
ConstantEvaluator exprEvaluator;
54795489

5480-
StatementConstantEvaluator(this.exprEvaluator) {
5481-
if (!exprEvaluator.enableConstFunctions &&
5482-
!exprEvaluator.inExtensionTypeConstConstructor) {
5483-
throw new UnsupportedError("Statement evaluation is only supported when "
5484-
"in inline class const constructors or when the const functions "
5485-
"feature is enabled.");
5486-
}
5487-
}
5490+
StatementConstantEvaluator(this.exprEvaluator);
54885491

54895492
/// Evaluate the expression using the [ConstantEvaluator].
54905493
Constant evaluate(Expression expr) => expr.accept(exprEvaluator);

pkg/vm/lib/transformations/unreachable_code_elimination.dart

Lines changed: 122 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,32 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
3636
return result;
3737
}
3838

39-
bool _isBoolConstant(Expression node) =>
40-
node is BoolLiteral ||
41-
(node is ConstantExpression && node.constant is BoolConstant);
42-
43-
bool _getBoolConstantValue(Expression node) {
44-
if (node is BoolLiteral) {
45-
return node.value;
46-
}
47-
if (node is ConstantExpression) {
48-
final constant = node.constant;
49-
if (constant is BoolConstant) {
50-
return constant.value;
51-
}
39+
bool? _getBoolConstantValue(Expression node) {
40+
if (node is BoolLiteral) return node.value;
41+
if (node is! ConstantExpression) return null;
42+
final constant = node.constant;
43+
return constant is BoolConstant ? constant.value : null;
44+
}
45+
46+
Expression _makeConstantExpression(Constant constant, Expression node) {
47+
if (constant is UnevaluatedConstant &&
48+
constant.expression is InvalidExpression) {
49+
return constant.expression;
50+
}
51+
ConstantExpression constantExpression = new ConstantExpression(
52+
constant, node.getStaticType(_staticTypeContext!))
53+
..fileOffset = node.fileOffset;
54+
if (node is FileUriExpression) {
55+
return new FileUriConstantExpression(constantExpression.constant,
56+
type: constantExpression.type, fileUri: node.fileUri)
57+
..fileOffset = node.fileOffset;
5258
}
53-
throw 'Expected bool constant: $node';
59+
return constantExpression;
5460
}
5561

56-
Expression _createBoolLiteral(bool value, int fileOffset) =>
57-
new BoolLiteral(value)..fileOffset = fileOffset;
62+
Expression _createBoolConstantExpression(bool value, Expression node) =>
63+
_makeConstantExpression(
64+
constantEvaluator.canonicalize(BoolConstant(value)), node);
5865

5966
Statement _makeEmptyBlockIfEmptyStatement(Statement node, TreeNode parent) =>
6067
node is EmptyStatement ? (Block(<Statement>[])..parent = parent) : node;
@@ -63,8 +70,8 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
6370
TreeNode visitIfStatement(IfStatement node, TreeNode? removalSentinel) {
6471
node.transformOrRemoveChildren(this);
6572
final condition = node.condition;
66-
if (_isBoolConstant(condition)) {
67-
final value = _getBoolConstantValue(condition);
73+
final value = _getBoolConstantValue(condition);
74+
if (value != null) {
6875
return value
6976
? node.then
7077
: (node.otherwise ?? removalSentinel ?? new EmptyStatement());
@@ -78,8 +85,8 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
7885
ConditionalExpression node, TreeNode? removalSentinel) {
7986
node.transformOrRemoveChildren(this);
8087
final condition = node.condition;
81-
if (_isBoolConstant(condition)) {
82-
final value = _getBoolConstantValue(condition);
88+
final value = _getBoolConstantValue(condition);
89+
if (value != null) {
8390
return value ? node.then : node.otherwise;
8491
}
8592
return node;
@@ -89,9 +96,9 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
8996
TreeNode visitNot(Not node, TreeNode? removalSentinel) {
9097
node.transformOrRemoveChildren(this);
9198
final operand = node.operand;
92-
if (_isBoolConstant(operand)) {
93-
return _createBoolLiteral(
94-
!_getBoolConstantValue(operand), node.fileOffset);
99+
final value = _getBoolConstantValue(operand);
100+
if (value != null) {
101+
return _createBoolConstantExpression(!value, node);
95102
}
96103
return node;
97104
}
@@ -100,47 +107,89 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
100107
TreeNode visitLogicalExpression(
101108
LogicalExpression node, TreeNode? removalSentinel) {
102109
node.transformOrRemoveChildren(this);
103-
final left = node.left;
104-
final right = node.right;
105-
final operatorEnum = node.operatorEnum;
106-
if (_isBoolConstant(left)) {
107-
final leftValue = _getBoolConstantValue(left);
108-
if (_isBoolConstant(right)) {
109-
final rightValue = _getBoolConstantValue(right);
110-
if (operatorEnum == LogicalExpressionOperator.OR) {
111-
return _createBoolLiteral(leftValue || rightValue, node.fileOffset);
112-
} else if (operatorEnum == LogicalExpressionOperator.AND) {
113-
return _createBoolLiteral(leftValue && rightValue, node.fileOffset);
114-
} else {
115-
throw 'Unexpected LogicalExpression operator ${operatorEnum}: $node';
116-
}
117-
} else {
118-
if (leftValue && operatorEnum == LogicalExpressionOperator.OR) {
119-
return _createBoolLiteral(true, node.fileOffset);
120-
} else if (!leftValue &&
121-
operatorEnum == LogicalExpressionOperator.AND) {
122-
return _createBoolLiteral(false, node.fileOffset);
110+
bool? value = _getBoolConstantValue(node.left);
111+
// Because of short-circuiting, these operators cannot be treated as
112+
// symmetric, so a non-constant left and a constant right is left as-is.
113+
if (value == null) return node;
114+
switch (node.operatorEnum) {
115+
case LogicalExpressionOperator.OR:
116+
return value ? node.left : node.right;
117+
case LogicalExpressionOperator.AND:
118+
return value ? node.right : node.left;
119+
}
120+
}
121+
122+
@override
123+
TreeNode visitSwitchStatement(
124+
SwitchStatement node, TreeNode? removalSentinel) {
125+
node.transformOrRemoveChildren(this);
126+
final tested = node.expression;
127+
if (tested is! ConstantExpression) return node;
128+
129+
// Set of SwitchCases that should be retained, either because they are
130+
// guaranteed to match, may match and no case before them is guaranteed
131+
// to match, or because they are the target of a case that may match or
132+
// is guaranteed to match.
133+
final toKeep = <SwitchCase>{};
134+
// Whether an expression from a previous case is guaranteed to match.
135+
bool foundMatchingCase = false;
136+
for (final c in node.cases) {
137+
// Trim any constant expressions that don't match, or any expression
138+
// that follows a guaranteed-to-match case. Perform this trimming even
139+
// on cases that follow a guaranteed to match case and thus are not
140+
// in the initial set to retain, since they may be the target of continue
141+
// statements and thus added to the set to retain later.
142+
bool containsMatchingCase = false;
143+
c.expressions.retainWhere((e) {
144+
if (foundMatchingCase) return false;
145+
if (e is! ConstantExpression) return true;
146+
containsMatchingCase = e.constant == tested.constant;
147+
return containsMatchingCase;
148+
});
149+
if (foundMatchingCase) continue;
150+
foundMatchingCase = containsMatchingCase;
151+
if (c.isDefault || c.expressions.isNotEmpty) {
152+
toKeep.add(c);
153+
}
154+
}
155+
156+
if (toKeep.isEmpty) {
157+
throw 'Expected at least one possibly matching case: $node';
158+
}
159+
160+
// Now iteratively keep cases that are targets of continue statements
161+
// within the cases we've already marked to keep.
162+
final worklist = [...toKeep];
163+
final collector = ContinueSwitchStatementTargetCollector(node);
164+
while (worklist.isNotEmpty) {
165+
final next = worklist.removeLast();
166+
final targets = collector.collectTargets(next);
167+
for (final target in targets) {
168+
if (toKeep.add(target)) {
169+
worklist.add(target);
123170
}
124171
}
125172
}
173+
174+
node.cases.retainWhere((c) => toKeep.contains(c));
175+
if (node.cases.length == 1) {
176+
return node.cases.first.body;
177+
}
126178
return node;
127179
}
128180

129181
@override
130-
visitStaticGet(StaticGet node, TreeNode? removalSentinel) {
182+
TreeNode visitStaticGet(StaticGet node, TreeNode? removalSentinel) {
131183
node.transformOrRemoveChildren(this);
132184
final target = node.target;
133185
if (target is Field && target.isConst) {
134186
throw 'StaticGet from const field $target should be evaluated by front-end: $node';
135187
}
136-
if (constantEvaluator.transformerShouldEvaluateExpression(node)) {
137-
final context = _staticTypeContext!;
138-
final result = constantEvaluator.evaluate(context, node);
139-
assert(result is! UnevaluatedConstant);
140-
return new ConstantExpression(result, node.getStaticType(context))
141-
..fileOffset = node.fileOffset;
188+
if (!constantEvaluator.transformerShouldEvaluateExpression(node)) {
189+
return node;
142190
}
143-
return node;
191+
final result = constantEvaluator.evaluate(_staticTypeContext!, node);
192+
return _makeConstantExpression(result, node);
144193
}
145194

146195
@override
@@ -235,3 +284,25 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
235284
return node;
236285
}
237286
}
287+
288+
class ContinueSwitchStatementTargetCollector extends RecursiveVisitor {
289+
final SwitchStatement parent;
290+
late Set<SwitchCase> collected;
291+
292+
ContinueSwitchStatementTargetCollector(this.parent);
293+
294+
Set<SwitchCase> collectTargets(SwitchCase node) {
295+
collected = {};
296+
node.accept(this);
297+
return collected;
298+
}
299+
300+
@override
301+
void visitContinueSwitchStatement(ContinueSwitchStatement node) {
302+
node.visitChildren(this);
303+
// Only keep targets that are within the original node being checked.
304+
if (node.target.parent == parent) {
305+
collected.add(node.target);
306+
}
307+
}
308+
}

pkg/vm/lib/transformations/vm_constant_evaluator.dart

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,14 @@ import 'package:front_end/src/base/nnbd_mode.dart';
1313
import 'package:front_end/src/fasta/kernel/constant_evaluator.dart'
1414
show
1515
AbortConstant,
16+
AbortStatus,
1617
ConstantEvaluator,
1718
ErrorReporter,
1819
EvaluationMode,
19-
SimpleErrorReporter;
20+
ProceedStatus,
21+
ReturnStatus,
22+
SimpleErrorReporter,
23+
StatementConstantEvaluator;
2024

2125
import '../target_os.dart';
2226

@@ -26,8 +30,10 @@ import '../target_os.dart';
2630
/// [targetOS] represents the target operating system and is used when
2731
/// evaluating static fields and getters annotated with "vm:platform-const".
2832
///
29-
/// If [enableConstFunctions] is false, then only getters that return the
30-
/// result of a single expression can be evaluated.
33+
/// To avoid restricting getters annotated with "vm:platform-const" to be just
34+
/// a single return statement whose body is evaluated, we treat annotated
35+
/// getters as const functions. If [enableConstFunctions] is false, then
36+
/// only annotated getters are treated this way.
3137
class VMConstantEvaluator extends ConstantEvaluator {
3238
final TargetOS? _targetOS;
3339
final Map<String, Constant> _constantFields = {};
@@ -124,6 +130,19 @@ class VMConstantEvaluator extends ConstantEvaluator {
124130
bool transformerShouldEvaluateExpression(Expression node) =>
125131
_targetOS != null && node is StaticGet && _isPlatformConst(node.target);
126132

133+
Constant _executePlatformConstBody(Statement statement) {
134+
final status = statement.accept(StatementConstantEvaluator(this));
135+
if (status is AbortStatus) return status.error;
136+
// Happens if there is no return statement in a void Function(...) body.
137+
if (status is ProceedStatus) return canonicalize(NullConstant());
138+
if (status is ReturnStatus) {
139+
final value = status.value;
140+
return value != null ? value : canonicalize(NullConstant());
141+
}
142+
// Other statuses denote intermediate states and not final ones.
143+
throw 'No valid constant returned after executing $statement';
144+
}
145+
127146
@override
128147
Constant visitStaticGet(StaticGet node) {
129148
assert(_targetOS != null);
@@ -153,20 +172,7 @@ class VMConstantEvaluator extends ConstantEvaluator {
153172
result = evaluateExpressionInContext(target, target.initializer!);
154173
} else if (target is Procedure && target.isGetter) {
155174
final body = target.function.body!;
156-
// If const functions are enabled, execute the getter as if it were
157-
// a const function. Otherwise the annotated getter must be a single
158-
// return statement whose expression is evaluated.
159-
if (enableConstFunctions) {
160-
result = executeBody(body);
161-
} else if (body is ReturnStatement) {
162-
if (body.expression == null) {
163-
return canonicalize(NullConstant());
164-
}
165-
result = evaluateExpressionInContext(target, body.expression!);
166-
} else {
167-
throw "Cannot evaluate method '$nameText' since it contains more "
168-
"than a single return statement.";
169-
}
175+
result = _executePlatformConstBody(body);
170176
}
171177
if (result is AbortConstant) {
172178
throw "The body or initialization of member '$nameText' does not "

0 commit comments

Comments
 (0)