[dartdevc] Fix for DDC overzealously expanding nested constants

Markzipan · Markzipan · commit 7d6024b0ffda · 2019-07-25T22:21:31.000Z
Fixes #37523 Change-Id: I140d7d8fdce836dc0f7f2961711d70ec965e46d4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109640 Reviewed-by: Nicholas Shahan <nshahan@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -32,13 +32,12 @@ import 'nullable_inference.dart';
 import 'property_model.dart';
 import 'type_table.dart';
 
-class ProgramCompiler extends Object
+class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     with SharedCompiler<Library, Class, InterfaceType, FunctionNode>
     implements
         StatementVisitor<js_ast.Statement>,
         ExpressionVisitor<js_ast.Expression>,
-        DartTypeVisitor<js_ast.Expression>,
-        ConstantVisitor<js_ast.Expression> {
+        DartTypeVisitor<js_ast.Expression> {
   final SharedCompilerOptions _options;
 
   /// Maps a library URI import, that is not in [_libraries], to the
@@ -62,6 +61,16 @@ class ProgramCompiler extends Object
   /// Let variables collected for the given function.
   List<js_ast.TemporaryId> _letVariables;
 
+  final _constTable = js_ast.TemporaryId("CT");
+
+  // Constant getters used to populate the constant table.
+  final _constLazyAccessors = List<js_ast.Method>();
+
+  /// Tracks the index in [moduleItems] where the const table must be inserted.
+  /// Required for SDK builds due to internal circular dependencies.
+  /// E.g., dart.constList depends on JSArray.
+  int _constTableInsertionIndex = 0;
+
   /// The class that is emitting its base class or mixin references, otherwise
   /// null.
   ///
@@ -185,6 +194,9 @@ class ProgramCompiler extends Object
   /// as targets for continue and break.
   final _switchLabelStates = HashMap<Statement, _SwitchLabelState>();
 
+  /// Maps Kernel constants to their JS aliases.
+  final constAliasCache = HashMap<Constant, js_ast.Expression>();
+
   final Class _jsArrayClass;
   final Class _privateSymbolClass;
   final Class _linkedHashMapImplClass;
@@ -287,6 +299,11 @@ class ProgramCompiler extends Object
       _pendingClasses.addAll(l.classes);
     }
 
+    // TODO(markzipan): Don't emit this when compiling the SDK.
+    moduleItems
+        .add(js.statement("const # = Object.create(null);", [_constTable]));
+    _constTableInsertionIndex = moduleItems.length;
+
     // Add implicit dart:core dependency so it is first.
     emitLibraryName(_coreTypes.coreLibrary);
 
@@ -297,6 +314,15 @@ class ProgramCompiler extends Object
     // This is done by forward declaring items.
     libraries.forEach(_emitLibrary);
 
+    // This can cause problems if it's ever true during the SDK build, as it's
+    // emitted before dart.defineLazy.
+    if (_constLazyAccessors.isNotEmpty) {
+      var constTableBody = runtimeStatement(
+          'defineLazy(#, { # })', [_constTable, _constLazyAccessors]);
+      moduleItems.insert(_constTableInsertionIndex, constTableBody);
+      _constLazyAccessors.clear();
+    }
+
     moduleItems.addAll(afterClassDefItems);
     afterClassDefItems.clear();
 
@@ -445,6 +471,11 @@ class ProgramCompiler extends Object
 
     moduleItems.add(_emitClassDeclaration(c));
 
+    // The const table depends on dart.defineLazy, so emit it after the SDK.
+    if (isSdkInternalRuntime(_currentLibrary)) {
+      _constTableInsertionIndex = moduleItems.length;
+    }
+
     _currentClass = savedClass;
     _types.thisType = savedClass?.thisType;
     _currentLibrary = savedLibrary;
@@ -2083,12 +2114,11 @@ class ProgramCompiler extends Object
   }
 
   js_ast.Fun _emitStaticFieldInitializer(Field field) {
-    return js_ast.Fun(
-        [],
-        js_ast.Block(_withLetScope(() => [
-              js_ast.Return(
-                  _visitInitializer(field.initializer, field.annotations))
-            ])));
+    return js_ast.Fun([], js_ast.Block(_withLetScope(() {
+      return [
+        js_ast.Return(_visitInitializer(field.initializer, field.annotations))
+      ];
+    })));
   }
 
   List<js_ast.Statement> _withLetScope(List<js_ast.Statement> visitBody()) {
@@ -3101,6 +3131,9 @@ class ProgramCompiler extends Object
 
   js_ast.Expression _visitExpression(Expression e) {
     if (e == null) return null;
+    if (e is ConstantExpression) {
+      return visitConstant(e.constant);
+    }
     var result = e.accept(this) as js_ast.Expression;
     result.sourceInformation ??= _nodeStart(e);
     return result;
@@ -3780,7 +3813,15 @@ class ProgramCompiler extends Object
 
   @override
   js_ast.Expression visitConstantExpression(ConstantExpression node) =>
-      node.constant.accept(this) as js_ast.Expression;
+      visitConstant(node.constant);
+
+  @override
+  js_ast.Expression canonicalizeConstObject(js_ast.Expression expr) {
+    if (isSdkInternalRuntime(_currentLibrary)) {
+      return super.canonicalizeConstObject(expr);
+    }
+    return runtimeCall('const(#)', [expr]);
+  }
 
   @override
   js_ast.Expression visitVariableGet(VariableGet node) {
@@ -4725,7 +4766,7 @@ class ProgramCompiler extends Object
     if (isFromEnvironmentInvocation(_coreTypes, node)) {
       var value = _constants.evaluate(node);
       if (value is PrimitiveConstant) {
-        return value.accept(this) as js_ast.Expression;
+        return visitConstant(value);
       }
     }
 
@@ -4864,7 +4905,7 @@ class ProgramCompiler extends Object
         node.accept(this);
         if (node is ConstantExpression) {
           var list = node.constant as ListConstant;
-          entries.addAll(list.entries.map(_visitConstant));
+          entries.addAll(list.entries.map(visitConstant));
         } else if (node is ListLiteral) {
           entries.addAll(node.expressions.map(_visitExpression));
         }
@@ -4886,7 +4927,7 @@ class ProgramCompiler extends Object
         node.accept(this);
         if (node is ConstantExpression) {
           var set = node.constant as SetConstant;
-          entries.addAll(set.entries.map(_visitConstant));
+          entries.addAll(set.entries.map(visitConstant));
         } else if (node is SetLiteral) {
           entries.addAll(node.expressions.map(_visitExpression));
         }
@@ -4909,8 +4950,8 @@ class ProgramCompiler extends Object
         if (node is ConstantExpression) {
           var map = node.constant as MapConstant;
           for (var entry in map.entries) {
-            entries.add(_visitConstant(entry.key));
-            entries.add(_visitConstant(entry.value));
+            entries.add(visitConstant(entry.key));
+            entries.add(visitConstant(entry.value));
           }
         } else if (node is MapLiteral) {
           for (var entry in node.entries) {
@@ -5032,11 +5073,7 @@ class ProgramCompiler extends Object
   js_ast.Expression 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, elements);
-    }
-    return _emitConstList(elementType, elements);
+    return _emitList(elementType, elements);
   }
 
   js_ast.Expression _emitList(
@@ -5270,8 +5307,48 @@ class ProgramCompiler extends Object
         findAnnotation(node, test), 'name') as String;
   }
 
-  js_ast.Expression _visitConstant(Constant node) =>
-      node.accept(this) as js_ast.Expression;
+  @override
+  js_ast.Expression cacheConst(js_ast.Expression jsExpr) {
+    if (isSdkInternalRuntime(_currentLibrary)) {
+      return super.cacheConst(jsExpr);
+    }
+    return jsExpr;
+  }
+
+  @override
+  js_ast.Expression visitConstant(Constant node) {
+    if (node is TearOffConstant) {
+      // JS() or JS interop functions should not be lazily loaded.
+      if (node.procedure.isExternal || _isInForeignJS) {
+        return _emitStaticTarget(node.procedure);
+      }
+    }
+    if (isSdkInternalRuntime(_currentLibrary) || node is PrimitiveConstant) {
+      return super.visitConstant(node);
+    }
+    var constAlias = constAliasCache[node];
+    if (constAlias != null) {
+      return constAlias;
+    }
+    var constAliasString = "C${constAliasCache.length}";
+    var constAliasProperty = propertyName(constAliasString);
+    var constAliasId = js_ast.TemporaryId(constAliasString);
+    var constAccessor =
+        js.call("# || #.#", [constAliasId, _constTable, constAliasProperty]);
+    constAliasCache[node] = constAccessor;
+    var constJs = super.visitConstant(node);
+    moduleItems.add(js.statement('let #;', [constAliasId]));
+
+    var func = js_ast.Fun(
+        [],
+        js_ast.Block([
+          js.statement("return # = #;", [constAliasId, constJs])
+        ]));
+    var accessor = js_ast.Method(constAliasProperty, func, isGetter: true);
+    _constLazyAccessors.add(accessor);
+    return constAccessor;
+  }
+
   @override
   js_ast.Expression visitNullConstant(NullConstant node) =>
       js_ast.LiteralNull();
@@ -5324,25 +5401,26 @@ class ProgramCompiler extends Object
   js_ast.Expression visitMapConstant(MapConstant node) {
     var entries = [
       for (var e in node.entries) ...[
-        _visitConstant(e.key),
-        _visitConstant(e.value),
+        visitConstant(e.key),
+        visitConstant(e.value),
       ],
     ];
     return _emitConstMap(node.keyType, node.valueType, entries);
   }
 
   @override
   js_ast.Expression visitListConstant(ListConstant node) => _emitConstList(
-      node.typeArgument, node.entries.map(_visitConstant).toList());
+      node.typeArgument, node.entries.map(visitConstant).toList());
 
   @override
   js_ast.Expression visitSetConstant(SetConstant node) => _emitConstSet(
-      node.typeArgument, node.entries.map(_visitConstant).toList());
+      node.typeArgument, node.entries.map(visitConstant).toList());
 
   @override
   js_ast.Expression visitInstanceConstant(InstanceConstant node) {
+    _declareBeforeUse(node.classNode);
     entryToProperty(MapEntry<Reference, Constant> entry) {
-      var constant = entry.value.accept(this) as js_ast.Expression;
+      var constant = visitConstant(entry.value);
       var member = entry.key.asField;
       return js_ast.Property(
           _emitMemberName(member.name.name, member: member), constant);
@@ -5359,20 +5437,22 @@ class ProgramCompiler extends Object
   }
 
   @override
-  js_ast.Expression visitTearOffConstant(TearOffConstant node) =>
-      _emitStaticGet(node.procedure);
+  js_ast.Expression visitTearOffConstant(TearOffConstant node) {
+    _declareBeforeUse(node.procedure.enclosingClass);
+    return _emitStaticGet(node.procedure);
+  }
 
   @override
   js_ast.Expression visitTypeLiteralConstant(TypeLiteralConstant node) =>
       _emitTypeLiteral(node.type);
 
   @override
   js_ast.Expression visitPartialInstantiationConstant(
-          PartialInstantiationConstant node) =>
-      runtimeCall('gbind(#, #)', [
-        _visitConstant(node.tearOffConstant),
-        node.types.map(_emitType).toList()
-      ]);
+      PartialInstantiationConstant node) =>
+    runtimeCall('gbind(#, #)', [
+      visitConstant(node.tearOffConstant),
+      node.types.map(_emitType).toList()
+    ]);
 
   @override
   js_ast.Expression visitUnevaluatedConstant(UnevaluatedConstant node) =>
