Add _TypedDataBuffer.addRange.

lib/typed_buffers.dart

 // Copyright (c) 2013, 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.
 
 /**
  * Growable typed-data lists.
  *
  * These lists works just as a typed-data list, except that they are growable.
  * They use an underlying buffer, and when that buffer becomes too small, it
  * is replaced by a new buffer.
@@ skipping 47 matching lines @@
       _buffer = newBuffer;
     }
     _length = newLength;
   }
 
   void _add(E value) {
     if (_length == _buffer.length) _grow();
     _buffer[_length++] = value;
   }
 
-  // We override the default implementation of `add` and `addAll` because
-  // they grow by setting the length in increments of one. We want to grow
-  // by doubling capacity in most cases.
+  // We override the default implementation of `add` because it grows the list
+  // by setting the length in increments of one. We want to grow by doubling
+  // capacity in most cases.
   void add(E value) { _add(value); }
 
-  void addAll(Iterable<E> values) {
-    for (E value in values) _add(value);
+  /// This adds values from [start] (inclusive) to [end] (exclusive) in
+  /// [values]. [start] defaults to `0` and [end] defaults to `values.length`.
+  void addAll(Iterable<E> values, [int start, int end]) {

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

Use [int start = 0, int end] like other range parameters.
Basically we treat start as non-nullable and end as nullable.

Start out by checking that  start >= 0  and  end == null || end >= start.

[nweiz, 2015/10/15 20:14:10]

Done for consistency, although in general default values for non-bools cause
more pain than they're worth IME.
Done.

+    start ??= 0;
+    if (start == 0 && end == null) {
+      // `ListBase.insertAll` bottoms out on `setRange`, which has good handling
+      // of lists, typed buffers, and typed data lists.
+      insertAll(_length, values);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

You could also update insertAll to also take start/end, and just forward to that
in all cases.

[nweiz, 2015/10/15 20:14:10]

Done.

+      return;
+    }
+
+    var list = values is List ? values : values.toList(growable: false);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

I still don't want to convert the iterable to a list unnecessarily.
That's going to run through the iterable anyway, so we might as well do it
ourselves and avoid the intermediate allocation.

Maybe just:
  if (values is List) {
    // smart stuff
  } else {
    for (var value in values.getRange(start, end)) {
      _add(value);
    }
  }

You can get cleverer than that with the iterable (a tighter loop while there is
room in the buffer), but it likely isn't worth it.

[nweiz, 2015/10/15 20:14:10]

Done.

+    end = RangeError.checkValidRange(start, end, list.length);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

That requires the end to be inside the iterable. We don't generally require that
for iterables because the user is not expected to be able to test it
efficiently.
It should allow a too large start and end, treating it as if you had done
iterable.getRange(start, end).
The fact that we use a list internally is an optimization, but the semantics
should match iterables in general.

So:
  if (values is List) {
    if (start >= values.length) return;
    if (end == null || end > values.length) end = values.length;
    ... code below ...

[nweiz, 2015/10/15 20:14:10]

List.getRange doesn't allow too large a start or end. Iterable doesn't have a
getRange method, but Iterable.take method does allow too large an index. Also,
existing methods in the same vein like List.setRange *do* throw if the iterable
doesn't have enough elements—which is basically the same as end being too high.

[Lasse Reichstein Nielsen, 2015/10/20 08:09:49]

Generally we check the limits on lists and not on iterables (because the user
can check the list to avoid invalid lengths, but can't do it for iterables).
The List.setRange is inconsistent on that - probably because the range is really
on the list and the iterable is just expected to fill it. It's still not
optimal. I would actually consider fixing it (either stop early and return the
number of elements used, or fill the rest with a default value).

[nweiz, 2015/10/20 21:24:39]

Wouldn't this be a breaking change? The error is documented behavior.

I'd really prefer to match List.setRange's current behavior, even if it's
flawed. If it changes later, we can come back and change this as well. But if
you insist, I can change it now.

[floitsch, 2015/10/20 22:30:46]

Since one can leave "end" empty I prefer to check, too.
I almost prefer if skip and take were stricter, but then we would need to add a
boolean flag (or a different method).

+
+    var newLength = _length + end - start;
+    _growBuffer(newLength);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

I prefer _ensureCapacity. The function doesn't actually grow the buffer if the
needed capacity is already available, so the name is misleading.

[nweiz, 2015/10/15 20:14:10]

Done.

+    _setRange(_length, newLength, list, start);
+    _length = newLength;
   }
 
   void insert(int index, E element) {
     if (index < 0 || index > _length) {
       throw new RangeError.range(index, 0, _length);
     }
     if (_length < _buffer.length) {
       _buffer.setRange(index + 1, _length + 1, _buffer, index);
       _buffer[index] = element;
       _length++;
       return;
     }
     List<E> newBuffer = _createBiggerBuffer(null);
     newBuffer.setRange(0, index, _buffer);
     newBuffer.setRange(index + 1, _length + 1, _buffer, index);
     newBuffer[index] = element;
     _length++;
     _buffer = newBuffer;
   }
 
+  /// Grows [_buffer] so that it's at least [requiredLength] long, preserving
+  /// the existing data.

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

/// Ensures that [_buffer] is at least [requiredCapacity] long.
///
/// Grows the buffer if necessary.
void _ensureCapacity(int requiredCapacity) { ....

The parameter isn't the required length, it's the required capacity (actual
length may be greater).

[nweiz, 2015/10/15 20:14:10]

Done.
Okay, I'll also change [_createBiggerBuffer].

+  void _growBuffer(int requiredLength) {
+    if (requiredLength < _buffer.length) return;
+    var newBuffer = _createBiggerBuffer(null);
+    newBuffer.setRange(0, _length, _buffer);
+    _buffer = newBuffer;
+  }
+
   /**
    * Create a bigger buffer.
    *
    * This method determines how much bigger a bigger buffer should
    * be. If [requiredLength] is not null, it will be at least that
    * size. It will always have at least have double the capacity of
    * the current buffer.
    */
   List<E> _createBiggerBuffer(int requiredLength) {
     int newLength = _buffer.length * 2;
     if (requiredLength != null && newLength < requiredLength) {
       newLength = requiredLength;
     } else if (newLength < INITIAL_LENGTH) {
       newLength = INITIAL_LENGTH;
     }
     return _createBuffer(newLength);
   }
 
   void _grow() {
     _buffer = _createBiggerBuffer(null)..setRange(0, _length, _buffer);
   }
 
   void setRange(int start, int end, Iterable<E> source, [int skipCount = 0]) {
     if (end > _length) throw new RangeError.range(end, 0, _length);
+    _setRange(start, end, source, skipCount);
+  }
+
+  /// Like [setRange], but with no bounds checking.
+  void _setRange(int start, int end, Iterable<E> source, int skipCount) {
     if (source is _TypedDataBuffer<E>) {
       _buffer.setRange(start, end, source._buffer, skipCount);
     } else {
       _buffer.setRange(start, end, source, skipCount);
     }
   }
 
   // TypedData.
 
   int get elementSizeInBytes => _buffer.elementSizeInBytes;
@@ skipping 97 matching lines @@
   Int32x4 get _defaultValue => _zero;
   Int32x4List _createBuffer(int size) => new Int32x4List(size);
 }
 
 class Float32x4Buffer extends _TypedDataBuffer<Float32x4> {
   Float32x4Buffer([int initialLength = 0])
       : super(new Float32x4List(initialLength));
   Float32x4 get _defaultValue => new Float32x4.zero();
   Float32x4List _createBuffer(int size) => new Float32x4List(size);
 }