Skip to content

Commit 0eac43b

Browse files
sstricklCommit Queue
authored andcommitted
[pkg/vm] Enable const functions during platform const field evaluation.
This allows more complicated field initializers that are written using immediately invoked closures, like @pragma("vm:platform-const") final bool foo = () { ... do some stuff ... }(); to be properly evaluated by the VM constant evaluator. Also throws errors at compile time if the annotated member cannot be evaluated to a constant or if an invalid member (not an initialized static field or a static getter) is annotated. TEST=pkg/vm/test/transformations/vm_constant_evaluator_test Issue: #31969 Issue: #50473 Issue: flutter/flutter#14233 Change-Id: I14be498bb5f7771f0f339baf7d3b1bec7df5903f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/348380 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Tess Strickland <[email protected]> Reviewed-by: Alexander Markov <[email protected]>
1 parent f992bfc commit 0eac43b

File tree

47 files changed

+876
-503
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+876
-503
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2451,7 +2451,7 @@ class ConstantEvaluator implements ExpressionVisitor<Constant> {
24512451

24522452
final bool enableTripleShift;
24532453
final bool enableAsserts;
2454-
bool enableConstFunctions;
2454+
final bool enableConstFunctions;
24552455
bool inExtensionTypeConstConstructor = false;
24562456

24572457
final Map<Constant, Constant> canonicalizationCache;

pkg/vm/lib/transformations/unreachable_code_elimination.dart

Lines changed: 65 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,56 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
3232
SimpleUnreachableCodeElimination(this.constantEvaluator,
3333
{required this.enableAsserts, required this.soundNullSafety});
3434

35+
bool _shouldEvaluateMember(Member node) =>
36+
constantEvaluator.hasTargetOS && constantEvaluator.isPlatformConst(node);
37+
38+
Never _throwPlatformConstError(Member node, String message) {
39+
final uri = constantEvaluator.getFileUri(node);
40+
final offset = constantEvaluator.getFileOffset(uri, node);
41+
throw PlatformConstError(message, node, uri, offset);
42+
}
43+
44+
void _checkPlatformConstMember(Member node) {
45+
if (node is Field) {
46+
if (!node.isStatic) {
47+
_throwPlatformConstError(node, 'not a static field');
48+
}
49+
// Static fields currently always have an initializer set, even if it's
50+
// the implicit null initializer for a nullable field.
51+
assert(node.initializer != null);
52+
} else if (node is Procedure) {
53+
if (!node.isStatic) {
54+
_throwPlatformConstError(node, 'not a static method');
55+
}
56+
if (!node.isGetter) {
57+
_throwPlatformConstError(node, 'not a getter');
58+
}
59+
} else {
60+
_throwPlatformConstError(node, 'not a field or method');
61+
}
62+
}
63+
3564
@override
3665
TreeNode defaultMember(Member node, TreeNode? removalSentinel) {
3766
_staticTypeContext =
3867
StaticTypeContext(node, constantEvaluator.typeEnvironment);
68+
if (_shouldEvaluateMember(node)) {
69+
_checkPlatformConstMember(node);
70+
// Create a StaticGet to ensure the member is evaluated at least once,
71+
// and then replace the field initializer or getter body with the result.
72+
final staticGet = StaticGet(node)..fileOffset = node.fileOffset;
73+
final result =
74+
staticGet.accept1(this, cannotRemoveSentinel) as ConstantExpression;
75+
if (node is Field) {
76+
node.initializer = result
77+
..fileOffset = node.initializer!.fileOffset
78+
..parent = node;
79+
} else if (node is Procedure) {
80+
node.function.body = ReturnStatement(result)
81+
..fileOffset = node.function.body!.fileOffset
82+
..parent = node.function;
83+
}
84+
}
3985
final result = super.defaultMember(node, removalSentinel);
4086
_staticTypeContext = null;
4187
return result;
@@ -205,20 +251,17 @@ class SimpleUnreachableCodeElimination extends RemovingTransformer {
205251
throw 'StaticGet from const field $target should be evaluated by front-end: $node';
206252
}
207253

208-
if (!constantEvaluator.hasTargetOS ||
209-
!constantEvaluator.isPlatformConst(target)) {
254+
if (!_shouldEvaluateMember(target)) {
210255
return super.visitStaticGet(node, removalSentinel);
211256
}
212257

213-
final result = constantEvaluator.evaluate(_staticTypeContext!, node);
214-
215-
if (result is UnevaluatedConstant &&
216-
result.expression is InvalidExpression) {
217-
return result.expression;
258+
_checkPlatformConstMember(target);
259+
final constant = constantEvaluator.evaluate(_staticTypeContext!, node);
260+
if (constant is UnevaluatedConstant) {
261+
_throwPlatformConstError(target, 'cannot evaluate to a constant');
218262
}
219-
220263
final type = node.getStaticType(_staticTypeContext!);
221-
return ConstantExpression(result, type)..fileOffset = node.fileOffset;
264+
return ConstantExpression(constant, type)..fileOffset = node.fileOffset;
222265
}
223266

224267
@override
@@ -335,3 +378,16 @@ class ContinueSwitchStatementTargetCollector extends RecursiveVisitor {
335378
}
336379
}
337380
}
381+
382+
class PlatformConstError extends Error {
383+
final Object? message;
384+
final Member member;
385+
final Uri? uri;
386+
final int offset;
387+
388+
PlatformConstError(this.message, this.member, this.uri, this.offset);
389+
390+
@override
391+
String toString() => '${uri ?? ''}:$offset '
392+
'Error for annotated member ${member.name}: $message';
393+
}

pkg/vm/lib/transformations/vm_constant_evaluator.dart

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ import 'pragma.dart';
2323
/// fields and getters annotated with "vm:platform-const".
2424
///
2525
/// To avoid restricting getters annotated with "vm:platform-const" to be just
26-
/// a single return statement whose body is evaluated, we treat annotated
27-
/// getters as const functions. If [enableConstFunctions] is false, then
28-
/// only annotated getters are treated this way.
26+
/// a single return statement whose body is evaluated, as well as handling
27+
/// immediately-invoked closures wrapping complex initializing code in field
28+
/// initializers, we enable constant evaluation of functions.
2929
class VMConstantEvaluator extends ConstantEvaluator {
3030
final TargetOS? _targetOS;
3131
final Map<String, Constant> _constantFields = {};
@@ -43,15 +43,16 @@ class VMConstantEvaluator extends ConstantEvaluator {
4343
this._targetOS,
4444
this._pragmaParser,
4545
{bool enableTripleShift = false,
46-
bool enableConstFunctions = false,
4746
bool enableAsserts = true,
4847
bool errorOnUnevaluatedConstant = false,
4948
EvaluationMode evaluationMode = EvaluationMode.weak})
5049
: _platformClass = typeEnvironment.coreTypes.platformClass,
5150
super(dartLibrarySupport, backend, component, environmentDefines,
5251
typeEnvironment, errorReporter,
5352
enableTripleShift: enableTripleShift,
54-
enableConstFunctions: enableConstFunctions,
53+
// We use evaluation of const functions for getters and immediately
54+
// invoked closures in field initializers.
55+
enableConstFunctions: true,
5556
enableAsserts: enableAsserts,
5657
errorOnUnevaluatedConstant: errorOnUnevaluatedConstant,
5758
evaluationMode: evaluationMode) {
@@ -68,10 +69,10 @@ class VMConstantEvaluator extends ConstantEvaluator {
6869
Target target, Component component, TargetOS? targetOS, NnbdMode nnbdMode,
6970
{bool evaluateAnnotations = true,
7071
bool enableTripleShift = false,
71-
bool enableConstFunctions = false,
7272
bool enableConstructorTearOff = false,
7373
bool enableAsserts = true,
7474
bool errorOnUnevaluatedConstant = false,
75+
ErrorReporter? errorReporter,
7576
Map<String, String>? environmentDefines,
7677
CoreTypes? coreTypes,
7778
ClassHierarchy? hierarchy}) {
@@ -90,11 +91,10 @@ class VMConstantEvaluator extends ConstantEvaluator {
9091
component,
9192
environmentDefines,
9293
typeEnvironment,
93-
const SimpleErrorReporter(),
94+
errorReporter ?? const SimpleErrorReporter(),
9495
targetOS,
9596
ConstantPragmaAnnotationParser(coreTypes, target),
9697
enableTripleShift: enableTripleShift,
97-
enableConstFunctions: enableConstFunctions,
9898
enableAsserts: enableAsserts,
9999
errorOnUnevaluatedConstant: errorOnUnevaluatedConstant,
100100
evaluationMode: EvaluationMode.fromNnbdMode(nnbdMode));
@@ -128,24 +128,14 @@ class VMConstantEvaluator extends ConstantEvaluator {
128128
}
129129
}
130130

131-
final initializer = target.initializer;
132-
if (initializer == null) {
133-
throw 'Cannot const evaluate annotated field with no initializer';
134-
}
131+
// The base class only evaluates const fields, so the VM constant
132+
// evaluator must manually request the initializer be evaluated.
133+
// instead of just calling super.visitStaticGet(target).
135134
return withNewEnvironment(
136-
() => evaluateExpressionInContext(target, initializer));
137-
}
138-
139-
if (target is Procedure && target.kind == ProcedureKind.Getter) {
140-
// Temporarily enable const functions and use the base class to evaluate
141-
// the getter with appropriate caching/recursive evaluation checks.
142-
final oldEnableConstFunctions = enableConstFunctions;
143-
enableConstFunctions = true;
144-
final result = super.visitStaticGet(node);
145-
enableConstFunctions = oldEnableConstFunctions;
146-
return result;
135+
() => evaluateExpressionInContext(target, target.initializer!));
147136
}
148137

149-
throw 'Expected annotated field with initializer or getter, got $target';
138+
// The base class already handles constant evaluation of a getter.
139+
return super.visitStaticGet(node);
150140
}
151141
}

0 commit comments

Comments
 (0)