dart2js: simplify constant expression generation.

pkg/compiler/lib/src/js_backend/constant_emitter.dart

Index: pkg/compiler/lib/src/js_backend/constant_emitter.dart
diff --git a/pkg/compiler/lib/src/js_backend/constant_emitter.dart b/pkg/compiler/lib/src/js_backend/constant_emitter.dart
index 234e2a01615904554a52c282c033e7ad507e039e..a8ccf2758de25266724732fb1ee827b9c2e972f4 100644
--- a/pkg/compiler/lib/src/js_backend/constant_emitter.dart
+++ b/pkg/compiler/lib/src/js_backend/constant_emitter.dart
@@ -4,165 +4,14 @@
 
 part of js_backend;
 
-class ConstantEmitter {
-  ConstantReferenceEmitter _referenceEmitter;
-  ConstantLiteralEmitter _literalEmitter;
-
-  ConstantEmitter(Compiler compiler,
-                  Namer namer,
-                  jsAst.Template makeConstantListTemplate) {
-    _literalEmitter = new ConstantLiteralEmitter(
-        compiler, namer, makeConstantListTemplate, this);
-    _referenceEmitter = new ConstantReferenceEmitter(compiler, namer, this);
-  }
-
-  /**
-   * Constructs an expression that is a reference to the constant.  Uses a
-   * canonical name unless the constant can be emitted multiple times (as for
-   * numbers and strings).
-   */
-  jsAst.Expression reference(ConstantValue constant) {
-    return _referenceEmitter.generate(constant);
-  }
-
-  /**
-   * Constructs a literal expression that evaluates to the constant. Uses a
-   * canonical name unless the constant can be emitted multiple times (as for
-   * numbers and strings).
-   */
-  jsAst.Expression literal(ConstantValue constant) {
-    return _literalEmitter.generate(constant);
-  }
-
-  /**
-   * Constructs an expression like [reference], but the expression is valid
-   * during isolate initialization.
-   */
-  jsAst.Expression referenceInInitializationContext(ConstantValue constant) {
-    return _referenceEmitter.generate(constant);
-  }
-
-  /**
-   * Constructs an expression used to initialize a canonicalized constant.
-   */
-  jsAst.Expression initializationExpression(ConstantValue constant) {
-    return _literalEmitter.generate(constant);
-  }
-}
-
 /**
- * Visitor for generating JavaScript expressions to refer to [ConstantValue]s.
- * Do not use directly, use methods from [ConstantEmitter].
+ * Generates the JavaScript expressions for constants.
+ *
+ * It uses a given [constantReference] to reference nested constants (if there
+ * are some). It is hence up to that function to decide which constants should
+ * be inlined or not.
  */
-class ConstantReferenceEmitter
-    implements ConstantValueVisitor<jsAst.Expression, Null> {
-  final Compiler compiler;
-  final Namer namer;
-
-  final ConstantEmitter constantEmitter;
-
-  ConstantReferenceEmitter(this.compiler, this.namer, this.constantEmitter);
-
-  JavaScriptBackend get backend => compiler.backend;
-
-  jsAst.Expression generate(ConstantValue constant) {
-    return _visit(constant);
-  }
-
-  jsAst.Expression _visit(ConstantValue constant) {
-    return constant.accept(this, null);
-  }
-
-  jsAst.Expression emitCanonicalVersion(ConstantValue constant) {
-    String name = namer.constantName(constant);
-    return new jsAst.PropertyAccess.field(
-        new jsAst.VariableUse(namer.globalObjectForConstant(constant)), name);
-  }
-
-  jsAst.Expression literal(ConstantValue constant) {
-      return constantEmitter.literal(constant);
-  }
-
-  @override
-  jsAst.Expression visitFunction(FunctionConstantValue constant, [_]) {
-    return backend.emitter.isolateStaticClosureAccess(constant.element);
-  }
-
-  @override
-  jsAst.Expression visitNull(NullConstantValue constant, [_]) {
-    return literal(constant);
-  }
-
-  @override
-  jsAst.Expression visitInt(IntConstantValue constant, [_]) {
-    return literal(constant);
-  }
-
-  @override
-  jsAst.Expression visitDouble(DoubleConstantValue constant, [_]) {
-    return literal(constant);
-  }
-
-  @override
-  jsAst.Expression visitBool(BoolConstantValue constant, [_]) {
-    return literal(constant);
-  }
-
-  /**
-   * Write the contents of the quoted string to a [CodeBuffer] in
-   * a form that is valid as JavaScript string literal content.
-   * The string is assumed quoted by double quote characters.
-   */
-  @override
-  jsAst.Expression visitString(StringConstantValue constant, [_]) {
-    // TODO(sra): If the string is long *and repeated* (and not on a hot path)
-    // then it should be assigned to a name.  We don't have reference counts (or
-    // profile information) here, so this is the wrong place.
-    return literal(constant);
-  }
-
-  @override
-  jsAst.Expression visitList(ListConstantValue constant, [_]) {
-    return emitCanonicalVersion(constant);
-  }
-
-  @override
-  jsAst.Expression visitMap(MapConstantValue constant, [_]) {
-    return emitCanonicalVersion(constant);
-  }
-
-  @override
-  jsAst.Expression visitType(TypeConstantValue constant, [_]) {
-    return emitCanonicalVersion(constant);
-  }
-
-  @override
-  jsAst.Expression visitConstructed(ConstructedConstantValue constant, [_]) {
-    return emitCanonicalVersion(constant);
-  }
-
-  @override
-  jsAst.Expression visitInterceptor(InterceptorConstantValue constant, [_]) {
-    return emitCanonicalVersion(constant);
-  }
-
-  @override
-  jsAst.Expression visitDummy(DummyConstantValue constant, [_]) {
-    return literal(constant);
-  }
-
-  @override
-  jsAst.Expression visitDeferred(DeferredConstantValue constant, [_]) {
-    return emitCanonicalVersion(constant);
-  }
-}
-
-/**
- * Visitor for generating JavaScript expressions that litterally represent
- * [ConstantValue]s. These can be used for inlining constants or in
- * initializers. Do not use directly, use methods from [ConstantEmitter].
- */
-class ConstantLiteralEmitter
+class ConstantEmitter
     implements ConstantValueVisitor<jsAst.Expression, Null> {
 
   // Matches blank lines, comment lines and trailing comments that can't be part
@@ -172,14 +21,25 @@ class ConstantLiteralEmitter
 
   final Compiler compiler;
   final Namer namer;
+  final Function constantReference;

[floitsch, 2015/02/24 23:08:48]

I know the function isn't typed, but I really hate all these typedefs. In this
case the function is checked in the constructor.

If I'm overruled I can add a typedef, though...

[Johnni Winther, 2015/02/25 11:37:02]

Make a typedef like

typedef jsAst.Expression ConstantReferenceGenerator(ConstantValue constant);

and rename the field to

ConstantReferenceGenerator generateConstantReference;

The latter will make the call-sites read nicer.

[floitsch, 2015/02/25 16:35:18]

Done.

   final jsAst.Template makeConstantListTemplate;
-  final ConstantEmitter constantEmitter;
 
-  ConstantLiteralEmitter(this.compiler,
-                         this.namer,
-                         this.makeConstantListTemplate,
-                         this.constantEmitter);
+  /**
+   * The given [constantReferenc] function must, when invoked with a constant,

[Johnni Winther, 2015/02/25 11:37:02]

[constantReferenc] -> [constantReference]  (if not superceeded by the effect of
the previous comment)

[floitsch, 2015/02/25 16:35:18]

Done.

+   * either return a reference or return its literal expression if it can
+   * be inlined.
+   */
+  ConstantEmitter(
+      this.compiler,
+      this.namer,
+      jsAst.Expression this.constantReference(ConstantValue constant),
+      this.makeConstantListTemplate);
 
+  /**
+   * Constructs a literal expression that evaluates to the constant. Uses a
+   * canonical name unless the constant can be emitted multiple times (as for
+   * numbers and strings).
+   */
   jsAst.Expression generate(ConstantValue constant) {
     return _visit(constant);
   }
@@ -294,7 +154,8 @@ class ConstantLiteralEmitter
 
   @override
   jsAst.Expression visitList(ListConstantValue constant, [_]) {
-    List<jsAst.Expression> elements = _array(constant.entries);

[floitsch, 2015/02/24 23:08:48]

I initially thought that I could get away with an Iterable here and removed the
"_array" function (which had a bad name anyway).
It turns out that I couldn't and the function is now inlined in two places. I
think that's ok, but I can revive the function again (with name "referenceAll"
?).

+    List<jsAst.Expression> elements =
+        constant.entries.map(constantReference).toList(growable: false);
     jsAst.ArrayInitializer array = new jsAst.ArrayInitializer(elements);
     jsAst.Expression value = makeConstantListTemplate.instantiate([array]);
     return maybeAddTypeArguments(constant.type, value);
@@ -313,7 +174,7 @@ class ConstantLiteralEmitter
         // Keys in literal maps must be emitted in place.
         jsAst.Literal keyExpression = _visit(key);
         jsAst.Expression valueExpression =
-            constantEmitter.reference(constant.values[i]);
+            constantReference(constant.values[i]);
         properties.add(new jsAst.Property(keyExpression, valueExpression));
       }
       return new jsAst.ObjectInitializer(properties);
@@ -322,10 +183,9 @@ class ConstantLiteralEmitter
     jsAst.Expression jsGeneralMap() {
       List<jsAst.Expression> data = <jsAst.Expression>[];
       for (int i = 0; i < constant.keys.length; i++) {
-        jsAst.Expression keyExpression =
-            constantEmitter.reference(constant.keys[i]);
+        jsAst.Expression keyExpression = constantReference(constant.keys[i]);
         jsAst.Expression valueExpression =
-            constantEmitter.reference(constant.values[i]);
+            constantReference(constant.values[i]);
         data.add(keyExpression);
         data.add(valueExpression);
       }
@@ -348,10 +208,10 @@ class ConstantLiteralEmitter
           } else if (field.name == JavaScriptMapConstant.JS_OBJECT_NAME) {
             arguments.add(jsMap());
           } else if (field.name == JavaScriptMapConstant.KEYS_NAME) {
-            arguments.add(constantEmitter.reference(constant.keyList));
+            arguments.add(constantReference(constant.keyList));
           } else if (field.name == JavaScriptMapConstant.PROTO_VALUE) {
             assert(constant.protoValue != null);
-            arguments.add(constantEmitter.reference(constant.protoValue));
+            arguments.add(constantReference(constant.protoValue));
           } else if (field.name == JavaScriptMapConstant.JS_DATA_NAME) {
             arguments.add(jsGeneralMap());
           } else {
@@ -415,8 +275,9 @@ class ConstantLiteralEmitter
     }
     jsAst.Expression constructor =
         backend.emitter.constructorAccess(constant.type.element);
-    jsAst.New instantiation =
-        new jsAst.New(constructor, _array(constant.fields));
+    List<jsAst.Expression> fields =
+        constant.fields.map(constantReference).toList(growable: false);
+    jsAst.New instantiation = new jsAst.New(constructor, fields);
     return maybeAddTypeArguments(constant.type, instantiation);
   }
 
@@ -424,10 +285,6 @@ class ConstantLiteralEmitter
     return rawJavaScript.replaceAll(COMMENT_RE, '');
   }
 
-  List<jsAst.Expression> _array(List<ConstantValue> values) {
-    return values.map(constantEmitter.reference).toList(growable: false);
-  }
-
   jsAst.Expression maybeAddTypeArguments(InterfaceType type,
                                          jsAst.Expression value) {
     if (type is InterfaceType &&
@@ -448,6 +305,6 @@ class ConstantLiteralEmitter
 
   @override
   jsAst.Expression visitDeferred(DeferredConstantValue constant, [_]) {
-    return constantEmitter.reference(constant.referenced);
+    return constantReference(constant.referenced);
   }
 }