[Kernel] Clean up error reporting in constant evaluation
Make the constant evaluator take an explicit error reporter so we have
to opt in to using the "simple" one that reports errors in an ad hoc
way. This is the start of a change to use Fasta-controlled error
messages throughout and eventually get rid of the simple error
handler, and to continue constant evaluation after the first constant
error.
Change-Id: If6b1801edab6063754b642cf4a603abf9d63103a
Reviewed-on: https://dart-review.googlesource.com/c/89501
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Auto-Submit: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/constants.dart b/pkg/dev_compiler/lib/src/kernel/constants.dart
index 52329f7..b05b840 100644
--- a/pkg/dev_compiler/lib/src/kernel/constants.dart
+++ b/pkg/dev_compiler/lib/src/kernel/constants.dart
@@ -187,8 +187,7 @@
{bool enableAsserts: false})
: unavailableConstant = InstanceConstant(null, [], {}),
super(_ConstantsBackend(types.coreTypes), types, types.coreTypes,
- enableAsserts,
- errorReporter: const _ErrorReporter()) {
+ enableAsserts, const _ErrorReporter()) {
env = EvaluationEnvironment();
}
@@ -311,7 +310,7 @@
lowerListConstant(constant) => constant;
}
-class _ErrorReporter extends ErrorReporterBase {
+class _ErrorReporter extends SimpleErrorReporter {
const _ErrorReporter();
@override
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
index 39ea072..75f57a8 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
@@ -44,7 +44,7 @@
import 'package:kernel/type_environment.dart' show TypeEnvironment;
import 'package:kernel/transformations/constants.dart' as constants
- show transformLibraries;
+ show SimpleErrorReporter, transformLibraries;
import '../../api_prototype/file_system.dart' show FileSystem;
@@ -754,7 +754,8 @@
new KernelConstantsBackend(),
loader.coreTypes,
new TypeEnvironment(loader.coreTypes, loader.hierarchy,
- legacyMode: false));
+ legacyMode: false),
+ const constants.SimpleErrorReporter());
ticker.logMs("Evaluated constants");
}
backendTarget.performModularTransformationsOnLibraries(
diff --git a/pkg/kernel/bin/transform.dart b/pkg/kernel/bin/transform.dart
index a2d90ae..157aa9c 100755
--- a/pkg/kernel/bin/transform.dart
+++ b/pkg/kernel/bin/transform.dart
@@ -12,13 +12,15 @@
import 'package:kernel/kernel.dart';
import 'package:kernel/src/tool/batch_util.dart';
import 'package:kernel/target/targets.dart';
-import 'package:kernel/transformations/constants.dart' as constants;
+
+import 'package:kernel/transformations/constants.dart' as constants
+ show SimpleErrorReporter, transformComponent;
+
import 'package:kernel/transformations/continuation.dart' as cont;
import 'package:kernel/transformations/empty.dart' as empty;
import 'package:kernel/transformations/method_call.dart' as method_call;
import 'package:kernel/transformations/mixin_full_resolution.dart' as mix;
import 'package:kernel/transformations/treeshaker.dart' as treeshaker;
-// import 'package:kernel/verifier.dart';
import 'package:kernel/transformations/coq.dart' as coq;
import 'package:kernel/vm/constants_native_effects.dart';
import 'package:kernel/src/tool/command_line_util.dart';
@@ -97,8 +99,9 @@
final Map<String, String> defines = null;
final VmConstantsBackend backend =
new VmConstantsBackend(defines, coreTypes);
- component =
- constants.transformComponent(component, backend, legacyMode: true);
+ component = constants.transformComponent(
+ component, backend, const constants.SimpleErrorReporter(),
+ legacyMode: true);
break;
case 'treeshake':
component = treeshaker.transformComponent(coreTypes, hierarchy, component,
diff --git a/pkg/kernel/lib/transformations/constants.dart b/pkg/kernel/lib/transformations/constants.dart
index 866b795..17bd047 100644
--- a/pkg/kernel/lib/transformations/constants.dart
+++ b/pkg/kernel/lib/transformations/constants.dart
@@ -28,37 +28,40 @@
import '../type_algebra.dart';
import '../type_environment.dart';
-Component transformComponent(Component component, ConstantsBackend backend,
+Component transformComponent(
+ Component component, ConstantsBackend backend, ErrorReporter errorReporter,
{bool keepFields: false,
bool legacyMode: false,
bool enableAsserts: false,
bool evaluateAnnotations: true,
CoreTypes coreTypes,
- ClassHierarchy hierarchy,
- ErrorReporter errorReporter: const _SimpleErrorReporter()}) {
+ ClassHierarchy hierarchy}) {
coreTypes ??= new CoreTypes(component);
hierarchy ??= new ClassHierarchy(component);
final typeEnvironment =
new TypeEnvironment(coreTypes, hierarchy, legacyMode: legacyMode);
- transformLibraries(component.libraries, backend, coreTypes, typeEnvironment,
+ transformLibraries(
+ component.libraries, backend, coreTypes, typeEnvironment, errorReporter,
keepFields: keepFields,
legacyMode: legacyMode,
enableAsserts: enableAsserts,
- evaluateAnnotations: evaluateAnnotations,
- errorReporter: errorReporter);
+ evaluateAnnotations: evaluateAnnotations);
return component;
}
-void transformLibraries(List<Library> libraries, ConstantsBackend backend,
- CoreTypes coreTypes, TypeEnvironment typeEnvironment,
+void transformLibraries(
+ List<Library> libraries,
+ ConstantsBackend backend,
+ CoreTypes coreTypes,
+ TypeEnvironment typeEnvironment,
+ ErrorReporter errorReporter,
{bool keepFields: false,
bool legacyMode: false,
bool keepVariables: false,
bool evaluateAnnotations: true,
- bool enableAsserts: false,
- ErrorReporter errorReporter: const _SimpleErrorReporter()}) {
+ bool enableAsserts: false}) {
final ConstantsTransformer constantsTransformer = new ConstantsTransformer(
backend,
keepFields,
@@ -95,8 +98,8 @@
ErrorReporter errorReporter,
{bool legacyMode: false})
: constantEvaluator = new ConstantEvaluator(
- backend, typeEnvironment, coreTypes, enableAsserts,
- legacyMode: legacyMode, errorReporter: errorReporter);
+ backend, typeEnvironment, coreTypes, enableAsserts, errorReporter,
+ legacyMode: legacyMode);
// Transform the library/class members:
@@ -388,10 +391,9 @@
InstanceBuilder instanceBuilder;
EvaluationEnvironment env;
- ConstantEvaluator(
- this.backend, this.typeEnvironment, this.coreTypes, this.enableAsserts,
- {this.legacyMode: false,
- this.errorReporter = const _SimpleErrorReporter()})
+ ConstantEvaluator(this.backend, this.typeEnvironment, this.coreTypes,
+ this.enableAsserts, this.errorReporter,
+ {this.legacyMode: false})
: canonicalizationCache = <Constant, Constant>{},
nodeCache = <Node, Constant>{};
@@ -1477,54 +1479,64 @@
abstract class ErrorReporter {
const ErrorReporter();
- freeTypeParameter(List<TreeNode> context, TreeNode node, DartType type);
- invalidDartType(List<TreeNode> context, TreeNode node, Constant receiver,
- DartType expectedType);
- invalidBinaryOperandType(List<TreeNode> context, TreeNode node,
- Constant receiver, String op, DartType expectedType, DartType actualType);
- invalidMethodInvocation(
- List<TreeNode> context, TreeNode node, Constant receiver, String op);
- invalidStaticInvocation(
- List<TreeNode> context, TreeNode node, Procedure target);
- invalidStringInterpolationOperand(
- List<TreeNode> context, TreeNode node, Constant constant);
- invalidSymbolName(List<TreeNode> context, TreeNode node, Constant constant);
- zeroDivisor(
- List<TreeNode> context, TreeNode node, IntConstant receiver, String op);
- negativeShift(List<TreeNode> context, TreeNode node, IntConstant receiver,
- String op, IntConstant argument);
- nonConstLiteral(List<TreeNode> context, TreeNode node, String klass);
- duplicateKey(List<TreeNode> context, TreeNode node, Constant key);
- failedAssertion(List<TreeNode> context, TreeNode node, String message);
- nonConstantVariableGet(
- List<TreeNode> context, TreeNode node, String variableName);
- deferredLibrary(List<TreeNode> context, TreeNode node, String importName);
-}
-
-abstract class ErrorReporterBase implements ErrorReporter {
- const ErrorReporterBase();
-
- report(List<TreeNode> context, String message, TreeNode node);
-
- getFileUri(TreeNode node) {
+ Uri getFileUri(TreeNode node) {
while (node is! FileUriNode) {
node = node.parent;
}
return (node as FileUriNode).fileUri;
}
- getFileOffset(TreeNode node) {
+ int getFileOffset(TreeNode node) {
while (node.fileOffset == TreeNode.noOffset) {
node = node.parent;
}
return node == null ? TreeNode.noOffset : node.fileOffset;
}
- freeTypeParameter(List<TreeNode> context, TreeNode node, DartType type) {
+ void freeTypeParameter(List<TreeNode> context, TreeNode node, DartType type);
+ void invalidDartType(List<TreeNode> context, TreeNode node, Constant receiver,
+ DartType expectedType);
+ void invalidBinaryOperandType(List<TreeNode> context, TreeNode node,
+ Constant receiver, String op, DartType expectedType, DartType actualType);
+ void invalidMethodInvocation(
+ List<TreeNode> context, TreeNode node, Constant receiver, String op);
+ void invalidStaticInvocation(
+ List<TreeNode> context, TreeNode node, Procedure target);
+ void invalidStringInterpolationOperand(
+ List<TreeNode> context, TreeNode node, Constant constant);
+ void invalidSymbolName(
+ List<TreeNode> context, TreeNode node, Constant constant);
+ void zeroDivisor(
+ List<TreeNode> context, TreeNode node, IntConstant receiver, String op);
+ void negativeShift(List<TreeNode> context, TreeNode node,
+ IntConstant receiver, String op, IntConstant argument);
+ void nonConstLiteral(List<TreeNode> context, TreeNode node, String klass);
+ void duplicateKey(List<TreeNode> context, TreeNode node, Constant key);
+ void failedAssertion(List<TreeNode> context, TreeNode node, String message);
+ void nonConstantVariableGet(
+ List<TreeNode> context, TreeNode node, String variableName);
+ void deferredLibrary(
+ List<TreeNode> context, TreeNode node, String importName);
+}
+
+class SimpleErrorReporter extends ErrorReporter {
+ const SimpleErrorReporter();
+
+ void report(List<TreeNode> context, String message, TreeNode node) {
+ io.exitCode = 42;
+ final Uri uri = getFileUri(node);
+ final int fileOffset = getFileOffset(node);
+
+ io.stderr.writeln('$uri:$fileOffset Constant evaluation error: $message');
+ }
+
+ @override
+ void freeTypeParameter(List<TreeNode> context, TreeNode node, DartType type) {
report(context, 'Expected type to be instantiated but was ${type}', node);
}
- invalidDartType(List<TreeNode> context, TreeNode node, Constant receiver,
+ @override
+ void invalidDartType(List<TreeNode> context, TreeNode node, Constant receiver,
DartType expectedType) {
report(
context,
@@ -1532,7 +1544,8 @@
node);
}
- invalidBinaryOperandType(
+ @override
+ void invalidBinaryOperandType(
List<TreeNode> context,
TreeNode node,
Constant receiver,
@@ -1546,19 +1559,22 @@
node);
}
- invalidMethodInvocation(
+ @override
+ void invalidMethodInvocation(
List<TreeNode> context, TreeNode node, Constant receiver, String op) {
report(context, 'Cannot call "$op" on "$receiver" in constant expression',
node);
}
- invalidStaticInvocation(
+ @override
+ void invalidStaticInvocation(
List<TreeNode> context, TreeNode node, Procedure target) {
report(
context, 'Cannot invoke "$target" inside a constant expression', node);
}
- invalidStringInterpolationOperand(
+ @override
+ void invalidStringInterpolationOperand(
List<TreeNode> context, TreeNode node, Constant constant) {
report(
context,
@@ -1567,7 +1583,9 @@
node);
}
- invalidSymbolName(List<TreeNode> context, TreeNode node, Constant constant) {
+ @override
+ void invalidSymbolName(
+ List<TreeNode> context, TreeNode node, Constant constant) {
report(
context,
'The symbol name must be a valid public Dart member name, public '
@@ -1575,7 +1593,8 @@
node);
}
- zeroDivisor(
+ @override
+ void zeroDivisor(
List<TreeNode> context, TreeNode node, IntConstant receiver, String op) {
report(
context,
@@ -1584,8 +1603,9 @@
node);
}
- negativeShift(List<TreeNode> context, TreeNode node, IntConstant receiver,
- String op, IntConstant argument) {
+ @override
+ void negativeShift(List<TreeNode> context, TreeNode node,
+ IntConstant receiver, String op, IntConstant argument) {
report(
context,
"Binary operator '$op' on '${receiver.value}' requires non-negative "
@@ -1593,28 +1613,32 @@
node);
}
- nonConstLiteral(List<TreeNode> context, TreeNode node, String klass) {
+ @override
+ void nonConstLiteral(List<TreeNode> context, TreeNode node, String klass) {
report(
context,
'Cannot have a non-constant $klass literal within a const context.',
node);
}
- duplicateKey(List<TreeNode> context, TreeNode node, Constant key) {
+ @override
+ void duplicateKey(List<TreeNode> context, TreeNode node, Constant key) {
report(
context,
'Duplicate keys are not allowed in constant maps (found duplicate key "$key")',
node);
}
- failedAssertion(List<TreeNode> context, TreeNode node, String message) {
+ @override
+ void failedAssertion(List<TreeNode> context, TreeNode node, String message) {
report(
context,
'The assertion condition evaluated to "false" with message "$message"',
node);
}
- nonConstantVariableGet(
+ @override
+ void nonConstantVariableGet(
List<TreeNode> context, TreeNode node, String variableName) {
report(
context,
@@ -1623,7 +1647,9 @@
node);
}
- deferredLibrary(List<TreeNode> context, TreeNode node, String importName) {
+ @override
+ void deferredLibrary(
+ List<TreeNode> context, TreeNode node, String importName) {
report(
context,
'Deferred "$importName" cannot be used inside a constant '
@@ -1632,18 +1658,6 @@
}
}
-class _SimpleErrorReporter extends ErrorReporterBase {
- const _SimpleErrorReporter();
-
- report(List<TreeNode> context, String message, TreeNode node) {
- io.exitCode = 42;
- final Uri uri = getFileUri(node);
- final int fileOffset = getFileOffset(node);
-
- io.stderr.writeln('$uri:$fileOffset Constant evaluation error: $message');
- }
-}
-
class IsInstantiatedVisitor extends DartTypeVisitor<bool> {
final _availableVariables = new Set<TypeParameter>();
diff --git a/pkg/vm/lib/bytecode/gen_bytecode.dart b/pkg/vm/lib/bytecode/gen_bytecode.dart
index 11f5bcf..04a58fb 100644
--- a/pkg/vm/lib/bytecode/gen_bytecode.dart
+++ b/pkg/vm/lib/bytecode/gen_bytecode.dart
@@ -830,8 +830,7 @@
locals = new LocalVariables(node);
// TODO(alexmarkov): improve caching in ConstantEvaluator and reuse it
constantEvaluator = new ConstantEvaluator(constantsBackend, typeEnvironment,
- coreTypes, /* enableAsserts = */ true,
- errorReporter: errorReporter)
+ coreTypes, /* enableAsserts = */ true, errorReporter)
..env = new EvaluationEnvironment();
labeledStatements = <LabeledStatement, Label>{};
switchCases = <SwitchCase, Label>{};
diff --git a/pkg/vm/lib/constants_error_reporter.dart b/pkg/vm/lib/constants_error_reporter.dart
index eba53b4..cadbc0e 100644
--- a/pkg/vm/lib/constants_error_reporter.dart
+++ b/pkg/vm/lib/constants_error_reporter.dart
@@ -12,11 +12,11 @@
import 'package:front_end/src/api_unstable/vm.dart' as codes;
import 'package:kernel/ast.dart'
- show Constant, DartType, FileUriNode, IntConstant, Procedure, TreeNode;
+ show Constant, DartType, IntConstant, Procedure, TreeNode;
import 'package:kernel/transformations/constants.dart' as constants;
import 'package:kernel/type_environment.dart' show TypeEnvironment;
-class ForwardConstantEvaluationErrors implements constants.ErrorReporter {
+class ForwardConstantEvaluationErrors extends constants.ErrorReporter {
// This will get the currently active [CompilerContext] from a zone variable.
// If there is no active context, this will throw.
final CompilerContext compilerContext = CompilerContext.current;
@@ -25,25 +25,29 @@
ForwardConstantEvaluationErrors(this.typeEnvironment);
- freeTypeParameter(List<TreeNode> context, TreeNode node, DartType type) {
+ @override
+ void freeTypeParameter(List<TreeNode> context, TreeNode node, DartType type) {
final message =
codes.templateConstEvalFreeTypeParameter.withArguments(type);
reportIt(context, message, node);
}
- duplicateKey(List<TreeNode> context, TreeNode node, Constant key) {
+ @override
+ void duplicateKey(List<TreeNode> context, TreeNode node, Constant key) {
final message = codes.templateConstEvalDuplicateKey.withArguments(key);
reportIt(context, message, node);
}
- invalidDartType(List<TreeNode> context, TreeNode node, Constant receiver,
+ @override
+ void invalidDartType(List<TreeNode> context, TreeNode node, Constant receiver,
DartType expectedType) {
final message = codes.templateConstEvalInvalidType.withArguments(
receiver, expectedType, receiver.getType(typeEnvironment));
reportIt(context, message, node);
}
- invalidBinaryOperandType(
+ @override
+ void invalidBinaryOperandType(
List<TreeNode> context,
TreeNode node,
Constant receiver,
@@ -55,54 +59,63 @@
reportIt(context, message, node);
}
- invalidMethodInvocation(
+ @override
+ void invalidMethodInvocation(
List<TreeNode> context, TreeNode node, Constant receiver, String op) {
final message = codes.templateConstEvalInvalidMethodInvocation
.withArguments(op, receiver);
reportIt(context, message, node);
}
- invalidStaticInvocation(
+ @override
+ void invalidStaticInvocation(
List<TreeNode> context, TreeNode node, Procedure target) {
final message = codes.templateConstEvalInvalidStaticInvocation
.withArguments(target.name.toString());
reportIt(context, message, node);
}
- invalidStringInterpolationOperand(
+ @override
+ void invalidStringInterpolationOperand(
List<TreeNode> context, TreeNode node, Constant constant) {
final message = codes.templateConstEvalInvalidStringInterpolationOperand
.withArguments(constant);
reportIt(context, message, node);
}
- invalidSymbolName(List<TreeNode> context, TreeNode node, Constant constant) {
+ @override
+ void invalidSymbolName(
+ List<TreeNode> context, TreeNode node, Constant constant) {
final message =
codes.templateConstEvalInvalidSymbolName.withArguments(constant);
reportIt(context, message, node);
}
- zeroDivisor(
+ @override
+ void zeroDivisor(
List<TreeNode> context, TreeNode node, IntConstant receiver, String op) {
final message = codes.templateConstEvalZeroDivisor
.withArguments(op, '${receiver.value}');
reportIt(context, message, node);
}
- negativeShift(List<TreeNode> context, TreeNode node, IntConstant receiver,
- String op, IntConstant argument) {
+ @override
+ void negativeShift(List<TreeNode> context, TreeNode node,
+ IntConstant receiver, String op, IntConstant argument) {
final message = codes.templateConstEvalNegativeShift
.withArguments(op, '${receiver.value}', '${argument.value}');
reportIt(context, message, node);
}
- nonConstLiteral(List<TreeNode> context, TreeNode node, String klass) {
+ @override
+ void nonConstLiteral(List<TreeNode> context, TreeNode node, String klass) {
final message =
codes.templateConstEvalNonConstantLiteral.withArguments(klass);
reportIt(context, message, node);
}
- failedAssertion(List<TreeNode> context, TreeNode node, String string) {
+ @override
+ void failedAssertion(List<TreeNode> context, TreeNode node, String string) {
final message = string == null
? codes.messageConstEvalFailedAssertion
: codes.templateConstEvalFailedAssertionWithMessage
@@ -110,20 +123,23 @@
reportIt(context, message, node);
}
- nonConstantVariableGet(
+ @override
+ void nonConstantVariableGet(
List<TreeNode> context, TreeNode node, String variableName) {
final message = codes.templateConstEvalNonConstantVariableGet
.withArguments(variableName);
reportIt(context, message, node);
}
- deferredLibrary(List<TreeNode> context, TreeNode node, String importName) {
+ @override
+ void deferredLibrary(
+ List<TreeNode> context, TreeNode node, String importName) {
final message =
codes.templateConstEvalDeferredLibrary.withArguments(importName);
reportIt(context, message, node);
}
- reportIt(List<TreeNode> context, codes.Message message, TreeNode node) {
+ void reportIt(List<TreeNode> context, codes.Message message, TreeNode node) {
final Uri uri = getFileUri(node);
final int fileOffset = getFileOffset(node);
@@ -141,18 +157,4 @@
compilerContext.options
.report(locatedMessage, Severity.error, context: contextMessages);
}
-
- getFileUri(TreeNode node) {
- while (node is! FileUriNode) {
- node = node.parent;
- }
- return (node as FileUriNode).fileUri;
- }
-
- getFileOffset(TreeNode node) {
- while (node?.fileOffset == TreeNode.noOffset) {
- node = node.parent;
- }
- return node == null ? TreeNode.noOffset : node.fileOffset;
- }
}
diff --git a/pkg/vm/lib/kernel_front_end.dart b/pkg/vm/lib/kernel_front_end.dart
index d84850d..2105181 100644
--- a/pkg/vm/lib/kernel_front_end.dart
+++ b/pkg/vm/lib/kernel_front_end.dart
@@ -385,10 +385,10 @@
// TFA will remove constants fields which are unused (and respects the
// vm/embedder entrypoints).
constants.transformComponent(component, vmConstants,
+ new ForwardConstantEvaluationErrors(typeEnvironment),
keepFields: true,
evaluateAnnotations: true,
- enableAsserts: enableAsserts,
- errorReporter: new ForwardConstantEvaluationErrors(typeEnvironment));
+ enableAsserts: enableAsserts);
});
}
