dart2js-kernel: Recognize some List constructors.

pkg/compiler/lib/src/ssa/builder_kernel.dart

Index: pkg/compiler/lib/src/ssa/builder_kernel.dart
diff --git a/pkg/compiler/lib/src/ssa/builder_kernel.dart b/pkg/compiler/lib/src/ssa/builder_kernel.dart
index d97fe22aaa4c95a60c2667a3c3d337f01869ed55..25a73a3afad7b47f70e0ad8e55c86b71a8d1c8ab 100644
--- a/pkg/compiler/lib/src/ssa/builder_kernel.dart
+++ b/pkg/compiler/lib/src/ssa/builder_kernel.dart
@@ -2016,8 +2016,7 @@ class KernelSsaGraphBuilder extends ir.Visitor
         element.accept(this);
         elements.add(pop());
       }
-      listInstruction =
-          new HLiteralList(elements, commonMasks.extendableArrayType);
+      listInstruction = buildLiteralList(elements);
       add(listInstruction);
       InterfaceType type = localsHandler.substInContext(_commonElements
           .listType(_elementMap.getDartType(listLiteral.typeArgument)));
@@ -2055,8 +2054,7 @@ class KernelSsaGraphBuilder extends ir.Visitor
       constructor = _commonElements.mapLiteralConstructorEmpty;
     } else {
       constructor = _commonElements.mapLiteralConstructor;
-      HLiteralList argList =
-          new HLiteralList(constructorArgs, commonMasks.extendableArrayType);
+      HLiteralList argList = buildLiteralList(constructorArgs);
       add(argList);
       inputs.add(argList);
     }
@@ -2404,7 +2402,6 @@ class KernelSsaGraphBuilder extends ir.Visitor
     List<HInstruction> arguments =
         _visitArgumentsForStaticTarget(target.function, invocation.arguments);
 
-    // TODO(johnniwinther): Move factory calls to a helper function?
     if (function is ConstructorEntity && function.isFactoryConstructor) {
       if (function.isExternal && function.isFromEnvironmentConstructor) {

[Siggi Cherem (dart-lang), 2017/07/25 16:54:45]

nit: consider moving this at the beginning of handleInvokeFactoryConstructor?

[sra1, 2017/07/25 20:08:15]

Done.

         if (invocation.isConst) {
@@ -2420,36 +2417,126 @@ class KernelSsaGraphBuilder extends ir.Visitor
         return;
       }
 
-      // Factory constructors take type parameters; other static methods ignore
-      // them.
+      handleInvokeFactoryConstructor(invocation, function, typeMask, arguments);
+      return;
+    }
 
-      if (closedWorld.rtiNeed.classNeedsRti(function.enclosingClass)) {
-        _addTypeArguments(arguments, invocation.arguments);
-      }
+    // Static methods currently ignore the type parameters.
+    _pushStaticInvocation(function, arguments, typeMask);
+  }
 
-      _pushStaticInvocation(function, arguments, typeMask);
+  void handleInvokeFactoryConstructor(
+      ir.StaticInvocation invocation,
+      FunctionEntity function,
+      TypeMask typeMask,
+      List<HInstruction> arguments) {
+    bool isFixedList = false; // Any fixed list, e.g. new List(10),  UInt8List.
+
+    // Recognize `new List()` and `new List(n)`.
+    bool isFixedListConstructorCall = false;
+    bool isGrowableListConstructorCall = false;
+    if (commonElements.isUnnamedListConstructor(function) &&
+        invocation.arguments.named.isEmpty) {
+      int argumentCount = invocation.arguments.positional.length;
+      isFixedListConstructorCall = argumentCount == 1;
+      isGrowableListConstructorCall = argumentCount == 0;
+      isFixedList = isFixedListConstructorCall;
+    }
+
+    TypeMask resultType = typeMask;
+
+    bool isJSArrayTypedConstructor =
+        function == commonElements.jsArrayTypedConstructor;
+
+    if (isFixedListConstructorCall) {
+      assert(arguments.length == 1);
+      HInstruction lengthInput = arguments.first;
+      if (!lengthInput.isNumber(closedWorld)) {
+        HTypeConversion conversion = new HTypeConversion(
+            null,
+            HTypeConversion.ARGUMENT_TYPE_CHECK,
+            commonMasks.numType,
+            lengthInput);
+        add(conversion);
+        lengthInput = conversion;
+      }
+      js.Template code = js.js.parseForeignJS('new Array(#)');
+      var behavior = new native.NativeBehavior();
+      // TODO(sra): Find the full type being created here, e.g. JSArray<Set<T>>.
+      var expectedType = commonElements.listType();
+      behavior.typesInstantiated.add(expectedType);
+      behavior.typesReturned.add(expectedType);
+
+      // The allocation can throw only if the given length is a double or
+      // outside the unsigned 32 bit range.
+      // TODO(sra): Array allocation should be an instruction so that canThrow
+      // can depend on a length type discovered in optimization.
+      bool canThrow = true;
+      if (lengthInput.isUInt32(closedWorld)) {
+        canThrow = false;
+      }
 
-      bool isFixedListConstructorCall = false;
-      bool isGrowableListConstructorCall = false;
-      if (commonElements.isUnnamedListConstructor(function) &&
-          invocation.arguments.named.isEmpty) {
-        isFixedListConstructorCall =
-            invocation.arguments.positional.length == 1;
-        isGrowableListConstructorCall = invocation.arguments.positional.isEmpty;
+      // TODO(sra): Pick up type inference type, which might be more precise,
+      // e.g. a container type.
+      resultType = commonMasks.fixedListType;
+      HForeignCode foreign = new HForeignCode(code, resultType, arguments,
+          nativeBehavior: behavior,
+          throwBehavior: canThrow
+              ? native.NativeThrowBehavior.MAY
+              : native.NativeThrowBehavior.NEVER);
+      push(foreign);
+      // TODO(sra): Global type analysis tracing may have determined that the
+      // fixed-length property is never checked. If so, we can avoid marking the
+      // array.
+      if (true) {

[Siggi Cherem (dart-lang), 2017/07/25 16:54:45]

(no action required)

In case you were considering alternatives, another style I've seen for these
blocks that are always true is to define a block, but no if:

push(foreign);
// TODO: blah blah
{
  doSomething();
}

[sra1, 2017/07/25 20:08:15]

Done.

+        js.Template code = js.js.parseForeignJS(r'#.fixed$length = Array');
+        // We set the instruction as [canThrow] to avoid it being dead code.
+        // We need a finer grained side effect.
+        add(new HForeignCode(code, commonMasks.nullType, [stack.last],
+            throwBehavior: native.NativeThrowBehavior.MAY));
       }
-      bool isJSArrayTypedConstructor =
-          function == commonElements.jsArrayTypedConstructor;
-      if (rtiNeed.classNeedsRti(commonElements.listClass) &&
-          (isFixedListConstructorCall ||
-              isGrowableListConstructorCall ||
-              isJSArrayTypedConstructor)) {
-        InterfaceType type = _elementMap.createInterfaceType(
-            target.enclosingClass, invocation.arguments.types);
-        stack.add(_setListRuntimeTypeInfoIfNeeded(pop(), type));
+    } else if (isGrowableListConstructorCall) {
+      push(buildLiteralList(<HInstruction>[]));
+      // TODO(sra): Pick up type inference type, which might be more precise,

[Siggi Cherem (dart-lang), 2017/07/25 16:54:45]

consider using "TODO(redemption)" (so we can grep for everything we left aside
during the migration, as opposed to TODOs that are for further out in the
future.

[sra1, 2017/07/25 20:08:16]

Done.

+      // e.g. a container type.
+      resultType = commonMasks.growableListType;
+      stack.last.instructionType = resultType;
+    } else if (isJSArrayTypedConstructor) {
+      // TODO(sra): Instead of calling the identity-like factory constructor,
+      // simply select the single argument.
+      // Factory constructors take type parameters.
+      if (closedWorld.rtiNeed.classNeedsRti(function.enclosingClass)) {
+        _addTypeArguments(arguments, invocation.arguments);
       }
+      _pushStaticInvocation(function, arguments, typeMask);
     } else {
+      // Factory constructors take type parameters.
+      if (closedWorld.rtiNeed.classNeedsRti(function.enclosingClass)) {
+        _addTypeArguments(arguments, invocation.arguments);
+      }
       _pushStaticInvocation(function, arguments, typeMask);
     }
+
+    HInstruction newInstance = stack.last;
+
+    if (isFixedList) {
+      // If we inlined a constructor the call-site-specific type from type
+      // inference (e.g. a container type) will not be on the node. Store the
+      // more specialized type on the allocation.
+      newInstance.instructionType = resultType;
+      graph.allocatedFixedLists.add(newInstance);
+    }
+
+    if (rtiNeed.classNeedsRti(commonElements.listClass) &&
+        (isFixedListConstructorCall ||
+            isGrowableListConstructorCall ||
+            isJSArrayTypedConstructor)) {
+      InterfaceType type = _elementMap.createInterfaceType(
+          invocation.target.enclosingClass, invocation.arguments.types);
+      stack.add(_setListRuntimeTypeInfoIfNeeded(pop(), type));
+    }
+
+    // TODO(sra): For redirecting factory constructors, check or trust the type.
   }
 
   void handleInvokeStaticForeign(
@@ -3024,8 +3111,7 @@ class KernelSsaGraphBuilder extends ir.Visitor
 
     js.Name internalName = namer.invocationName(selector);
 
-    var argumentsInstruction =
-        new HLiteralList(arguments, commonMasks.extendableArrayType);
+    var argumentsInstruction = buildLiteralList(arguments);
     add(argumentsInstruction);
 
     var argumentNames = new List<HInstruction>();
@@ -3034,8 +3120,7 @@ class KernelSsaGraphBuilder extends ir.Visitor
           constantSystem.createString(argumentName);
       argumentNames.add(graph.addConstant(argumentNameConstant, closedWorld));
     }
-    var argumentNamesInstruction =
-        new HLiteralList(argumentNames, commonMasks.extendableArrayType);
+    var argumentNamesInstruction = buildLiteralList(argumentNames);
     add(argumentNamesInstruction);
 
     ConstantValue kindConstant =