Avoid reparsing metadata expressions

BUG= http://dartbug.com/23404
R=floitsch@google.com

Review URL: https://codereview.chromium.org//1126423002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@45600 260f80e4-7a28-3924-810f-c04153c831b5

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

@@ -124,8 +124,7 @@ class TypeVariableHandler {
    * there, otherwise a new entry for [c] is created.
    */
   int _reifyTypeVariableConstant(ConstantValue c, TypeVariableElement variable) {
-    String name = jsAst.prettyPrint(_task.constantReference(c),
-                                    _compiler).getText();
+    jsAst.Expression name = _task.constantReference(c);
     int index;
     if (_typeVariableConstants.containsKey(variable)) {
       index = _typeVariableConstants[variable];
@@ -153,7 +152,7 @@ class TypeVariableHandler {
     }
 
     // TODO(15613): Remove quotes.
-    _metadataCollector.globalMetadata.add('"Placeholder for ${variable}"');
+    _metadataCollector.globalMetadata.add(js('"Placeholder for ${variable}"'));
     return _typeVariableConstants[variable] =
         _metadataCollector.globalMetadata.length - 1;
   }

pkg/compiler/lib/src/js_emitter/metadata_collector.dart

@@ -10,14 +10,15 @@ class MetadataCollector {
 
   /// A list of JS expressions that represent metadata, parameter names and
   /// type variable types.
-  final List<String> globalMetadata = <String>[];
+  final List<jsAst.Expression> globalMetadata = <jsAst.Expression>[];
 
   /// A map used to canonicalize the entries of globalMetadata.
   final Map<String, int> _globalMetadataMap = <String, int>{};
 
   /// A map with lists of JS expressions, one list for each output unit. The
   /// entries represent types including function types and typedefs.
-  final Map<OutputUnit, List<String>> types = <OutputUnit, List<String>>{};
+  final Map<OutputUnit, List<jsAst.Expression>> types =
+      <OutputUnit, List<jsAst.Expression>>{};
 
   /// A map used to canonicalize the entries of types.
   final Map<OutputUnit, Map<String, int>> _typesMap =
@@ -70,11 +71,10 @@ class MetadataCollector {
     for (ParameterElement element in signature.optionalParameters) {
       ConstantExpression constant =
           _backend.constants.getConstantForVariable(element);
-      String stringRepresentation = (constant == null)
-          ? "null"
-          : jsAst.prettyPrint(
-              _emitter.constantReference(constant.value), _compiler).getText();
-      defaultValues.add(addGlobalMetadata(stringRepresentation));
+      jsAst.Expression expression = (constant == null)
+          ? null
+          : _emitter.constantReference(constant.value);
+      defaultValues.add(addGlobalMetadata(expression));
     }
     return defaultValues;
   }
@@ -86,9 +86,7 @@ class MetadataCollector {
       _compiler.internalError(annotation, 'Annotation value is null.');
       return -1;
     }
-    return addGlobalMetadata(
-        jsAst.prettyPrint(
-            _emitter.constantReference(constant.value), _compiler).getText());
+    return addGlobalMetadata(_emitter.constantReference(constant.value));
   }
 
   int reifyType(DartType type, {bool ignoreTypeVariables: false}) {
@@ -112,31 +110,33 @@ class MetadataCollector {
               return _backend.isAccessibleByReflection(typedef.element);
             });
 
-    return addTypeInOutputUnit(
-        jsAst.prettyPrint(representation, _compiler).getText(), outputUnit);
+    return addTypeInOutputUnit(representation, outputUnit);
   }
 
   int reifyName(String name) {
-    return addGlobalMetadata('"$name"');
+    return addGlobalMetadata(js('"$name"'));
   }
 
-  int addGlobalMetadata(String string) {
+  int addGlobalMetadata(jsAst.Expression expression) {
+    // TODO(sigmund): consider adding an effient way to compare expressions
+    String string = jsAst.prettyPrint(expression, _compiler).getText();
     return _globalMetadataMap.putIfAbsent(string, () {
-      globalMetadata.add(string);
+      globalMetadata.add(expression);
       return globalMetadata.length - 1;
     });
   }
 
-  int addTypeInOutputUnit(String compiledType, OutputUnit outputUnit) {
+  int addTypeInOutputUnit(jsAst.Expression type, OutputUnit outputUnit) {
+    String string = jsAst.prettyPrint(type, _compiler).getText();
     if (_typesMap[outputUnit] == null) {
       _typesMap[outputUnit] = <String, int>{};
     }
-    return _typesMap[outputUnit].putIfAbsent(compiledType, () {
+    return _typesMap[outputUnit].putIfAbsent(string, () {
 
       if (types[outputUnit] == null)
-        types[outputUnit] = <String>[];
+        types[outputUnit] = <jsAst.Expression>[];
 
-      types[outputUnit].add(compiledType);
+      types[outputUnit].add(type);
       return types[outputUnit].length - 1;
     });
   }

pkg/compiler/lib/src/js_emitter/model.dart

@@ -43,7 +43,7 @@ class Program {
     assert(hasIsolateSupport != null);
   }
 
-  /// A list of pretty-printed JavaScript expressions.
+  /// A list of metadata expressions.
   ///
   /// This list must be emitted in the `METADATA` embedded global.
   /// The list references constants and must hence be emitted after constants
@@ -53,9 +53,9 @@ class Program {
   /// list must not be emitted before all operations on it are done. For
   /// example, the old emitter generates metadata when emitting reflection
   /// data.
-  List<String> get metadata => _metadataCollector.globalMetadata;
+  List<js.Expression> get metadata => _metadataCollector.globalMetadata;
 
-  /// A map with lists of pretty-printed JavaScript expressions.
+  /// A map with lists of type expressions.
   ///
   /// There is one list for each output unit. The list belonging to the main
   /// unit must be emitted in the `TYPES` embedded global. The list references
@@ -65,7 +65,7 @@ class Program {
   /// list must not be emitted before all operations on it are done. For
   /// example, the old emitter generates metadata when emitting reflection
   /// data.
-  Map<OutputUnit, List<String>> get metadataTypes
+  Map<OutputUnit, List<js.Expression>> get metadataTypes
       => _metadataCollector.types;
 
 

pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart

@@ -138,7 +138,8 @@ class ModelEmitter {
     // deferred hash (which depends on the output) when emitting the main
     // fragment.
     fragments.skip(1).forEach((DeferredFragment deferredUnit) {
-      List<String> types = program.metadataTypes[deferredUnit.outputUnit];
+      List<js.Expression> types =
+          program.metadataTypes[deferredUnit.outputUnit];
       js.Expression ast = emitDeferredFragment(types, deferredUnit,
                                                program.holders);
       String code = js.prettyPrint(ast, compiler).getText();
@@ -421,26 +422,19 @@ class ModelEmitter {
   }
 
   List<js.Property> emitMetadata(Program program) {
-
     List<js.Property> metadataGlobals = <js.Property>[];
-
-    js.Property createGlobal(List<String> list, String global) {
-      String listAsString = "[${list.join(",")}]";
-      js.Expression metadata =
-                js.js.uncachedExpressionTemplate(listAsString).instantiate([]);
-      return new js.Property(js.string(global), metadata);
-    }
-
-    metadataGlobals.add(createGlobal(program.metadata, METADATA));
-    List<String> types =
+    metadataGlobals.add(new js.Property(
+        js.string(METADATA), new js.ArrayInitializer(program.metadata)));
+    List<js.Expression> types =
         program.metadataTypes[program.fragments.first.outputUnit];
-    if (types == null) types = <String>[];
-    metadataGlobals.add(createGlobal(types, TYPES));
+    if (types == null) types = <js.Expression>[];
+    metadataGlobals.add(new js.Property(
+        js.string(TYPES), new js.ArrayInitializer(types)));
 
     return metadataGlobals;
   }
 
-  js.Expression emitDeferredFragment(List<String> types,
+  js.Expression emitDeferredFragment(List<js.Expression> types,
                                      DeferredFragment fragment,
                                      List<Holder> holders) {
     // TODO(floitsch): initialize eager classes.
@@ -466,7 +460,7 @@ class ModelEmitter {
 
     js.Expression deferredTypes = types == null
         ? js.string("[]")
-        : js.string("[${types.join(",")}]");
+        : unparse(compiler, new js.ArrayInitializer(types));
 
     js.ArrayInitializer hunk =
         new js.ArrayInitializer([deferredArray, immediateString,

pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart

@@ -675,12 +675,11 @@ class OldEmitter implements Emitter {
 
   void emitMetadata(Program program, CodeOutput output, OutputUnit outputUnit) {
 
-    jsAst.Expression constructList(List<String> list) {
-      String listAsString = list == null ? '[]' : '[${list.join(",")}]';
-      return js.uncachedExpressionTemplate(listAsString).instantiate([]);
+    jsAst.Expression constructList(List<jsAst.Expression> list) {
+      return new jsAst.ArrayInitializer(list == null ? [] : list);
     }
 
-    List<String> types = program.metadataTypes[outputUnit];
+    List<jsAst.Expression> types = program.metadataTypes[outputUnit];
 
     if (outputUnit == compiler.deferredLoadTask.mainOutputUnit) {
       jsAst.Expression metadataAccess =

tests/compiler/dart2js_extra/23404_test.dart

@@ -0,0 +1,21 @@
+// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Regression test for http://dartbug.com/23404/
+import 'package:expect/expect.dart';
+
+// Dart2js crashed when the global metadata had escaped characters. That
+// happens, for example, when tearing off a function that uses a default
+// argument containing escape characters.
+foo([a='\u00a0']) => a;
+bar() => '';
+
+@NoInline
+@AssumeDynamic
+confuse(x) => x;
+
+main() {
+  Expect.equals('\u00a0', confuse(foo)());
+  Expect.equals('', confuse(bar)());
+}