[dart2js] List.generate tweaks

rakudrama · commit-bot@chromium.org · commit 82637793bf0f · 2020-11-13T19:04:00.000Z
- Use 'isBoundsSafe' property of `[]=` to avoid a bounds check. - Accept closures with a block containing a single return statement. Change-Id: I97db02dae14d926345487e863fdb4a1513ab773b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171804 Commit-Queue: Stephen Adams <sra@google.com> Reviewed-by: Mayank Patke <fishythefish@google.com>
diff --git a/pkg/compiler/lib/src/kernel/transformations/list_factory_specializer.dart b/pkg/compiler/lib/src/kernel/transformations/list_factory_specializer.dart
@@ -251,9 +251,22 @@ class ListGenerateLoopBodyInliner extends CloneVisitorNotMembers {
     if (function.namedParameters.isNotEmpty) return false;
 
     final body = function.body;
-    // For now, only arrow functions.
+
+    // Arrow functions.
     if (body is ReturnStatement) return true;
 
+    if (body is Block) {
+      // Simple body containing just a return.
+      final statements = body.statements;
+      if (statements.length == 1 && statements.single is ReturnStatement) {
+        return true;
+      }
+
+      // TODO(sra): We can accept more complex closures but, with diminishing
+      // returns. It would probably be best to handle more complex cases by
+      // improving environment design and inlining.
+    }
+
     return false;
   }
 
diff --git a/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart b/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart
@@ -109,6 +109,13 @@ class InvokeDynamicSpecializer {
     return const InvokeDynamicSpecializer();
   }
 
+  bool requiresBoundsCheck(HInvokeDynamic node, JClosedWorld closedWorld) {
+    if (node.isBoundsSafe) return false;
+    return closedWorld.annotationsData
+        .getIndexBoundsCheckPolicy(node.instructionContext)
+        .isEmitted;
+  }
+
   HBoundsCheck insertBoundsCheck(HInstruction indexerNode, HInstruction array,
       HInstruction indexArgument, JClosedWorld closedWorld) {
     final abstractValueDomain = closedWorld.abstractValueDomain;
@@ -202,9 +209,7 @@ class IndexAssignSpecializer extends InvokeDynamicSpecializer {
     }
 
     HInstruction checkedIndex = index;
-    if (closedWorld.annotationsData
-        .getIndexBoundsCheckPolicy(instruction.instructionContext)
-        .isEmitted) {
+    if (requiresBoundsCheck(instruction, closedWorld)) {
       checkedIndex =
           insertBoundsCheck(instruction, receiver, index, closedWorld);
     }
@@ -283,9 +288,7 @@ class IndexSpecializer extends InvokeDynamicSpecializer {
     }
 
     HInstruction checkedIndex = index;
-    if (closedWorld.annotationsData
-        .getIndexBoundsCheckPolicy(instruction.instructionContext)
-        .isEmitted) {
+    if (requiresBoundsCheck(instruction, closedWorld)) {
       checkedIndex =
           insertBoundsCheck(instruction, receiver, index, closedWorld);
     }
@@ -315,9 +318,7 @@ class RemoveLastSpecializer extends InvokeDynamicSpecializer {
     // We are essentially inlining `result = a[a.length - 1]`. `0` is the only
     // index that can fail so we check zero directly, but we want to report the
     // error index as `-1`, so we add `-1` as an extra input that to the check.
-    if (closedWorld.annotationsData
-        .getIndexBoundsCheckPolicy(instruction.instructionContext)
-        .isEmitted) {
+    if (requiresBoundsCheck(instruction, closedWorld)) {
       HConstant zeroIndex = graph.addConstantInt(0, closedWorld);
       HBoundsCheck check =
           insertBoundsCheck(instruction, receiver, zeroIndex, closedWorld);
diff --git a/pkg/compiler/test/kernel/data/list_generate_3.dart b/pkg/compiler/test/kernel/data/list_generate_3.dart
@@ -0,0 +1,40 @@
+// Copyright (c) 2020, 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.
+
+var list1 = List<int>.generate(10, (i) {
+  return i;
+});
+
+var list2 = List<int>.generate(10, (i) {
+  return i;
+}, growable: true);
+
+var list3 = List<int>.generate(10, (i) {
+  return i;
+}, growable: false);
+
+var list4 = List<int>.generate(10, (i) {
+  return i;
+}, growable: someGrowable);
+
+// Not expanded - complex closure.
+var list5 = List<int>.generate(10, (i) {
+  if (i.isEven) return i + 1;
+  return i - 1;
+});
+
+// Not expanded - inscrutable closure.
+var list6 = List<int>.generate(10, foo);
+int foo(int i) => i;
+
+// Not expanded - inscrutable closure.
+var list7 = List<int>.generate(10, bar);
+int Function(int) get bar => foo;
+
+bool someGrowable = true;
+
+void main() {
+  someGrowable = !someGrowable;
+  print([list1, list2, list3, list4, list5, list6, list7]);
+}
diff --git a/pkg/compiler/test/kernel/data/list_generate_3.dart.expect b/pkg/compiler/test/kernel/data/list_generate_3.dart.expect
@@ -0,0 +1,51 @@
+library #lib;
+import self as self;
+import "dart:core" as core;
+import "dart:_interceptors" as _in;
+
+static field core::List<core::int*>* list1 = block {
+  final _in::JSArray<core::int*> _list = _in::JSArray::allocateGrowable<core::int*>(10);
+  for (core::int i = 0; i.{core::num::<}(10); i = i.{core::num::+}(1)) {
+    core::int* i = i;
+    {
+      _list.{_in::JSArray::[]=}{Invariant,BoundsSafe}(i, i);
+    }
+  }
+} =>_list;
+static field core::List<core::int*>* list2 = block {
+  final _in::JSArray<core::int*> _list = _in::JSArray::allocateGrowable<core::int*>(10);
+  for (core::int i = 0; i.{core::num::<}(10); i = i.{core::num::+}(1)) {
+    core::int* i = i;
+    {
+      _list.{_in::JSArray::[]=}{Invariant,BoundsSafe}(i, i);
+    }
+  }
+} =>_list;
+static field core::List<core::int*>* list3 = block {
+  final _in::JSArray<core::int*> _list = _in::JSArray::allocateFixed<core::int*>(10);
+  for (core::int i = 0; i.{core::num::<}(10); i = i.{core::num::+}(1)) {
+    core::int* i = i;
+    {
+      _list.{_in::JSArray::[]=}{Invariant,BoundsSafe}(i, i);
+    }
+  }
+} =>_list;
+static field core::List<core::int*>* list4 = core::List::generate<core::int*>(10, (core::int* i) → core::int* {
+  return i;
+}, growable: self::someGrowable);
+static field core::List<core::int*>* list5 = core::List::generate<core::int*>(10, (core::int* i) → core::int* {
+  if(i.{core::int::isEven})
+    return i.{core::num::+}(1);
+  return i.{core::num::-}(1);
+});
+static field core::List<core::int*>* list6 = core::List::generate<core::int*>(10, #C1);
+static field core::List<core::int*>* list7 = core::List::generate<core::int*>(10, self::bar);
+static field core::bool* someGrowable = true;
+static method foo(core::int* i) → core::int*
+  return i;
+static get bar() → (core::int*) →* core::int*
+  return #C1;
+static method main() → void {
+  self::someGrowable = !self::someGrowable;
+  core::print(<core::List<core::int*>*>[self::list1, self::list2, self::list3, self::list4, self::list5, self::list6, self::list7]);
+}
