[CFE] Move DDC specific behavior into ConstantsBackend.

This avoids subclassing the constant evaluator in DDC.

Change-Id: If529761dc3fac474fade925bf0daaff51ad6e71b
Reviewed-on: https://dart-review.googlesource.com/c/92046
Reviewed-by: Jenny Messerly <jmesserly@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/constants.dart b/pkg/dev_compiler/lib/src/kernel/constants.dart
index fc4dbd5..7b2bee2 100644
--- a/pkg/dev_compiler/lib/src/kernel/constants.dart
+++ b/pkg/dev_compiler/lib/src/kernel/constants.dart
@@ -14,12 +14,13 @@
 /// [evaluate] computes the value of a constant expression, if available.
 class DevCompilerConstants {
   final _ConstantVisitor _visitor;
-  final _ConstantEvaluator _evaluator;
+  final ConstantEvaluator _evaluator;
 
   DevCompilerConstants(
       TypeEnvironment types, Map<String, String> declaredVariables)
       : _visitor = _ConstantVisitor(types.coreTypes),
-        _evaluator = _ConstantEvaluator(types, declaredVariables);
+        _evaluator = ConstantEvaluator(_ConstantsBackend(), declaredVariables,
+            types, types.coreTypes, false, const _ErrorReporter());
 
   /// Determines if an expression is constant.
   bool isConstant(Expression e) => _visitor.isConstant(e);
@@ -36,7 +37,7 @@
 
     try {
       var result = cache ? _evaluator.evaluate(e) : e.accept(_evaluator);
-      return identical(result, _evaluator.unavailableConstant) ? null : result;
+      return result is UnevaluatedConstant ? null : result;
     } on _AbortCurrentEvaluation {
       // TODO(jmesserly): the try+catch is necessary because the front end is
       // not issuing sufficient errors, so the constant evaluation can fail.
@@ -167,104 +168,26 @@
   }
 }
 
-/// The visitor that evaluates constants, building on Kernel's
-/// [ConstantEvaluator] class (used by the VM) and fixing some of its behavior
-/// to work better for DDC.
-//
-// TODO(jmesserly): make some changes in the base class to make it a better fit
-// for compilers like DDC?
-class _ConstantEvaluator extends ConstantEvaluator {
-  /// Used to denote an unavailable constant value from another module
-  ///
-  // TODO(jmesserly): this happens when we try to evaluate constant values from
-  // an external library, that was from an outline kernel file. The kernel file
-  // does not contain the initializer value of the constant.
-  final Constant unavailableConstant;
-
-  _ConstantEvaluator(
-      TypeEnvironment types, Map<String, String> declaredVariables,
-      {bool enableAsserts: false})
-      : unavailableConstant = InstanceConstant(null, [], {}),
-        super(_ConstantsBackend(), declaredVariables, types, types.coreTypes,
-            enableAsserts, const _ErrorReporter()) {
-    env = EvaluationEnvironment();
-  }
+/// Implement the class for compiler specific behavior.
+class _ConstantsBackend extends ConstantsBackend {
+  _ConstantsBackend();
 
   @override
-  visitVariableGet(node) {
-    // The base evaluator expects that variable declarations are visited during
-    // the transformation step, so it doesn't handle constant variables.
-    // Instead handle them here.
-    if (node.variable.isConst) {
-      return evaluate(node.variable.initializer);
-    }
-    // Fall back to the base evaluator for other cases (e.g. parameters of a
-    // constant constructor).
-    return super.visitVariableGet(node);
-  }
-
-  @override
-  visitStaticGet(StaticGet node) {
-    // Handle unavailable field constants. This happens if an external library
-    // only has its outline available.
-    var target = node.target;
-    if (target is Field &&
-        target.isConst &&
-        target.isInExternalLibrary &&
-        target.initializer == null) {
-      return unavailableConstant;
-    }
-    return super.visitStaticGet(node);
-  }
-
-  @override
-  visitConstructorInvocation(ConstructorInvocation node) {
-    // Handle unavailable constructor bodies.
-    // This happens if an external library only has its outline available.
-    var target = node.target;
-    if (target.isConst &&
-        target.isInExternalLibrary &&
-        target.function.body is EmptyStatement &&
-        target.initializers.isEmpty) {
-      return unavailableConstant;
-    }
-    return super.visitConstructorInvocation(node);
-  }
-
-  @override
-  evaluateBinaryNumericOperation(String op, num a, num b, TreeNode node) {
-    // Use doubles to match JS number semantics.
-    return super
-        .evaluateBinaryNumericOperation(op, a.toDouble(), b.toDouble(), node);
-  }
-
-  @override
-  canonicalize(Constant constant) {
+  Constant lowerConstant(Constant constant) {
     if (constant is DoubleConstant) {
       // Convert to an integer when possible (matching the runtime behavior
       // of `is int`).
       var d = constant.value;
       if (d.isFinite) {
         var i = d.toInt();
-        if (d == i.toDouble()) return super.canonicalize(IntConstant(i));
+        if (d == i.toDouble()) return IntConstant(i);
       }
     }
-    return super.canonicalize(constant);
+    return constant;
   }
-}
 
-/// Implement the class for compiler specific behavior.
-///
-/// This is mostly unused by DDC, because we don't use the global constant
-/// transformer.
-class _ConstantsBackend implements ConstantsBackend {
-  _ConstantsBackend();
-
-  @override
-  lowerMapConstant(constant) => constant;
-
-  @override
-  lowerListConstant(constant) => constant;
+  // Use doubles to match JS number semantics.
+  num prepareNumericOperand(num operand) => operand.toDouble();
 }
 
 class _ErrorReporter extends SimpleErrorReporter {
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_constants.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_constants.dart
index c777765..edbc584 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_constants.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_constants.dart
@@ -5,16 +5,7 @@
 library fasta.kernel_constants;
 
 import 'package:kernel/ast.dart'
-    show
-        Constant,
-        DartType,
-        IntConstant,
-        Library,
-        ListConstant,
-        MapConstant,
-        Member,
-        Procedure,
-        TreeNode;
+    show Constant, DartType, IntConstant, Library, Member, Procedure, TreeNode;
 
 import 'package:kernel/type_environment.dart' show TypeEnvironment;
 
@@ -189,10 +180,4 @@
   }
 }
 
-class KernelConstantsBackend implements ConstantsBackend {
-  @override
-  Constant lowerListConstant(ListConstant constant) => constant;
-
-  @override
-  Constant lowerMapConstant(MapConstant constant) => constant;
-}
+class KernelConstantsBackend extends ConstantsBackend {}
diff --git a/pkg/kernel/lib/transformations/constants.dart b/pkg/kernel/lib/transformations/constants.dart
index dba7281..a659c4c 100644
--- a/pkg/kernel/lib/transformations/constants.dart
+++ b/pkg/kernel/lib/transformations/constants.dart
@@ -402,7 +402,8 @@
   ConstantEvaluator(this.backend, this.environmentDefines, this.typeEnvironment,
       this.coreTypes, this.enableAsserts, this.errorReporter)
       : canonicalizationCache = <Constant, Constant>{},
-        nodeCache = <Node, Constant>{};
+        nodeCache = <Node, Constant>{},
+        env = new EvaluationEnvironment();
 
   /// Evaluates [node] and possibly cache the evaluation result.
   Constant evaluate(Expression node) {
@@ -509,8 +510,7 @@
       entries[i] = node.expressions[i].accept(this);
     }
     final DartType typeArgument = evaluateDartType(node, node.typeArgument);
-    final ListConstant listConstant = new ListConstant(typeArgument, entries);
-    return canonicalize(backend.lowerListConstant(listConstant));
+    return canonicalize(new ListConstant(typeArgument, entries));
   }
 
   visitMapLiteral(MapLiteral node) {
@@ -535,9 +535,7 @@
     }
     final DartType keyType = evaluateDartType(node, node.keyType);
     final DartType valueType = evaluateDartType(node, node.valueType);
-    final MapConstant mapConstant =
-        new MapConstant(keyType, valueType, entries);
-    return canonicalize(backend.lowerMapConstant(mapConstant));
+    return canonicalize(new MapConstant(keyType, valueType, entries));
   }
 
   visitFunctionExpression(FunctionExpression node) {
@@ -556,6 +554,12 @@
         constructor.function.body is! EmptyStatement) {
       throw 'Constructor "$node" has non-trivial body "${constructor.function.body.runtimeType}".';
     }
+    if (constructor.isInExternalLibrary && constructor.initializers.isEmpty) {
+      // The constructor is unavailable due to separate compilation.
+      return new UnevaluatedConstant(new ConstructorInvocation(
+          constructor, unevaluatedArguments(node.arguments),
+          isConst: true));
+    }
     if (klass.isAbstract) {
       throw 'Constructor "$node" belongs to abstract class "${klass}".';
     }
@@ -1111,6 +1115,10 @@
       final Member target = node.target;
       if (target is Field) {
         if (target.isConst) {
+          if (target.isInExternalLibrary && target.initializer == null) {
+            // The variable is unavailable due to separate compilation.
+            return new UnevaluatedConstant(node);
+          }
           return runInsideContext(target, () {
             return _evaluateSubexpression(target.initializer);
           });
@@ -1201,17 +1209,9 @@
           // TODO(askesc): Give more meaningful error message if name is null.
         } else {
           // Leave environment constant unevaluated.
-          for (int i = 0; i < arguments.positional.length; ++i) {
-            Constant constant = arguments.positional[i].accept(this);
-            arguments.positional[i] = constant.asExpression()
-              ..parent = arguments;
-          }
-          for (int i = 0; i < arguments.named.length; ++i) {
-            Constant constant = arguments.named[i].value.accept(this);
-            arguments.named[i].value = constant.asExpression()
-              ..parent = arguments;
-          }
-          return new UnevaluatedConstant(node);
+          return new UnevaluatedConstant(new StaticInvocation(
+              target, unevaluatedArguments(arguments),
+              isConst: true));
         }
       }
     } else if (target.name.name == 'identical') {
@@ -1327,7 +1327,23 @@
     return named;
   }
 
-  canonicalize(Constant constant) {
+  Arguments unevaluatedArguments(Arguments arguments) {
+    final positional = new List<Expression>(arguments.positional.length);
+    final named = new List<NamedExpression>(arguments.named.length);
+    for (int i = 0; i < arguments.positional.length; ++i) {
+      Constant constant = arguments.positional[i].accept(this);
+      positional[i] = constant.asExpression();
+    }
+    for (int i = 0; i < arguments.named.length; ++i) {
+      NamedExpression arg = arguments.named[i];
+      Constant constant = arg.value.accept(this);
+      named[i] = new NamedExpression(arg.name, constant.asExpression());
+    }
+    return new Arguments(positional, named: named, types: arguments.types);
+  }
+
+  Constant canonicalize(Constant constant) {
+    constant = backend.lowerConstant(constant);
     return canonicalizationCache.putIfAbsent(constant, () => constant);
   }
 
@@ -1351,7 +1367,10 @@
     }
   }
 
-  evaluateBinaryNumericOperation(String op, num a, num b, TreeNode node) {
+  Constant evaluateBinaryNumericOperation(
+      String op, num a, num b, TreeNode node) {
+    a = backend.prepareNumericOperand(a);
+    b = backend.prepareNumericOperand(b);
     num result;
     switch (op) {
       case '+':
@@ -1473,9 +1492,16 @@
   }
 }
 
-abstract class ConstantsBackend {
-  Constant lowerListConstant(ListConstant constant);
-  Constant lowerMapConstant(MapConstant constant);
+// Backend specific constant evaluation behavior
+class ConstantsBackend {
+  /// Transformation of constants prior to canonicalization, e.g. to change the
+  /// representation of certain kinds of constants, or to implement specific
+  /// number semantics.
+  Constant lowerConstant(Constant constant) => constant;
+
+  /// Transformation of numeric operands prior to a binary operation,
+  /// e.g. to implement specific number semantics.
+  num prepareNumericOperand(num operand) => operand;
 }
 
 // Used as control-flow to abort the current evaluation.
diff --git a/pkg/kernel/lib/vm/constants_native_effects.dart b/pkg/kernel/lib/vm/constants_native_effects.dart
index 78bf18f..58c8eca 100644
--- a/pkg/kernel/lib/vm/constants_native_effects.dart
+++ b/pkg/kernel/lib/vm/constants_native_effects.dart
@@ -8,7 +8,7 @@
 import '../transformations/constants.dart';
 import '../core_types.dart';
 
-class VmConstantsBackend implements ConstantsBackend {
+class VmConstantsBackend extends ConstantsBackend {
   final Class immutableMapClass;
 
   VmConstantsBackend._(this.immutableMapClass);
@@ -26,32 +26,30 @@
   }
 
   @override
-  Constant lowerMapConstant(MapConstant constant) {
-    // The _ImmutableMap class is implemented via one field pointing to a list
-    // of key/value pairs -- see runtime/lib/immutable_map.dart!
-    final List<Constant> kvListPairs =
-        new List<Constant>(2 * constant.entries.length);
-    for (int i = 0; i < constant.entries.length; i++) {
-      final ConstantMapEntry entry = constant.entries[i];
-      kvListPairs[2 * i] = entry.key;
-      kvListPairs[2 * i + 1] = entry.value;
+  Constant lowerConstant(Constant constant) {
+    if (constant is MapConstant) {
+      // The _ImmutableMap class is implemented via one field pointing to a list
+      // of key/value pairs -- see runtime/lib/immutable_map.dart!
+      final List<Constant> kvListPairs =
+          new List<Constant>(2 * constant.entries.length);
+      for (int i = 0; i < constant.entries.length; i++) {
+        final ConstantMapEntry entry = constant.entries[i];
+        kvListPairs[2 * i] = entry.key;
+        kvListPairs[2 * i + 1] = entry.value;
+      }
+      // This is a bit fishy, since we merge the key and the value type by
+      // putting both into the same list.
+      final kvListConstant = new ListConstant(const DynamicType(), kvListPairs);
+      assert(immutableMapClass.fields.length == 1);
+      final Field kvPairListField = immutableMapClass.fields[0];
+      return new InstanceConstant(immutableMapClass.reference, <DartType>[
+        constant.keyType,
+        constant.valueType,
+      ], <Reference, Constant>{
+        kvPairListField.reference: kvListConstant,
+      });
     }
-    // This is a bit fishy, since we merge the key and the value type by
-    // putting both into the same list.
-    final kvListConstant = new ListConstant(const DynamicType(), kvListPairs);
-    assert(immutableMapClass.fields.length == 1);
-    final Field kvPairListField = immutableMapClass.fields[0];
-    return new InstanceConstant(immutableMapClass.reference, <DartType>[
-      constant.keyType,
-      constant.valueType,
-    ], <Reference, Constant>{
-      kvPairListField.reference: kvListConstant,
-    });
-  }
 
-  @override
-  Constant lowerListConstant(ListConstant constant) {
-    // Currently we let vipunen deal with the [ListConstant]s.
     return constant;
   }
 }