[vm/kernel/bytecode] Report compile-time errors from constant evaluation while generating bytecode

Change-Id: I931622f5e442a439f5bfa241aade686b1bc91afd
Reviewed-on: https://dart-review.googlesource.com/68222
Commit-Queue: Alexander Markov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: alexmarkov

pkg/kernel/lib/transformations/constants.dart

@@ -343,19 +343,11 @@ class ConstantsTransformer extends Transformer {
 
   tryEvaluateWithContext(TreeNode treeContext, Expression node) {
     if (treeContext == node) {
-      try {
-        return constantEvaluator.evaluate(node);
-      } on _AbortCurrentEvaluation catch (_) {
-        return null;
-      }
+      return constantEvaluator.evaluate(node);
     }
 
     return constantEvaluator.runInsideContext(treeContext, () {
-      try {
-        return constantEvaluator.evaluate(node);
-      } on _AbortCurrentEvaluation catch (_) {
-        return null;
-      }
+      return constantEvaluator.evaluate(node);
     });
   }
 }
@@ -387,7 +379,18 @@ class ConstantEvaluator extends RecursiveVisitor {
         nodeCache = <Node, Constant>{};
 
   /// Evaluates [node] and possibly cache the evaluation result.
+  /// Returns `null` if expression can't be evaluated.
   Constant evaluate(Expression node) {
+    try {
+      return _evaluateSubexpression(node);
+    } on _AbortCurrentEvaluation catch (_) {
+      return null;
+    }
+  }
+
+  /// Evaluates [node] and possibly cache the evaluation result.
+  /// @throws _AbortCurrentEvaluation if expression can't be evaluated.
+  Constant _evaluateSubexpression(Expression node) {
     if (node == null) return nullConstant;
     if (env.isEmpty) {
       // We only try to evaluate the same [node] *once* within an empty
@@ -572,27 +575,30 @@ class ConstantEvaluator extends RecursiveVisitor {
               function.positionalParameters[i];
           final Constant value = (i < positionalArguments.length)
               ? positionalArguments[i]
-              : evaluate(parameter.initializer);
+              : _evaluateSubexpression(parameter.initializer);
           env.addVariableValue(parameter, value);
         }
         for (final VariableDeclaration parameter in function.namedParameters) {
-          final Constant value =
-              namedArguments[parameter.name] ?? evaluate(parameter.initializer);
+          final Constant value = namedArguments[parameter.name] ??
+              _evaluateSubexpression(parameter.initializer);
           env.addVariableValue(parameter, value);
         }
 
         // Step 2) Run all initializers (including super calls) with environment setup.
         for (final Field field in klass.fields) {
           if (!field.isStatic) {
-            instanceBuilder.setFieldValue(field, evaluate(field.initializer));
+            instanceBuilder.setFieldValue(
+                field, _evaluateSubexpression(field.initializer));
           }
         }
         for (final Initializer init in constructor.initializers) {
           if (init is FieldInitializer) {
-            instanceBuilder.setFieldValue(init.field, evaluate(init.value));
+            instanceBuilder.setFieldValue(
+                init.field, _evaluateSubexpression(init.value));
           } else if (init is LocalInitializer) {
             final VariableDeclaration variable = init.variable;
-            env.addVariableValue(variable, evaluate(variable.initializer));
+            env.addVariableValue(
+                variable, _evaluateSubexpression(variable.initializer));
           } else if (init is SuperInitializer) {
             handleConstructorInvocation(
                 init.target,
@@ -653,7 +659,7 @@ class ConstantEvaluator extends RecursiveVisitor {
     // We have no support for generic method invocation atm.
     assert(node.arguments.named.isEmpty);
 
-    final Constant receiver = evaluate(node.receiver);
+    final Constant receiver = _evaluateSubexpression(node.receiver);
     final List<Constant> arguments =
         evaluatePositionalArguments(node.arguments);
 
@@ -810,13 +816,13 @@ class ConstantEvaluator extends RecursiveVisitor {
   }
 
   visitLogicalExpression(LogicalExpression node) {
-    final Constant left = evaluate(node.left);
+    final Constant left = _evaluateSubexpression(node.left);
     switch (node.operator) {
       case '||':
         if (left is BoolConstant) {
           if (left.value) return trueConstant;
 
-          final Constant right = evaluate(node.right);
+          final Constant right = _evaluateSubexpression(node.right);
           if (right is BoolConstant) {
             return right;
           }
@@ -836,7 +842,7 @@ class ConstantEvaluator extends RecursiveVisitor {
         if (left is BoolConstant) {
           if (!left.value) return falseConstant;
 
-          final Constant right = evaluate(node.right);
+          final Constant right = _evaluateSubexpression(node.right);
           if (right is BoolConstant) {
             return right;
           }
@@ -853,7 +859,9 @@ class ConstantEvaluator extends RecursiveVisitor {
             contextChain, node, left, '${node.operator}');
         throw const _AbortCurrentEvaluation();
       case '??':
-        return (left is! NullConstant) ? left : evaluate(node.right);
+        return (left is! NullConstant)
+            ? left
+            : _evaluateSubexpression(node.right);
       default:
         errorReporter.invalidMethodInvocation(
             contextChain, node, left, '${node.operator}');
@@ -862,11 +870,11 @@ class ConstantEvaluator extends RecursiveVisitor {
   }
 
   visitConditionalExpression(ConditionalExpression node) {
-    final Constant constant = evaluate(node.condition);
+    final Constant constant = _evaluateSubexpression(node.condition);
     if (constant == trueConstant) {
-      return evaluate(node.then);
+      return _evaluateSubexpression(node.then);
     } else if (constant == falseConstant) {
-      return evaluate(node.otherwise);
+      return _evaluateSubexpression(node.otherwise);
     } else {
       errorReporter.invalidDartType(
           contextChain, node, constant, typeEnvironment.boolType);
@@ -885,7 +893,7 @@ class ConstantEvaluator extends RecursiveVisitor {
       throw 'Could not evaluate field get ${node.name} on incomplete instance';
     }
 
-    final Constant receiver = evaluate(node.receiver);
+    final Constant receiver = _evaluateSubexpression(node.receiver);
     if (receiver is StringConstant && node.name.name == 'length') {
       return canonicalize(new IntConstant(receiver.value.length));
     } else if (receiver is InstanceConstant) {
@@ -899,7 +907,8 @@ class ConstantEvaluator extends RecursiveVisitor {
   }
 
   visitLet(Let node) {
-    env.addVariableValue(node.variable, evaluate(node.variable.initializer));
+    env.addVariableValue(
+        node.variable, _evaluateSubexpression(node.variable.initializer));
     return node.body.accept(this);
   }
 
@@ -920,7 +929,7 @@ class ConstantEvaluator extends RecursiveVisitor {
       return constant;
     }
     if (variable.isConst) {
-      return evaluate(variable.initializer);
+      return _evaluateSubexpression(variable.initializer);
     }
     throw new Exception('The front-end should ensure we do not encounter a '
         'variable get of a non-const variable.');
@@ -931,7 +940,7 @@ class ConstantEvaluator extends RecursiveVisitor {
       final Member target = node.target;
       if (target is Field && target.isConst) {
         return runInsideContext(target, () {
-          return evaluate(target.initializer);
+          return _evaluateSubexpression(target.initializer);
         });
       } else if (target is Procedure) {
         if (target.kind == ProcedureKind.Method) {
@@ -1021,7 +1030,7 @@ class ConstantEvaluator extends RecursiveVisitor {
   }
 
   visitInstantiation(Instantiation node) {
-    final Constant constant = evaluate(node.expression);
+    final Constant constant = _evaluateSubexpression(node.expression);
     if (constant is TearOffConstant) {
       if (node.typeArguments.length ==
           constant.procedure.function.typeParameters.length) {

pkg/kernel/lib/vm/constants_native_effects.dart

@@ -59,7 +59,7 @@ class VmConstantsBackend implements ConstantsBackend {
         } else {
           value = new bool.fromEnvironment(name, defaultValue: defaultValue);
         }
-        return new BoolConstant(value);
+        return value != null ? new BoolConstant(value) : new NullConstant();
       case 'Integer_fromEnvironment':
         final String name = (positionalArguments[0] as StringConstant).value;
         final Constant constant = namedArguments['defaultValue'];

pkg/vm/lib/bytecode/assembler.dart

@@ -6,8 +6,8 @@ library vm.bytecode.assembler;
 
 import 'dart:typed_data';
 
-import 'package:vm/bytecode/dbc.dart';
-import 'package:vm/bytecode/exceptions.dart' show ExceptionsTable;
+import 'dbc.dart';
+import 'exceptions.dart' show ExceptionsTable;
 
 class Label {
   List<int> _jumps = <int>[];

pkg/vm/lib/bytecode/disassembler.dart

@@ -6,8 +6,8 @@ library vm.bytecode.disassembler;
 
 import 'dart:typed_data';
 
-import 'package:vm/bytecode/dbc.dart';
-import 'package:vm/bytecode/exceptions.dart';
+import 'dbc.dart';
+import 'exceptions.dart';
 
 class _Instruction {
   final Opcode opcode;

pkg/vm/lib/bytecode/gen_bytecode.dart

@@ -10,18 +10,23 @@ import 'package:kernel/core_types.dart' show CoreTypes;
 import 'package:kernel/external_name.dart' show getExternalName;
 import 'package:kernel/library_index.dart' show LibraryIndex;
 import 'package:kernel/transformations/constants.dart'
-    show ConstantEvaluator, ConstantsBackend, EvaluationEnvironment;
+    show
+        ConstantEvaluator,
+        ConstantsBackend,
+        EvaluationEnvironment,
+        ErrorReporter;
 import 'package:kernel/type_algebra.dart'
     show Substitution, containsTypeVariable;
 import 'package:kernel/type_environment.dart' show TypeEnvironment;
 import 'package:kernel/vm/constants_native_effects.dart'
     show VmConstantsBackend;
-import 'package:vm/bytecode/assembler.dart';
-import 'package:vm/bytecode/constant_pool.dart';
-import 'package:vm/bytecode/dbc.dart';
-import 'package:vm/bytecode/exceptions.dart';
-import 'package:vm/bytecode/local_vars.dart' show LocalVariables;
-import 'package:vm/metadata/bytecode.dart';
+import 'assembler.dart';
+import 'constant_pool.dart';
+import 'dbc.dart';
+import 'exceptions.dart';
+import 'local_vars.dart' show LocalVariables;
+import '../constants_error_reporter.dart' show ForwardConstantEvaluationErrors;
+import '../metadata/bytecode.dart';
 
 /// Flag to toggle generation of bytecode in kernel files.
 const bool isKernelBytecodeEnabled = false;
@@ -32,16 +37,20 @@ const bool isKernelBytecodeEnabledForPlatform = isKernelBytecodeEnabled;
 void generateBytecode(Component component,
     {bool strongMode: true,
     bool dropAST: false,
-    bool omitSourcePositions: false}) {
+    bool omitSourcePositions: false,
+    Map<String, String> environmentDefines,
+    ErrorReporter errorReporter}) {
   final coreTypes = new CoreTypes(component);
   void ignoreAmbiguousSupertypes(Class cls, Supertype a, Supertype b) {}
   final hierarchy = new ClassHierarchy(component,
       onAmbiguousSupertypes: ignoreAmbiguousSupertypes);
   final typeEnvironment =
       new TypeEnvironment(coreTypes, hierarchy, strongMode: strongMode);
-  final constantsBackend = new VmConstantsBackend(null, coreTypes);
+  final constantsBackend =
+      new VmConstantsBackend(environmentDefines, coreTypes);
+  final errorReporter = new ForwardConstantEvaluationErrors(typeEnvironment);
   new BytecodeGenerator(component, coreTypes, hierarchy, typeEnvironment,
-          constantsBackend, strongMode, omitSourcePositions)
+          constantsBackend, strongMode, omitSourcePositions, errorReporter)
       .visitComponent(component);
   if (dropAST) {
     new DropAST().visitComponent(component);
@@ -56,6 +65,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
   final ConstantsBackend constantsBackend;
   final bool strongMode;
   final bool omitSourcePositions;
+  final ErrorReporter errorReporter;
   final BytecodeMetadataRepository metadata = new BytecodeMetadataRepository();
 
   Class enclosingClass;
@@ -78,6 +88,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
   ConstantEmitter constantEmitter;
   BytecodeAssembler asm;
   List<BytecodeAssembler> savedAssemblers;
+  bool hasErrors;
 
   BytecodeGenerator(
       this.component,
@@ -86,7 +97,8 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
       this.typeEnvironment,
       this.constantsBackend,
       this.strongMode,
-      this.omitSourcePositions) {
+      this.omitSourcePositions,
+      this.errorReporter) {
     component.addMetadataRepository(metadata);
   }
 
@@ -288,8 +300,19 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     asm.emitPushConstant(cpIndex);
   }
 
-  void _genPushConstExpr(Expression expr) {
+  Constant _evaluateConstantExpression(Expression expr) {
     final constant = constantEvaluator.evaluate(expr);
+    if (constant == null) {
+      // Compile-time error is already reported. Proceed with compilation
+      // in order to report as many errors as possible.
+      hasErrors = true;
+      return new NullConstant();
+    }
+    return constant;
+  }
+
+  void _genPushConstExpr(Expression expr) {
+    final constant = _evaluateConstantExpression(expr);
     asm.emitPushConstant(constant.accept(constantEmitter));
   }
 
@@ -526,7 +549,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     if (param.initializer == null) {
       return cp.add(const ConstantNull());
     }
-    final constant = constantEvaluator.evaluate(param.initializer);
+    final constant = _evaluateConstantExpression(param.initializer);
     return constant.accept(constantEmitter);
   }
 
@@ -567,6 +590,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     enclosingMember = node;
     enclosingFunction = node.function;
     parentFunction = null;
+    hasErrors = false;
     if (node.isInstanceMember ||
         node is Constructor ||
         (node is Procedure && node.isFactory)) {
@@ -595,7 +619,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     locals = new LocalVariables(node);
     // TODO(alexmarkov): improve caching in ConstantEvaluator and reuse it
     constantEvaluator = new ConstantEvaluator(constantsBackend, typeEnvironment,
-        coreTypes, strongMode, /* enableAsserts = */ true)
+        coreTypes, strongMode, /* enableAsserts = */ true, errorReporter)
       ..env = new EvaluationEnvironment();
     labeledStatements = <LabeledStatement, Label>{};
     switchCases = <SwitchCase, Label>{};
@@ -642,8 +666,10 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
   }
 
   void end(Member node) {
-    metadata.mapping[node] =
-        new BytecodeMetadata(cp, asm.bytecode, asm.exceptionsTable, closures);
+    if (!hasErrors) {
+      metadata.mapping[node] =
+          new BytecodeMetadata(cp, asm.bytecode, asm.exceptionsTable, closures);
+    }
 
     enclosingClass = null;
     enclosingMember = null;
@@ -665,6 +691,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     constantEmitter = null;
     asm = null;
     savedAssemblers = null;
+    hasErrors = false;
   }
 
   void _genPrologue(Node node, FunctionNode function) {
@@ -2269,7 +2296,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
   @override
   visitVariableDeclaration(VariableDeclaration node) {
     if (node.isConst) {
-      final Constant constant = constantEvaluator.evaluate(node.initializer);
+      final Constant constant = _evaluateConstantExpression(node.initializer);
       constantEvaluator.env.addVariableValue(node, constant);
     } else {
       final bool isCaptured = locals.isCaptured(node);

pkg/vm/lib/bytecode/local_vars.dart

@@ -9,7 +9,7 @@ import 'dart:math' show max;
 import 'package:kernel/ast.dart';
 import 'package:kernel/transformations/continuation.dart'
     show ContinuationVariables;
-import 'package:vm/bytecode/dbc.dart';
+import 'dbc.dart';
 
 class LocalVariables {
   final Map<TreeNode, Scope> _scopes = <TreeNode, Scope>{};