[dartdevc] simplify code for emitting constants

This moves the code into a shared location (instead of being copied to
both of the Analyzer/Kernel-based backends), and removes support for
detecting type parameters in constants, which was allowed by strong
mode but is not legal in Dart 2.

Change-Id: Ic8bcf0aa1107bbb7147fd15b648a45e39478cef5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96839
Commit-Queue: Jenny Messerly <jmesserly@google.com>
Auto-Submit: Jenny Messerly <jmesserly@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
diff --git a/pkg/dev_compiler/lib/src/analyzer/code_generator.dart b/pkg/dev_compiler/lib/src/analyzer/code_generator.dart
index a02e070..f2a0d1f 100644
--- a/pkg/dev_compiler/lib/src/analyzer/code_generator.dart
+++ b/pkg/dev_compiler/lib/src/analyzer/code_generator.dart
@@ -70,8 +70,10 @@
     with
         UIAsCodeVisitorMixin<JS.Node>,
         NullableTypeInference,
-        SharedCompiler<LibraryElement, ClassElement>
-    implements AstVisitor<JS.Node> {
+        SharedCompiler<LibraryElement, ClassElement, InterfaceType,
+            FunctionBody>
+    implements
+        AstVisitor<JS.Node> {
   final SummaryDataStore summaryData;
 
   final CompilerOptions options;
@@ -142,6 +144,7 @@
   final ClassElement objectClass;
   final ClassElement stringClass;
   final ClassElement functionClass;
+  final ClassElement internalSymbolClass;
   final ClassElement privateSymbolClass;
   final InterfaceType linkedHashMapImplType;
   final InterfaceType identityHashMapImplType;
@@ -183,8 +186,6 @@
 
   final _superHelpers = Map<String, JS.Method>();
 
-  List<TypeParameterType> _typeParamInConst;
-
   /// Whether we are currently generating code for the body of a `JS()` call.
   bool _isInForeignJS = false;
 
@@ -226,6 +227,7 @@
         objectClass = driver.getClass('dart:core', 'Object'),
         stringClass = driver.getClass('dart:core', 'String'),
         functionClass = driver.getClass('dart:core', 'Function'),
+        internalSymbolClass = driver.getClass('dart:_internal', 'Symbol'),
         privateSymbolClass =
             driver.getClass('dart:_js_helper', 'PrivateSymbol'),
         linkedHashMapImplType =
@@ -246,8 +248,18 @@
 
   LibraryElement get currentLibrary => _currentElement.library;
 
+  @override
   Uri get currentLibraryUri => _currentElement.librarySource.uri;
 
+  @override
+  FunctionBody get currentFunction => _currentFunction;
+
+  @override
+  InterfaceType get privateSymbolType => privateSymbolClass.type;
+
+  @override
+  InterfaceType get internalSymbolType => internalSymbolClass.type;
+
   CompilationUnitElement get _currentCompilationUnit {
     for (var e = _currentElement;; e = e.enclosingElement) {
       if (e is CompilationUnitElement) return e;
@@ -1863,7 +1875,7 @@
       return _emitInstanceCreationExpression(
           element,
           element.returnType as InterfaceType,
-          () => _emitArgumentList(node.arguments),
+          _emitArgumentList(node.arguments),
           isConst: true);
     } else {
       return _visitExpression(node.name);
@@ -2004,7 +2016,7 @@
         values.add(JS.PropertyAccess(
             _emitStaticClassName(classElem), _declareMemberName(f.getter)));
         var enumValue = runtimeCall('const(new (#.#)(#))', [
-          _emitConstructorAccess(type),
+          emitConstructorAccess(type),
           _constructorName(''),
           js.number(index++)
         ]);
@@ -3295,8 +3307,8 @@
     return _emitAnnotatedResult(result, metadata);
   }
 
-  /// Emits an expression that lets you access statics on a [type] from code.
-  JS.Expression _emitConstructorAccess(DartType type) {
+  @override
+  JS.Expression emitConstructorAccess(DartType type) {
     return _emitJSInterop(type.element) ?? _emitType(type);
   }
 
@@ -3348,7 +3360,6 @@
 
     var name = type.name;
     if (type is TypeParameterType) {
-      _typeParamInConst?.add(type);
       return JS.Identifier(name);
     }
 
@@ -3683,7 +3694,7 @@
   JS.Expression _emitTarget(Expression target, Element member, bool isStatic) {
     if (isStatic) {
       if (member is ConstructorElement) {
-        return _emitConstructorAccess(member.enclosingElement.type)
+        return emitConstructorAccess(member.enclosingElement.type)
           ..sourceInformation = _nodeSpan(target);
       }
       if (member is PropertyAccessorElement) {
@@ -4351,7 +4362,7 @@
 
   JS.Expression _emitConstructorName(DartType type, String name) {
     return _emitJSInterop(type.element) ??
-        JS.PropertyAccess(_emitConstructorAccess(type), _constructorName(name));
+        JS.PropertyAccess(emitConstructorAccess(type), _constructorName(name));
   }
 
   @override
@@ -4359,8 +4370,8 @@
     return _emitConstructorName(node.type.type, node.staticElement.name);
   }
 
-  JS.Expression _emitInstanceCreationExpression(ConstructorElement element,
-      InterfaceType type, List<JS.Expression> Function() emitArguments,
+  JS.Expression _emitInstanceCreationExpression(
+      ConstructorElement element, InterfaceType type, List<JS.Expression> args,
       {bool isConst = false, ConstructorName ctorNode}) {
     if (element == null) {
       return _throwUnsafe('unresolved constructor: ${type?.name ?? '<null>'}'
@@ -4369,13 +4380,11 @@
 
     var classElem = type.element;
     if (_isObjectLiteral(classElem)) {
-      var args = emitArguments();
       return args.isEmpty ? js.call('{}') : args.single as JS.ObjectInitializer;
     }
 
-    var name = element.name;
     JS.Expression emitNew() {
-      var args = emitArguments();
+      var name = element.name;
       if (args.isEmpty && classElem.source.isInSystemLibrary) {
         // Skip the slow SDK factory constructors when possible.
         switch (classElem.name) {
@@ -4414,7 +4423,8 @@
           : JS.New(ctor, args);
     }
 
-    return isConst ? _emitConst(emitNew) : emitNew();
+    var result = emitNew();
+    return isConst ? canonicalizeConstObject(result) : result;
   }
 
   bool _isObjectLiteral(Element classElem) {
@@ -4468,25 +4478,23 @@
       return js.escapedString(stringValue);
     }
     if (type == types.symbolType) {
-      return _emitDartSymbol(value.toSymbolValue());
+      return emitDartSymbol(value.toSymbolValue());
     }
     if (type == types.typeType) {
       return _emitType(value.toTypeValue());
     }
     if (type is InterfaceType) {
       if (type.element == types.listType.element) {
-        return _cacheConst(() => _emitConstList(type.typeArguments[0],
-            value.toListValue().map(_emitDartObject).toList()));
+        return _emitConstList(type.typeArguments[0],
+            value.toListValue().map(_emitDartObject).toList());
       }
       if (type.element == types.mapType.element) {
-        return _cacheConst(() {
-          var entries = <JS.Expression>[];
-          value.toMapValue().forEach((key, value) {
-            entries.add(_emitDartObject(key));
-            entries.add(_emitDartObject(value));
-          });
-          return _emitConstMap(type, entries);
+        var entries = <JS.Expression>[];
+        value.toMapValue().forEach((key, value) {
+          entries.add(_emitDartObject(key));
+          entries.add(_emitDartObject(value));
         });
+        return _emitConstMap(type, entries);
       }
       if (value is DartObjectImpl && value.isUserDefinedObject) {
         var ctor = value.getInvocation();
@@ -4502,15 +4510,14 @@
               classElem.fields.where((f) => f.type == type).elementAt(index);
           return _emitClassMemberElement(field, field.getter, null);
         }
-        return _emitInstanceCreationExpression(ctor.constructor, type, () {
-          var args = ctor.positionalArguments.map(_emitDartObject).toList();
-          var named = <JS.Property>[];
-          ctor.namedArguments.forEach((name, value) {
-            named.add(JS.Property(_propertyName(name), _emitDartObject(value)));
-          });
-          if (named.isNotEmpty) args.add(JS.ObjectInitializer(named));
-          return args;
-        }, isConst: true);
+        var args = ctor.positionalArguments.map(_emitDartObject).toList();
+        var named = <JS.Property>[];
+        ctor.namedArguments.forEach((name, value) {
+          named.add(JS.Property(_propertyName(name), _emitDartObject(value)));
+        });
+        if (named.isNotEmpty) args.add(JS.ObjectInitializer(named));
+        return _emitInstanceCreationExpression(ctor.constructor, type, args,
+            isConst: true);
       }
     }
     if (value is DartObjectImpl && type is FunctionType) {
@@ -4557,7 +4564,7 @@
     return _emitInstanceCreationExpression(
         element,
         getType(constructor.type) as InterfaceType,
-        () => _emitArgumentList(node.argumentList),
+        _emitArgumentList(node.argumentList),
         isConst: node.isConst,
         ctorNode: constructor);
   }
@@ -4954,28 +4961,6 @@
     return id;
   }
 
-  JS.Expression _cacheConst(JS.Expression expr()) {
-    var savedTypeParams = _typeParamInConst;
-    _typeParamInConst = [];
-
-    var jsExpr = expr();
-
-    bool usesTypeParams = _typeParamInConst.isNotEmpty;
-    _typeParamInConst = savedTypeParams;
-
-    // TODO(jmesserly): if it uses type params we can still hoist it up as far
-    // as it will go, e.g. at the level the generic class is defined where type
-    // params are available.
-    if (_currentFunction == null || usesTypeParams) return jsExpr;
-
-    var temp = JS.TemporaryId('const');
-    moduleItems.add(js.statement('let #;', [temp]));
-    return js.call('# || (# = #)', [temp, temp, jsExpr]);
-  }
-
-  JS.Expression _emitConst(JS.Expression expr()) =>
-      _cacheConst(() => runtimeCall('const(#)', expr()));
-
   /// Returns a new expression, which can be be used safely *once* on the
   /// left hand side, and *once* on the right side of an assignment.
   /// For example: `expr1[expr2] += y` can be compiled as
@@ -5464,7 +5449,7 @@
     var createStreamIter = _emitInstanceCreationExpression(
         streamIterator.element.unnamedConstructor,
         streamIterator,
-        () => [_visitExpression(iterable)]);
+        [_visitExpression(iterable)]);
     var iter = JS.TemporaryId('iter');
     var variable = identifier ?? loopVariable.identifier;
     var init = _visitExpression(identifier);
@@ -5701,24 +5686,7 @@
 
   @override
   visitSymbolLiteral(SymbolLiteral node) {
-    return _emitConst(() => _emitDartSymbol(node.components.join('.')));
-  }
-
-  JS.Expression _emitDartSymbol(String name) {
-    // TODO(vsm): Handle qualified symbols correctly.
-    var last = name.substring(name.lastIndexOf('.') + 1);
-    var jsName = js.string(name, "'");
-    if (last.startsWith('_')) {
-      var nativeSymbol = emitPrivateNameSymbol(currentLibrary, last);
-      return js.call('new #.new(#, #)', [
-        _emitConstructorAccess(privateSymbolClass.type),
-        jsName,
-        nativeSymbol
-      ]);
-    } else {
-      return js
-          .call('#.new(#)', [_emitConstructorAccess(types.symbolType), jsName]);
-    }
+    return emitDartSymbol(node.components.join('.'));
   }
 
   @override
@@ -5728,8 +5696,7 @@
     if (!node.isConst) {
       return _emitList(elementType, elements);
     }
-
-    return _cacheConst(() => _emitConstList(elementType, elements));
+    return _emitConstList(elementType, elements);
   }
 
   // TODO(nshahan) Cleanup after control flow collections experiments are removed.
@@ -5745,7 +5712,7 @@
       }
       return js.call('#.from([#])', [setType, jsElements]);
     }
-    return _cacheConst(() =>
+    return cacheConst(
         runtimeCall('constSet(#, [#])', [_emitType(elementType), jsElements]));
   }
 
@@ -5753,7 +5720,8 @@
       DartType elementType, List<JS.Expression> elements) {
     // dart.constList helper internally depends on _interceptors.JSArray.
     _declareBeforeUse(_jsArray);
-    return runtimeCall('constList([#], #)', [elements, _emitType(elementType)]);
+    return cacheConst(
+        runtimeCall('constList([#], #)', [elements, _emitType(elementType)]));
   }
 
   JS.Expression _emitList(DartType itemType, List<JS.Expression> items) {
@@ -5781,13 +5749,13 @@
       }
       return js.call('new #.from([#])', [mapType, jsElements]);
     }
-    return _cacheConst(() => _emitConstMap(type, jsElements));
+    return _emitConstMap(type, jsElements);
   }
 
   JS.Expression _emitConstMap(InterfaceType type, List<JS.Expression> entries) {
     var typeArgs = type.typeArguments;
-    return runtimeCall('constMap(#, #, [#])',
-        [_emitType(typeArgs[0]), _emitType(typeArgs[1]), entries]);
+    return cacheConst(runtimeCall('constMap(#, #, [#])',
+        [_emitType(typeArgs[0]), _emitType(typeArgs[1]), entries]));
   }
 
   JS.Expression _emitMapImplType(InterfaceType type, {bool identity}) {
diff --git a/pkg/dev_compiler/lib/src/compiler/shared_compiler.dart b/pkg/dev_compiler/lib/src/compiler/shared_compiler.dart
index be9b13f..b6d91c6 100644
--- a/pkg/dev_compiler/lib/src/compiler/shared_compiler.dart
+++ b/pkg/dev_compiler/lib/src/compiler/shared_compiler.dart
@@ -3,6 +3,8 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:collection';
+import 'package:meta/meta.dart';
+
 import '../compiler/js_metalet.dart' as JS;
 import '../compiler/js_names.dart' as JS;
 import '../compiler/js_utils.dart' as JS;
@@ -13,19 +15,27 @@
 ///
 /// This class should only implement functionality that depends purely on JS
 /// classes, rather than on Analyzer/Kernel types.
-abstract class SharedCompiler<Library, Class> {
+abstract class SharedCompiler<Library, Class, InterfaceType, FunctionNode> {
   /// When inside a `[]=` operator, this will be a non-null value that should be
   /// returned by any `return;` statement.
   ///
   /// This lets DDC use the setter method's return value directly.
   final List<JS.Identifier> _operatorSetResultStack = [];
 
+  /// The identifier used to reference DDC's core "dart:_runtime" library from
+  /// generated JS code, typically called "dart" e.g. `dart.dcall`.
+  @protected
   JS.Identifier runtimeModule;
+
+  /// The temporary variable that stores named arguments (these are passed via a
+  /// JS object literal, to match JS conventions).
+  @protected
   final namedArgumentTemp = JS.TemporaryId('opts');
 
   final _privateNames = HashMap<Library, HashMap<String, JS.TemporaryId>>();
 
   /// The list of output module items, in the order they need to be emitted in.
+  @protected
   final moduleItems = <JS.ModuleItem>[];
 
   /// Like [moduleItems] but for items that should be emitted after classes.
@@ -34,6 +44,33 @@
   /// classes.
   final afterClassDefItems = <JS.ModuleItem>[];
 
+  /// The type used for private Dart [Symbol]s.
+  @protected
+  InterfaceType get privateSymbolType;
+
+  /// The type used for public Dart [Symbol]s.
+  @protected
+  InterfaceType get internalSymbolType;
+
+  @protected
+  Library get currentLibrary;
+
+  /// The import URI of current library.
+  @protected
+  Uri get currentLibraryUri;
+
+  /// The current function being compiled, if any.
+  @protected
+  FunctionNode get currentFunction;
+
+  /// Whether any superclass of [c] defines a static [name].
+  @protected
+  bool superclassHasStatic(Class c, String name);
+
+  /// Emits the expression necessary to access a constructor of [type];
+  @protected
+  JS.Expression emitConstructorAccess(InterfaceType type);
+
   /// When compiling the body of a `operator []=` method, this will be non-null
   /// and will indicate the the value that should be returned from any `return;`
   /// statements.
@@ -42,9 +79,11 @@
     return stack.isEmpty ? null : stack.last;
   }
 
-  /// The import URI of current library.
-  Uri get currentLibraryUri;
-
+  /// Called when starting to emit methods/functions, in particular so we can
+  /// implement special handling of the user-defined `[]=` and `==` methods.
+  ///
+  /// See also [exitFunction] and [emitReturnStatement].
+  @protected
   void enterFunction(String name, List<JS.Parameter> formals,
       bool Function() isLastParamMutated) {
     if (name == '[]=') {
@@ -56,6 +95,9 @@
     }
   }
 
+  /// Called when finished emitting methods/functions, and must correspond to a
+  /// previous [enterFunction] call.
+  @protected
   JS.Block exitFunction(
       String name, List<JS.Parameter> formals, JS.Block code) {
     if (name == "==" &&
@@ -93,6 +135,7 @@
 
   /// Emits a return statement `return <value>;`, handling special rules for
   /// the `operator []=` method.
+  @protected
   JS.Statement emitReturnStatement(JS.Expression value) {
     if (_operatorSetResult != null) {
       var result = JS.Return(_operatorSetResult);
@@ -112,6 +155,7 @@
   ///
   ///     dart.asInt(<expr>)
   ///
+  @protected
   JS.Expression runtimeCall(String code, [args]) {
     if (args != null) {
       var newArgs = <Object>[runtimeModule];
@@ -129,10 +173,17 @@
 
   /// Calls [runtimeCall] and uses `toStatement()` to convert the resulting
   /// expression into a statement.
+  @protected
   JS.Statement runtimeStatement(String code, [args]) {
     return runtimeCall(code, args).toStatement();
   }
 
+  /// Emits a private name JS Symbol for [name] scoped to the Dart [library].
+  ///
+  /// If the same name is used in multiple libraries in the same module,
+  /// distinct symbols will be used, so the names cannot be referenced outside
+  /// of their library.
+  @protected
   JS.TemporaryId emitPrivateNameSymbol(Library library, String name) {
     return _privateNames.putIfAbsent(library, () => HashMap()).putIfAbsent(name,
         () {
@@ -152,7 +203,8 @@
   ///
   /// This will use `className.name = value` if possible, otherwise it will use
   /// `dart.defineValue(className, name, value)`. This is required when
-  /// `Function.prototype` already defins a getters with the same name.
+  /// `FunctionNode.prototype` already defins a getters with the same name.
+  @protected
   JS.Expression defineValueOnClass(Class c, JS.Expression className,
       JS.Expression nameExpr, JS.Expression value) {
     var args = [className, nameExpr, value];
@@ -165,8 +217,66 @@
     return js.call('#.# = #', args);
   }
 
-  /// Whether any superclass of [c] defines a static [name].
-  bool superclassHasStatic(Class c, String name);
+  /// Caches a constant (list/set/map or class instance) in a variable, so it's
+  /// only canonicalized once at this location in the code, which improves
+  /// performance.
+  ///
+  /// This method ensures the constant is not initialized until use.
+  ///
+  /// The expression [jsExpr] should contain the already-canonicalized constant.
+  /// If the constant is not canonicalized yet, it should be wrapped in the
+  /// appropriate call, such as:
+  ///
+  /// - dart.constList (for Lists),
+  /// - dart.constMap (for Maps),
+  /// - dart.constSet (for Sets),
+  /// - dart.const (for other instances of classes)
+  ///
+  /// [canonicalizeConstObject] can be used for class instances; it will wrap
+  /// the expression in `dart.const` and then call this method.
+  ///
+  /// If the same consant is used elsewhere (in this module, or another module),
+  /// that will require a second canonicalization. In general it is uncommon
+  /// to define the same large constant (such as lists, maps) in different
+  /// locations, because that requires copy+paste, so in practice this
+  /// optimization is rather effective (we should consider caching once
+  /// per-module, though, as that would be relatively easy for the compiler to
+  /// implement once we have a single Kernel backend).
+  @protected
+  JS.Expression cacheConst(JS.Expression jsExpr) {
+    if (currentFunction == null) return jsExpr;
+
+    var temp = JS.TemporaryId('const');
+    moduleItems.add(js.statement('let #;', [temp]));
+    return js.call('# || (# = #)', [temp, temp, jsExpr]);
+  }
+
+  /// Emits a Dart Symbol with the given member [symbolName].
+  ///
+  /// If the symbol refers to a private name, its library will be set to the
+  /// [currentLibrary], so the Symbol is scoped properly.
+  @protected
+  JS.Expression emitDartSymbol(String symbolName) {
+    // TODO(vsm): Handle qualified symbols correctly.
+    var last = symbolName.split('.').last;
+    var name = js.escapedString(symbolName, "'");
+    JS.Expression result;
+    if (last.startsWith('_')) {
+      var nativeSymbol = emitPrivateNameSymbol(currentLibrary, last);
+      result = js.call('new #.new(#, #)',
+          [emitConstructorAccess(privateSymbolType), name, nativeSymbol]);
+    } else {
+      result = js.call(
+          'new #.new(#)', [emitConstructorAccess(internalSymbolType), name]);
+    }
+    return canonicalizeConstObject(result);
+  }
+
+  /// Calls the `dart.const` function in "dart:_runtime" to canonicalize a
+  /// constant instance of a user-defined class stored in [expr].
+  @protected
+  JS.Expression canonicalizeConstObject(JS.Expression expr) =>
+      cacheConst(runtimeCall('const(#)', expr));
 }
 
 /// Whether a variable with [name] is referenced in the [node].
diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
index 2322008..f8f90bb 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -32,7 +32,7 @@
 import 'type_table.dart';
 
 class ProgramCompiler extends Object
-    with SharedCompiler<Library, Class>
+    with SharedCompiler<Library, Class, InterfaceType, FunctionNode>
     implements
         StatementVisitor<JS.Statement>,
         ExpressionVisitor<JS.Expression>,
@@ -100,8 +100,6 @@
 
   FunctionNode _currentFunction;
 
-  List<TypeParameter> _typeParamInConst;
-
   /// Whether we are currently generating code for the body of a `JS()` call.
   bool _isInForeignJS = false;
 
@@ -233,8 +231,21 @@
         syncIterableClass = sdk.getClass('dart:_js_helper', 'SyncIterable'),
         asyncStarImplClass = sdk.getClass('dart:async', '_AsyncStarImpl');
 
+  @override
   Uri get currentLibraryUri => _currentLibrary.importUri;
 
+  @override
+  Library get currentLibrary => _currentLibrary;
+
+  @override
+  FunctionNode get currentFunction => _currentFunction;
+
+  @override
+  InterfaceType get privateSymbolType => privateSymbolClass.rawType;
+
+  @override
+  InterfaceType get internalSymbolType => coreTypes.internalSymbolClass.rawType;
+
   bool get emitMetadata => options.emitMetadata;
 
   JS.Program emitModule(Component component, List<Component> summaries,
@@ -1119,25 +1130,6 @@
     return runtimeStatement('addTypeTests(#, #)', [defaultInst, isClassSymbol]);
   }
 
-  JS.Expression _emitDartSymbol(String symbolName) {
-    // TODO(vsm): Handle qualified symbols correctly.
-    var last = symbolName.split('.').last;
-    var name = js.escapedString(symbolName, "'");
-    if (last.startsWith('_')) {
-      var nativeSymbol = emitPrivateNameSymbol(_currentLibrary, last);
-      return js.call('new #.new(#, #)', [
-        _emitConstructorAccess(privateSymbolClass.rawType),
-        name,
-        nativeSymbol
-      ]);
-    } else {
-      return js.call('new #.new(#)', [
-        _emitConstructorAccess(coreTypes.internalSymbolClass.rawType),
-        name
-      ]);
-    }
-  }
-
   void _emitDartSymbols(
       Iterable<JS.TemporaryId> vars, List<JS.ModuleItem> body) {
     for (var id in vars) {
@@ -1901,7 +1893,7 @@
       ..sourceInformation = _nodeEnd(node.fileEndOffset);
   }
 
-  /// Emits an expression that lets you access statics on a [type] from code.
+  @override
   JS.Expression emitConstructorAccess(InterfaceType type) {
     return _emitJSInterop(type.classNode) ?? visitInterfaceType(type);
   }
@@ -2725,7 +2717,6 @@
   visitTypeParameterType(type) => _emitTypeParameter(type.parameter);
 
   JS.Identifier _emitTypeParameter(TypeParameter t) {
-    _typeParamInConst?.add(t);
     return JS.Identifier(getTypeParameterName(t));
   }
 
@@ -4623,12 +4614,10 @@
   visitConstructorInvocation(ConstructorInvocation node) {
     var ctor = node.target;
     var args = node.arguments;
-    JS.Expression emitNew() {
-      return JS.New(_emitConstructorName(node.constructedType, ctor),
-          _emitArgumentList(args, types: false));
-    }
+    var result = JS.New(_emitConstructorName(node.constructedType, ctor),
+        _emitArgumentList(args, types: false));
 
-    return node.isConst ? _emitConst(emitNew) : emitNew();
+    return node.isConst ? canonicalizeConstObject(result) : result;
   }
 
   JS.Expression _emitFactoryInvocation(StaticInvocation node) {
@@ -4681,12 +4670,10 @@
       }
     }
 
-    JS.Expression emitNew() {
-      return JS.Call(_emitConstructorName(type, ctor),
-          _emitArgumentList(args, types: false));
-    }
+    var result = JS.Call(_emitConstructorName(type, ctor),
+        _emitArgumentList(args, types: false));
 
-    return node.isConst ? _emitConst(emitNew) : emitNew();
+    return node.isConst ? canonicalizeConstObject(result) : result;
   }
 
   JS.Expression _emitJSInteropNew(Member ctor, Arguments args) {
@@ -4863,31 +4850,7 @@
   }
 
   @override
-  visitSymbolLiteral(SymbolLiteral node) {
-    return _emitConst(() => _emitDartSymbol(node.value));
-  }
-
-  JS.Expression _cacheConst(JS.Expression expr()) {
-    var savedTypeParams = _typeParamInConst;
-    _typeParamInConst = [];
-
-    var jsExpr = expr();
-
-    bool usesTypeParams = _typeParamInConst.isNotEmpty;
-    _typeParamInConst = savedTypeParams;
-
-    // TODO(jmesserly): if it uses type params we can still hoist it up as far
-    // as it will go, e.g. at the level the generic class is defined where type
-    // params are available.
-    if (_currentFunction == null || usesTypeParams) return jsExpr;
-
-    var temp = JS.TemporaryId('const');
-    moduleItems.add(js.statement('let #;', [temp]));
-    return js.call('# || (# = #)', [temp, temp, jsExpr]);
-  }
-
-  JS.Expression _emitConst(JS.Expression expr()) =>
-      _cacheConst(() => runtimeCall('const(#)', expr()));
+  visitSymbolLiteral(SymbolLiteral node) => emitDartSymbol(node.value);
 
   @override
   visitTypeLiteral(TypeLiteral node) => _emitTypeLiteral(node.type);
@@ -4914,12 +4877,12 @@
   @override
   visitListLiteral(ListLiteral node) {
     var elementType = node.typeArgument;
+    var elements = _visitExpressionList(node.expressions);
     // TODO(markzipan): remove const check when we use front-end const eval
     if (!node.isConst) {
-      return _emitList(elementType, _visitExpressionList(node.expressions));
+      return _emitList(elementType, elements);
     }
-    return _cacheConst(() =>
-        _emitConstList(elementType, _visitExpressionList(node.expressions)));
+    return _emitConstList(elementType, elements);
   }
 
   @override
@@ -4934,17 +4897,18 @@
       return js.call(
           '#.from([#])', [setType, _visitExpressionList(node.expressions)]);
     }
-    return _cacheConst(() => runtimeCall('constSet(#, [#])', [
-          _emitType(node.typeArgument),
-          _visitExpressionList(node.expressions)
-        ]));
+    return cacheConst(runtimeCall('constSet(#, [#])', [
+      _emitType(node.typeArgument),
+      _visitExpressionList(node.expressions)
+    ]));
   }
 
   JS.Expression _emitConstList(
       DartType elementType, List<JS.Expression> elements) {
     // dart.constList helper internally depends on _interceptors.JSArray.
     _declareBeforeUse(_jsArrayClass);
-    return runtimeCall('constList([#], #)', [elements, _emitType(elementType)]);
+    return cacheConst(
+        runtimeCall('constList([#], #)', [elements, _emitType(elementType)]));
   }
 
   JS.Expression _emitList(DartType itemType, List<JS.Expression> items) {
@@ -4962,13 +4926,10 @@
 
   @override
   visitMapLiteral(MapLiteral node) {
-    emitEntries() {
-      var entries = <JS.Expression>[];
-      for (var e in node.entries) {
-        entries.add(_visitExpression(e.key));
-        entries.add(_visitExpression(e.value));
-      }
-      return JS.ArrayInitializer(entries);
+    var entries = <JS.Expression>[];
+    for (var e in node.entries) {
+      entries.add(_visitExpression(e.key));
+      entries.add(_visitExpression(e.value));
     }
 
     // TODO(markzipan): remove const check when we use front-end const eval
@@ -4978,10 +4939,10 @@
       if (node.entries.isEmpty) {
         return js.call('new #.new()', [mapType]);
       }
-      return js.call('new #.from(#)', [mapType, emitEntries()]);
+      return js.call('new #.from([#])', [mapType, entries]);
     }
-    return _cacheConst(() => runtimeCall('constMap(#, #, #)',
-        [_emitType(node.keyType), _emitType(node.valueType), emitEntries()]));
+    return cacheConst(runtimeCall('constMap(#, #, [#])',
+        [_emitType(node.keyType), _emitType(node.valueType), entries]));
   }
 
   @override
@@ -5159,44 +5120,35 @@
   // are emitted via their normal expression nodes.
   @override
   defaultConstant(Constant node) => _emitInvalidNode(node);
+
   @override
-  visitSymbolConstant(node) {
-    return _emitConst(() => _emitDartSymbol(node.name));
-  }
+  visitSymbolConstant(node) => emitDartSymbol(node.name);
 
   @override
   visitMapConstant(node) {
-    emitEntries() {
-      var entries = <JS.Expression>[];
-      for (var e in node.entries) {
-        entries.add(visitConstant(e.key));
-        entries.add(visitConstant(e.value));
-      }
-      return JS.ArrayInitializer(entries);
+    var entries = <JS.Expression>[];
+    for (var e in node.entries) {
+      entries.add(visitConstant(e.key));
+      entries.add(visitConstant(e.value));
     }
 
-    return _cacheConst(() => runtimeCall('constMap(#, #, #)',
-        [_emitType(node.keyType), _emitType(node.valueType), emitEntries()]));
+    return cacheConst(runtimeCall('constMap(#, #, [#])',
+        [_emitType(node.keyType), _emitType(node.valueType), entries]));
   }
 
   @override
-  visitListConstant(node) => visitConstantList(node.typeArgument, node.entries);
-
-  /// Visits [Constant] with [_visitConstant].
-  visitConstantList(DartType typeArgument, List<Constant> entries) {
-    return _cacheConst(() =>
-        _emitConstList(typeArgument, entries.map(visitConstant).toList()));
-  }
+  visitListConstant(node) => _emitConstList(
+      node.typeArgument, node.entries.map(visitConstant).toList());
 
   @override
   visitSetConstant(node) {
     // Set literals are currently desugared in the frontend.
     // Implement this method before flipping the supportsSetLiterals flag
     // in DevCompilerTarget to true.
-    return _cacheConst(() => runtimeCall('constSet(#, #)', [
-          _emitType(node.typeArgument),
-          visitConstantList(node.typeArgument, node.entries)
-        ]));
+    return cacheConst(runtimeCall('constSet(#, [#])', [
+      _emitType(node.typeArgument),
+      node.entries.map(visitConstant).toList()
+    ]));
   }
 
   @override
@@ -5214,12 +5166,11 @@
     }
 
     var type = visitInterfaceType(node.getType(types) as InterfaceType);
-    JS.Expression prototype = js("#.prototype", [type]);
-    JS.Property proto_prop = JS.Property(_propertyName("__proto__"), prototype);
-    List<JS.Property> properties = [proto_prop]
+    var prototype = js.call("#.prototype", [type]);
+    var properties = [JS.Property(_propertyName("__proto__"), prototype)]
       ..addAll(node.fieldValues.entries.map(entryToProperty));
-    var objectInit = JS.ObjectInitializer(properties, multiline: true);
-    return _emitConst(() => objectInit);
+    return canonicalizeConstObject(
+        JS.ObjectInitializer(properties, multiline: true));
   }
 
   @override