[dart2wasm] Improve WasmListBase.{setRange,setAll}

- In `setRange`, use `array.copy` instruction when the iterable argument
  is another Wasm-array-backed list. (instead of only when the iterable
  and `this` are identical)

- In `setRange`, add special case for `SubListIterable`.

- In `setAll`, use unchecked reads from `iterable` when it's a
  Wasm-array-backed list.

Also fix various error checking in `setAll` and `setRange`.

`setAll` and `setRange` tests updated to test handling of different
types of iterable arguments.

CoreLibraryReviewExempt: adds internal method
Change-Id: Ib3ed566018e929950eac4d1f3d41dcd406003f91
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/383860
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>

sdk/lib/_internal/wasm/lib/list.dart

@@ -67,54 +67,116 @@ abstract class _ModifiableList<E> extends WasmListBase<E> {
       : super._withData(length, data);
 
   @pragma('wasm:prefer-inline')
+  @override
   void operator []=(int index, E value) {
     indexCheckWithName(index, _length, "[]=");
     _data[index] = value;
   }
 
-  // List interface.
+  @override
   void setRange(int start, int end, Iterable<E> iterable, [int skipCount = 0]) {
     RangeError.checkValidRange(start, end, this.length);
     int length = end - start;
     if (length == 0) return;
     RangeError.checkNotNegative(skipCount, "skipCount");
-    if (identical(this, iterable)) {
-      _data.copy(start, _data, skipCount, length);
-    } else if (iterable is List<E>) {
-      Lists.copy(iterable, skipCount, this, start, length);
-    } else {
-      Iterator<E> it = iterable.iterator;
-      while (skipCount > 0) {
-        if (!it.moveNext()) return;
-        skipCount--;
+
+    // Look through `SubListIterable`s while still testing for the fast case as
+    // first thing.
+    while (true) {
+      if (iterable is WasmListBase) {
+        final iterableWasmList = unsafeCast<WasmListBase>(iterable);
+        if (skipCount + length > iterableWasmList.length) {
+          throw IterableElementError.tooFew();
+        }
+        _data.copy(start, iterableWasmList._data, skipCount, length);
+        return;
       }
-      for (int i = start; i < end; i++) {
-        if (!it.moveNext()) return;
-        _data[i] = it.current;
+
+      if (iterable is List) {
+        final iterableList = unsafeCast<List<E>>(iterable);
+        for (int i = skipCount, j = start; i < skipCount + length; i++, j++) {
+          _data[j] = iterableList[i];
+        }
+        return;
       }
+
+      if (iterable is SubListIterable) {
+        final listIterable = unsafeCast<SubListIterable<E>>(iterable);
+        var sourceLength = listIterable.length;
+        if (sourceLength - skipCount < length) {
+          throw IterableElementError.tooFew();
+        }
+        iterable = SubListIterable.iterableOf(listIterable);
+        skipCount += SubListIterable.startOf(listIterable);
+        continue;
+      }
+
+      break;
+    }
+
+    Iterator<E> it = iterable.iterator;
+    while (skipCount > 0) {
+      if (!it.moveNext()) throw IterableElementError.tooFew();
+      skipCount--;
+    }
+    for (int i = start; i < end; i++) {
+      if (!it.moveNext()) throw IterableElementError.tooFew();
+      _data[i] = it.current;
     }
   }
 
+  @override
   void setAll(int index, Iterable<E> iterable) {
-    if (index < 0 || index > this.length) {
-      throw RangeError.range(index, 0, this.length, "index");
+    final length = this.length;
+
+    // index < 0 || index > length
+    if (index.gtU(length)) {
+      throw RangeError.range(index, 0, length, "index");
     }
-    List<E> iterableAsList;
-    if (identical(this, iterable)) {
-      iterableAsList = this;
-    } else if (iterable is List<E>) {
-      iterableAsList = iterable;
-    } else {
-      for (var value in iterable) {
-        this[index++] = value;
+
+    if (iterable is WasmListBase) {
+      final iterableWasmList = unsafeCast<WasmListBase>(iterable);
+      final elementCount = iterableWasmList.length;
+
+      // Elements to copy = min(length - index, elementCount).
+      int copyCount = length - index;
+      if (copyCount > elementCount) {
+        copyCount = elementCount;
       }
+
+      _data.copy(index, iterableWasmList._data, 0, copyCount);
+
+      if (elementCount > copyCount) {
+        throw IndexError.withLength(length, length);
+      }
+
       return;
     }
-    int length = iterableAsList.length;
-    if (index + length > this.length) {
-      throw RangeError.range(index + length, 0, this.length);
+
+    if (iterable is List) {
+      final iterableList = unsafeCast<List<E>>(iterable);
+      final elementCount = iterableList.length;
+
+      // Elements to copy = min(length - index, elementCount).
+      int copyCount = length - index;
+      if (copyCount > elementCount) {
+        copyCount = elementCount;
+      }
+
+      for (int i = 0, j = index; i < copyCount; i++, j++) {
+        _data[j] = iterableList[i];
+      }
+
+      if (elementCount > copyCount) {
+        throw IndexError.withLength(length, length);
+      }
+
+      return;
+    }
+
+    for (var value in iterable) {
+      this[index++] = value;
     }
-    Lists.copy(iterableAsList, 0, this, index, length);
   }
 
   @override

sdk/lib/internal/iterable.dart

@@ -238,6 +238,14 @@ base class SubListIterable<E> extends ListIterable<E> {
   /** If null, represents the length of the iterable. */
   final int? _endOrLength;
 
+  /// Returns `_iterable` for for internal code.
+  static Iterable<E> iterableOf<E>(SubListIterable<E> subListIterable) =>
+      subListIterable._iterable;
+
+  /// Returns `_start` for for internal code.
+  static int startOf<E>(SubListIterable<E> subListIterable) =>
+      subListIterable._start;
+
   SubListIterable(this._iterable, this._start, this._endOrLength) {
     RangeError.checkNotNegative(_start, "start");
     int? endOrLength = _endOrLength;

tests/corelib/list_set_all_test.dart

@@ -2,10 +2,11 @@
 // 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.
 
-import "package:expect/expect.dart";
 import "dart:collection";
 
-test(List<int> list, int index, Iterable<int> iterable) {
+import "package:expect/expect.dart";
+
+void test(List<int> list, int index, Iterable<int> iterable) {
   List copy = list.toList();
   list.setAll(index, iterable);
   Expect.equals(copy.length, list.length);
@@ -22,7 +23,7 @@ test(List<int> list, int index, Iterable<int> iterable) {
 }
 
 class MyList<T> extends ListBase<T> {
-  List list;
+  final List<T> list;
   MyList(this.list);
   get length => list.length;
   set length(value) {
@@ -37,61 +38,55 @@ class MyList<T> extends ListBase<T> {
   toString() => list.toString();
 }
 
-main() {
-  test([1, 2, 3], 0, [4, 5]);
-  test([1, 2, 3], 1, [4, 5]);
-  test([1, 2, 3], 2, [4]);
-  test([1, 2, 3], 3, []);
-  test([1, 2, 3], 0, [4, 5].map((x) => x));
-  test([1, 2, 3], 1, [4, 5].map((x) => x));
-  test([1, 2, 3], 2, [4].map((x) => x));
-  test([1, 2, 3], 3, [].map((x) => x));
-  test([1, 2, 3], 0, const [4, 5]);
-  test([1, 2, 3], 1, const [4, 5]);
-  test([1, 2, 3], 2, const [4]);
-  test([1, 2, 3], 3, const []);
-  test([1, 2, 3], 0, new Iterable.generate(2, (x) => x + 4));
-  test([1, 2, 3], 1, new Iterable.generate(2, (x) => x + 4));
-  test([1, 2, 3], 2, new Iterable.generate(1, (x) => x + 4));
-  test([1, 2, 3], 3, new Iterable.generate(0, (x) => x + 4));
-  test([1, 2, 3].toList(growable: false), 0, [4, 5]);
-  test([1, 2, 3].toList(growable: false), 1, [4, 5]);
-  test([1, 2, 3].toList(growable: false), 2, [4]);
-  test([1, 2, 3].toList(growable: false), 3, []);
-  test([1, 2, 3].toList(growable: false), 0, [4, 5].map((x) => x));
-  test([1, 2, 3].toList(growable: false), 1, [4, 5].map((x) => x));
-  test([1, 2, 3].toList(growable: false), 2, [4].map((x) => x));
-  test([1, 2, 3].toList(growable: false), 3, [].map((x) => x));
-  test([1, 2, 3].toList(growable: false), 0, const [4, 5]);
-  test([1, 2, 3].toList(growable: false), 1, const [4, 5]);
-  test([1, 2, 3].toList(growable: false), 2, const [4]);
-  test([1, 2, 3].toList(growable: false), 3, const []);
-  test([1, 2, 3].toList(growable: false), 0,
-      new Iterable.generate(2, (x) => x + 4));
-  test([1, 2, 3].toList(growable: false), 1,
-      new Iterable.generate(2, (x) => x + 4));
-  test([1, 2, 3].toList(growable: false), 2,
-      new Iterable.generate(1, (x) => x + 4));
-  test([1, 2, 3].toList(growable: false), 3,
-      new Iterable.generate(0, (x) => x + 4));
-  test(new MyList([1, 2, 3]), 0, [4, 5]);
-  test(new MyList([1, 2, 3]), 1, [4, 5]);
-  test(new MyList([1, 2, 3]), 2, [4]);
-  test(new MyList([1, 2, 3]), 3, []);
-  test(new MyList([1, 2, 3]), 0, [4, 5].map((x) => x));
-  test(new MyList([1, 2, 3]), 1, [4, 5].map((x) => x));
-  test(new MyList([1, 2, 3]), 2, [4].map((x) => x));
-  test(new MyList([1, 2, 3]), 3, [].map((x) => x));
+void main() {
+  for (var makeIterable in iterableMakers) {
+    test([1, 2, 3], 0, makeIterable([4, 5]));
+    test([1, 2, 3], 1, makeIterable([4, 5]));
+    test([1, 2, 3], 2, makeIterable([4]));
+    test([1, 2, 3], 3, makeIterable([]));
+    test([1, 2, 3], 0, makeIterable(const [4, 5]));
+    test([1, 2, 3], 1, makeIterable(const [4, 5]));
+    test([1, 2, 3], 2, makeIterable(const [4]));
+    test([1, 2, 3], 3, makeIterable(const []));
+    test([1, 2, 3].toList(growable: false), 0, makeIterable([4, 5]));
+    test([1, 2, 3].toList(growable: false), 1, makeIterable([4, 5]));
+    test([1, 2, 3].toList(growable: false), 2, makeIterable([4]));
+    test([1, 2, 3].toList(growable: false), 3, makeIterable([]));
+    test([1, 2, 3].toList(growable: false), 0, makeIterable(const [4, 5]));
+    test([1, 2, 3].toList(growable: false), 1, makeIterable(const [4, 5]));
+    test([1, 2, 3].toList(growable: false), 2, makeIterable(const [4]));
+    test([1, 2, 3].toList(growable: false), 3, makeIterable(const []));
+    test(MyList([1, 2, 3]), 0, makeIterable([4, 5]));
+    test(MyList([1, 2, 3]), 1, makeIterable([4, 5]));
+    test(MyList([1, 2, 3]), 2, makeIterable([4]));
+    test(MyList([1, 2, 3]), 3, makeIterable([]));
 
-  Expect.throwsRangeError(() => test([1, 2, 3], -1, [4, 5]));
-  Expect.throwsRangeError(
-      () => test([1, 2, 3].toList(growable: false), -1, [4, 5]));
-  Expect.throwsRangeError(() => test([1, 2, 3], 1, [4, 5, 6]));
-  Expect.throwsRangeError(
-      () => test([1, 2, 3].toList(growable: false), 1, [4, 5, 6]));
-  Expect.throwsRangeError(() => test(new MyList([1, 2, 3]), -1, [4, 5]));
-  Expect.throwsRangeError(() => test(new MyList([1, 2, 3]), 1, [4, 5, 6]));
-  Expect.throwsUnsupportedError(() => test(const [1, 2, 3], 0, [4, 5]));
-  Expect.throwsUnsupportedError(() => test(const [1, 2, 3], -1, [4, 5]));
-  Expect.throwsUnsupportedError(() => test(const [1, 2, 3], 1, [4, 5, 6]));
+    Expect.throwsRangeError(() => test([1, 2, 3], -1, makeIterable([4, 5])));
+    Expect.throwsRangeError(() =>
+        test([1, 2, 3].toList(growable: false), -1, makeIterable([4, 5])));
+    Expect.throwsRangeError(() => test([1, 2, 3], 1, makeIterable([4, 5, 6])));
+    Expect.throwsRangeError(() =>
+        test([1, 2, 3].toList(growable: false), 1, makeIterable([4, 5, 6])));
+    Expect.throwsRangeError(
+        () => test(MyList([1, 2, 3]), -1, makeIterable([4, 5])));
+    Expect.throwsRangeError(
+        () => test(MyList([1, 2, 3]), 1, makeIterable([4, 5, 6])));
+    Expect.throwsUnsupportedError(
+        () => test(const [1, 2, 3], 0, makeIterable([4, 5])));
+    Expect.throwsUnsupportedError(
+        () => test(const [1, 2, 3], -1, makeIterable([4, 5])));
+    Expect.throwsUnsupportedError(
+        () => test(const [1, 2, 3], 1, makeIterable([4, 5, 6])));
+  }
 }
+
+// `setAll` implementations can have type tests and special cases to handle
+// different types of iterables differently, so we test with a few different
+// types of iterables.
+List<Iterable<int> Function(List<int>)> iterableMakers = [
+  (list) => list,
+  MyList.new,
+  (list) => list.where((x) => true),
+  (list) => list.map((x) => x),
+  (list) => list.getRange(0, list.length),
+];

tests/corelib/list_set_range_test.dart

@@ -2,84 +2,136 @@
 // 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.
 
+import "dart:collection";
+
 import "package:expect/expect.dart";
 
-main() {
-  var list = [];
-  list.setRange(0, 0, const []);
-  list.setRange(0, 0, []);
-  list.setRange(0, 0, const [], 1);
-  list.setRange(0, 0, [], 1);
-  Expect.equals(0, list.length);
-  Expect.throwsRangeError(() => list.setRange(0, 1, []));
-  Expect.throwsRangeError(() => list.setRange(0, 1, [], 1));
-  Expect.throwsRangeError(() => list.setRange(0, 1, [1], 0));
+void main() {
+  for (var makeIterable in iterableMakers) {
+    var list = [];
+    list.setRange(0, 0, makeIterable(const []));
+    list.setRange(0, 0, makeIterable([]));
+    list.setRange(0, 0, makeIterable(const []), 1);
+    list.setRange(0, 0, makeIterable([]), 1);
+    Expect.listEquals([], list);
+    Expect.throwsRangeError(() => list.setRange(0, 1, []));
+    Expect.throwsRangeError(() => list.setRange(0, 1, [], 1));
+    Expect.throwsRangeError(() => list.setRange(0, 1, [1], 0));
 
-  list.add(1);
-  list.setRange(0, 0, [], 0);
-  Expect.equals(1, list.length);
-  Expect.equals(1, list[0]);
-  list.setRange(0, 0, const [], 0);
-  Expect.equals(1, list.length);
-  Expect.equals(1, list[0]);
+    list = [1];
+    list.setRange(0, 0, makeIterable([]), 0);
+    Expect.listEquals([1], list);
+    list.setRange(0, 0, makeIterable(const []), 0);
+    Expect.listEquals([1], list);
 
-  Expect.throwsRangeError(() => list.setRange(0, 2, [1, 2]));
-  Expect.equals(1, list.length);
-  Expect.equals(1, list[0]);
+    Expect.throwsRangeError(() => list.setRange(0, 2, [1, 2]));
+    Expect.listEquals([1], list);
 
-  Expect.throwsStateError(() => list.setRange(0, 1, [1, 2], 2));
-  Expect.equals(1, list.length);
-  Expect.equals(1, list[0]);
+    Expect.throwsStateError(() => list.setRange(0, 1, [1, 2], 2));
+    Expect.listEquals([1], list);
 
-  list.setRange(0, 1, [2], 0);
-  Expect.equals(1, list.length);
-  Expect.equals(2, list[0]);
+    Expect.throwsStateError(() => list.setRange(0, 1, list, 2));
+    Expect.listEquals([1], list);
 
-  list.setRange(0, 1, const [3], 0);
-  Expect.equals(1, list.length);
-  Expect.equals(3, list[0]);
+    list.setRange(0, 1, makeIterable([2]), 0);
+    Expect.listEquals([2], list);
 
-  list.addAll([4, 5, 6]);
-  Expect.equals(4, list.length);
-  list.setRange(0, 4, [1, 2, 3, 4]);
-  Expect.listEquals([1, 2, 3, 4], list);
+    list.setRange(0, 1, makeIterable(const [3]), 0);
+    Expect.listEquals([3], list);
 
-  list.setRange(2, 4, [5, 6, 7, 8]);
-  Expect.listEquals([1, 2, 5, 6], list);
+    list = [3, 4, 5, 6];
+    list.setRange(0, 4, makeIterable([1, 2, 3, 4]));
+    Expect.listEquals([1, 2, 3, 4], list);
 
-  Expect.throwsRangeError(() => list.setRange(4, 5, [5, 6, 7, 8]));
-  Expect.listEquals([1, 2, 5, 6], list);
+    list.setRange(2, 4, makeIterable([5, 6, 7, 8]));
+    Expect.listEquals([1, 2, 5, 6], list);
 
-  list.setRange(1, 3, [9, 10, 11, 12]);
-  Expect.listEquals([1, 9, 10, 6], list);
+    Expect.throwsRangeError(
+        () => list.setRange(4, 5, makeIterable([5, 6, 7, 8])));
+    Expect.listEquals([1, 2, 5, 6], list);
+
+    list.setRange(1, 3, makeIterable([9, 10, 11, 12]));
+    Expect.listEquals([1, 9, 10, 6], list);
+  }
 
   testNegativeIndices();
 
   testNonExtendableList();
+
+  testNotEnoughElements();
 }
 
 void testNegativeIndices() {
-  var list = [1, 2];
-  Expect.throwsRangeError(() => list.setRange(-1, 1, [1]));
-  Expect.throwsArgumentError(() => list.setRange(0, 1, [1], -1));
+  for (var makeIterable in iterableMakers) {
+    var list = [1, 2];
+    Expect.throwsRangeError(() => list.setRange(-1, 1, makeIterable([1])));
+    Expect.throwsArgumentError(
+        () => list.setRange(0, 1, makeIterable([1]), -1));
 
-  Expect.throwsRangeError(() => list.setRange(2, 1, [1]));
+    Expect.throwsRangeError(() => list.setRange(2, 1, makeIterable([1])));
 
-  Expect.throwsArgumentError(() => list.setRange(-1, -2, [1], -1));
-  Expect.listEquals([1, 2], list);
+    Expect.throwsArgumentError(
+        () => list.setRange(-1, -2, makeIterable([1]), -1));
+    Expect.listEquals([1, 2], list);
 
-  Expect.throwsRangeError(() => list.setRange(-1, -1, [1]));
-  Expect.listEquals([1, 2], list);
+    Expect.throwsRangeError(() => list.setRange(-1, -1, makeIterable([1])));
+    Expect.listEquals([1, 2], list);
 
-  // The skipCount is only used if the length is not 0.
-  list.setRange(0, 0, [1], -1);
-  Expect.listEquals([1, 2], list);
+    // The skipCount is only used if the length is not 0.
+    list.setRange(0, 0, makeIterable([1]), -1);
+    Expect.listEquals([1, 2], list);
+  }
 }
 
 void testNonExtendableList() {
-  var list = new List<int?>.filled(6, null);
-  Expect.listEquals([null, null, null, null, null, null], list);
-  list.setRange(0, 3, [1, 2, 3, 4]);
-  list.setRange(3, 6, [1, 2, 3, 4]);
-  Expect.listEquals([1, 2, 3, 1, 2, 3], list);
+  for (var makeIterable in iterableMakers) {
+    var list = List<int?>.filled(6, null);
+    Expect.listEquals([null, null, null, null, null, null], list);
+    list.setRange(0, 3, makeIterable([1, 2, 3, 4]));
+    list.setRange(3, 6, makeIterable([1, 2, 3, 4]));
+    Expect.listEquals([1, 2, 3, 1, 2, 3], list);
+  }
 }
+
+// Test errors when there aren't enough elements in the source iterable.
+void testNotEnoughElements() {
+  // Check errors when the source doesn't have enough elements after skipping
+  // `skipCount` elements.
+  for (var makeIterable in iterableMakers) {
+    List<int> list = [1, 2, 3, 4, 5];
+    Expect.throws(() => list.setRange(0, 5, makeIterable([9, 9, 9])));
+  }
+
+  // Check errors when the source doesn't have `skipCount` elements.
+  for (var makeIterable in iterableMakers) {
+    List<int> list = [1, 2, 3, 4, 5];
+    Expect.throws(() => list.setRange(0, 5, makeIterable([9, 9, 9]), 5));
+  }
+}
+
+class MyList<T> extends ListBase<T> {
+  final List<T> list;
+  MyList(this.list);
+  get length => list.length;
+  set length(value) {
+    list.length = value;
+  }
+
+  operator [](index) => list[index];
+  operator []=(index, val) {
+    list[index] = val;
+  }
+
+  toString() => list.toString();
+}
+
+// `setRange` implementations can have type tests and special cases to handle
+// different types of iterables differently, so we test with a few different
+// types of iterables.
+List<Iterable<int> Function(List<int>)> iterableMakers = [
+  (list) => list,
+  MyList.new,
+  (list) => list.where((x) => true),
+  (list) => list.map((x) => x),
+  (list) => list.getRange(0, list.length),
+];