dart2js: Make String.split return List<String>

Two follow-on optimization would reduce the generated code to closer to the original size:

- It would be profitable to write an optimization the removes the type
  information from any list when it can be proven the type information
  is not used.

- Provided the split result list is not modified, we can strengthen
  accesses to be non-null Strings.

Bug: https://github.com/dart-lang/sdk/issues/30548
Change-Id: I87ecdd129ec0227f982bd2e1f34193b3d6b0d81b
Reviewed-on: https://dart-review.googlesource.com/35081
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

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

@@ -959,8 +959,8 @@ abstract class HInstruction implements Spannable {
   }
 
   /// An instruction is an 'allocation' is it is the sole alias for an object.
-  /// This applies to to instructions that allocate new objects and can be
-  /// extended to methods that return other allocations without escaping them.
+  /// This applies to instructions that allocate new objects and can be extended
+  /// to methods that return other allocations without escaping them.
   bool get isAllocation => false;
 
   /// Overridden by [HCheck] to return the actual non-[HCheck]
@@ -1658,11 +1658,7 @@ class HCreateBox extends HInstruction {
 }
 
 abstract class HInvoke extends HInstruction {
-  /**
-    * The first argument must be the target: either an [HStatic] node, or
-    * the receiver of a method-call. The remaining inputs are the arguments
-    * to the invocation.
-    */
+  bool isAllocation = false;
   HInvoke(List<HInstruction> inputs, type) : super(inputs, type) {
     sideEffects.setAllSideEffects();
     sideEffects.setDependsOnSomething();

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

@@ -421,10 +421,7 @@ class SsaInstructionSimplifier extends HBaseVisitor
         }
       } else if (input.isStringOrNull(_closedWorld)) {
         if (applies(commonElements.jsStringSplit)) {
-          HInstruction argument = node.inputs[2];
-          if (argument.isString(_closedWorld)) {
-            target = commonElements.jsStringSplit;
-          }
+          return handleStringSplit(node);
         } else if (applies(commonElements.jsStringOperatorAdd)) {
           // `operator+` is turned into a JavaScript '+' so we need to
           // make sure the receiver and the argument are not null.
@@ -466,6 +463,63 @@ class SsaInstructionSimplifier extends HBaseVisitor
     return node;
   }
 
+  HInstruction handleStringSplit(HInvokeDynamic node) {
+    HInstruction argument = node.inputs[2];
+    if (!argument.isString(_closedWorld)) return node;
+
+    // Replace `s.split$1(pattern)` with
+    //
+    //     t1 = s.split(pattern);
+    //     t2 = String;
+    //     t3 = JSArray<t2>;
+    //     t4 = setRuntimeTypeInfo(t1, t3);
+    //
+
+    TypeMask resultMask = _closedWorld.commonMasks.growableListType;
+
+    HInvokeDynamicMethod splitInstruction = new HInvokeDynamicMethod(
+        node.selector,
+        node.mask,
+        node.inputs.sublist(1),
+        resultMask,
+        node.sourceInformation)
+      ..element = commonElements.jsStringSplit
+      ..isAllocation = true;
+
+    if (!_closedWorld.rtiNeed
+        .classNeedsTypeArguments(commonElements.jsArrayClass)) {
+      return splitInstruction;
+    }
+
+    node.block.addBefore(node, splitInstruction);
+
+    HInstruction stringTypeInfo = new HTypeInfoExpression(
+        TypeInfoExpressionKind.COMPLETE,
+        _closedWorld.elementEnvironment.getThisType(commonElements.stringClass),
+        <HInstruction>[],
+        _closedWorld.commonMasks.dynamicType);
+    node.block.addBefore(node, stringTypeInfo);
+
+    HInstruction typeInfo = new HTypeInfoExpression(
+        TypeInfoExpressionKind.INSTANCE,
+        _closedWorld.elementEnvironment
+            .getThisType(commonElements.jsArrayClass),
+        <HInstruction>[stringTypeInfo],
+        _closedWorld.commonMasks.dynamicType);
+    node.block.addBefore(node, typeInfo);
+
+    HInvokeStatic tagInstruction = new HInvokeStatic(
+        commonElements.setRuntimeTypeInfo,
+        <HInstruction>[splitInstruction, typeInfo],
+        resultMask);
+    // 'Linear typing' trick: [tagInstruction] is the only use of the
+    // [splitInstruction], so it becomes the sole alias.
+    // TODO(sra): Build this knowledge into alias analysis.
+    tagInstruction.isAllocation = true;
+
+    return tagInstruction;
+  }
+
   HInstruction visitInvokeDynamicMethod(HInvokeDynamicMethod node) {
     propagateConstantValueToUses(node);
     if (node.isInterceptedCall) {

sdk/lib/_internal/js_runtime/lib/interceptors.dart

@@ -36,6 +36,7 @@ import 'dart:_js_helper'
         stringReplaceFirstUnchecked,
         stringReplaceFirstMappedUnchecked,
         stringReplaceRangeUnchecked,
+        stringSplitUnchecked,
         throwConcurrentModificationError,
         lookupAndCacheInterceptor,
         StringMatch,

sdk/lib/_internal/js_runtime/lib/js_string.dart

@@ -92,10 +92,10 @@ class JSString extends Interceptor implements String, JSIndexable {
   List<String> split(Pattern pattern) {
     checkNull(pattern);
     if (pattern is String) {
-      return JS('JSExtendableArray', r'#.split(#)', this, pattern);
+      return stringSplitUnchecked(this, pattern);
     } else if (pattern is JSSyntaxRegExp && regExpCaptureCount(pattern) == 0) {
       var re = regExpGetNative(pattern);
-      return JS('JSExtendableArray', r'#.split(#)', this, re);
+      return stringSplitUnchecked(this, re);
     } else {
       return _defaultSplit(pattern);
     }

sdk/lib/_internal/js_runtime/lib/string_helper.dart

@@ -20,6 +20,11 @@ stringContainsStringUnchecked(receiver, other, startIndex) {
   return stringIndexOfStringUnchecked(receiver, other, startIndex) >= 0;
 }
 
+List<String> stringSplitUnchecked(String receiver, pattern) {
+  return new JSArray<String>.markGrowable(JS(
+      'returns:JSExtendableArray;new:true', '#.split(#)', receiver, pattern));
+}
+
 class StringMatch implements Match {
   const StringMatch(int this.start, String this.input, String this.pattern);
 

tests/corelib_2/corelib_2.status

@@ -148,11 +148,11 @@ list_concurrent_modify_test: RuntimeError # dart2js does not fully implement the
 list_test/*: RuntimeError # dart2js doesn't implement strong mode covariance checks
 nan_infinity_test/01: RuntimeError
 regexp/pcre_test: Pass, Slow # Issue 21593
-string_split_test/checkedstore: RuntimeError # Issue 30548: does not check stores into List<String>
 
 [ $compiler == dart2js && $runtime != none && !$checked ]
 growable_list_test: RuntimeError # Concurrent modifications test always runs
 splay_tree_from_iterable_test: RuntimeError
+string_split_test/checkedstore: RuntimeError # Issue 30548: does not check stores into List<String>
 
 [ $compiler == dart2js && $runtime != none && $dart2js_with_kernel ]
 list_concurrent_modify_test: Crash # Issue 30559