Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 22718f4

Browse files
askeksacommit-bot@chromium.org
authored andcommitted
[CFE] Always call the constant evaluator by the evaluate method.
Reland of https://dart-review.googlesource.com/c/sdk/+/96321 Change-Id: Ifb2b955d500c18a470503bad9f62f47e4c6a1fc5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97117 Commit-Queue: Aske Simon Christensen <[email protected]> Reviewed-by: Kevin Millikin <[email protected]>
1 parent c10ee99 commit 22718f4

File tree

3 files changed

+11
-29
lines changed

3 files changed

+11
-29
lines changed

pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3840,7 +3840,7 @@ class ProgramCompiler extends Object
38403840
visitStaticGet(StaticGet node) {
38413841
var target = node.target;
38423842
if (target is Field && target.isConst) {
3843-
var value = _constants.evaluate(target.initializer, cache: true);
3843+
var value = _constants.evaluate(target.initializer);
38443844
if (value is PrimitiveConstant) return value.accept(this);
38453845
}
38463846

pkg/dev_compiler/lib/src/kernel/constants.dart

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,11 @@ class DevCompilerConstants {
3434
/// failed, or if the constant was unavailable.
3535
///
3636
/// Returns [NullConstant] to represent the `null` value.
37-
///
38-
/// To avoid performance costs associated with try+catch on invalid constant
39-
/// evaluation, call this after [isConstant] is known to be true.
40-
Constant evaluate(Expression e, {bool cache = false}) {
37+
Constant evaluate(Expression e) {
4138
if (e == null) return null;
4239

43-
try {
44-
var result = cache ? _evaluator.evaluate(e) : e.accept(_evaluator);
45-
return result is UnevaluatedConstant ? null : result;
46-
} on _AbortCurrentEvaluation {
47-
// TODO(jmesserly): the try+catch is necessary because the front end is
48-
// not issuing sufficient errors, so the constant evaluation can fail.
49-
//
50-
// It can also be caused by methods in the evaluator that don't understand
51-
// unavailable constants.
52-
return null;
53-
} on NoSuchMethodError {
54-
// TODO(jmesserly): this is probably the same issue as above, but verify
55-
// that it's fixed once Kernel does constant evaluation.
56-
return null;
57-
}
40+
Constant result = _evaluator.evaluate(e);
41+
return result is UnevaluatedConstant ? null : result;
5842
}
5943

6044
/// If [node] is an annotation with a field named `name`, returns that field's
@@ -184,12 +168,7 @@ class DevCompilerConstantsBackend extends ConstantsBackend {
184168
class _ErrorReporter extends SimpleErrorReporter {
185169
const _ErrorReporter();
186170

171+
// Ignore reported errors.
187172
@override
188-
report(context, message, node) => throw const _AbortCurrentEvaluation();
189-
}
190-
191-
// TODO(jmesserly): this class is private in Kernel constants library, so
192-
// we have our own version.
193-
class _AbortCurrentEvaluation {
194-
const _AbortCurrentEvaluation();
173+
report(context, message, node) => message;
195174
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,10 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
411411
nodeCache = <Node, Constant>{},
412412
env = new EvaluationEnvironment();
413413

414-
/// Evaluates [node] and possibly cache the evaluation result.
414+
/// Evaluate [node] and possibly cache the evaluation result.
415+
/// Returns UnevaluatedConstant if the constant could not be evaluated.
416+
/// If the expression in the UnevaluatedConstant is an InvalidExpression,
417+
/// an error occurred during constant evaluation.
415418
Constant evaluate(Expression node) {
416419
evaluationRoot = node;
417420
try {
@@ -459,7 +462,7 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
459462
return unevaluatedNodes != null && unevaluatedNodes.contains(node);
460463
}
461464

462-
/// Evaluates [node] and possibly cache the evaluation result.
465+
/// Evaluate [node] and possibly cache the evaluation result.
463466
/// @throws _AbortCurrentEvaluation if expression can't be evaluated.
464467
Constant _evaluateSubexpression(Expression node) {
465468
if (node == null) return nullConstant;

0 commit comments

Comments
 (0)