Fix 15413: setRange for views with overlapping buffers.

runtime/lib/typed_data.dart

Index: runtime/lib/typed_data.dart
===================================================================
--- runtime/lib/typed_data.dart	(revision 31504)
+++ runtime/lib/typed_data.dart	(working copy)
@@ -62,6 +62,8 @@
                                              int length]) {
     return new _Uint8ClampedArrayView(buffer, offsetInBytes, length);
   }
+
+  bool _isClamped() { return true; }
 }
 
 
@@ -523,11 +525,38 @@
     return IterableMixinWorkaround.getRangeList(this, start, end);
   }
 
-  void setRange(int start, int end, Iterable iterable, [int skipCount = 0]) {
-    if (!_setRange(start, end - start, iterable, skipCount)) {
-      IterableMixinWorkaround.setRangeList(this, start,
-                                           end, iterable, skipCount);
+  bool _isClamped() { return false; }
+
+  void setRange(int start, int end, Iterable from, [int skipCount = 0]) {
+    if (from is _TypedListBase){
+      final needsClamping =
+          this._isClamped() && (this._isClamped() != from._isClamped());
+      if (!needsClamping &&
+          (this.elementSizeInBytes == from.elementSizeInBytes)) {
+        if (this.buffer._setRange(

[sra1, 2014/01/07 18:55:48]

this.buffer is a ByteBuffer.
There is no guarantee that is is a typed list.

Even this.buffer it is a typed list, this could be a Int8List view of a
Float32List.

I have created a test for underlying-buffer-has-different-element-size-to-view,
which fails on the VM.

[srdjan, 2014/01/07 19:20:35]

OK.

On 2014/01/07 18:55:48, sra1 wrote:

+              start + (this.offsetInBytes ~/ this.elementSizeInBytes),
+              end - start,
+              from.buffer,
+              skipCount + (from.offsetInBytes  ~/ from.elementSizeInBytes))) {
+          return;
+        }
+      } else if (from.buffer == this.buffer) {
+        // Different element sizes, but same buffer means that we need
+        // an intermediate structure.

[sra1, 2014/01/07 18:55:48]

You never need an intermediate structure.
You might need two loops going in different directions.

In setRange_3_test, the first copy needs
(1) an up-loop copying a to aaaa, b to bbbb, c to cccc and d to dddd
(2) a down-loop copying h to hhhh, g to gggg, f to ffff and e to eeee

The point of switch-over is the element that is copied to the same address.
If there is no such address, then one loop is sufficient, but it might be the
up-loop or the down-loop.  This is just a more general case of choosing loop
direction in libc's memmove.

[srdjan, 2014/01/07 19:20:35]

Yes, next CL.

On 2014/01/07 18:55:48, sra1 wrote:

+        // TODO(srdjan): Optimize to skip copying if the range does not overlap.
+        final len = end - start;
+        final buffer = new List(len);
+        for (int i = 0; i < len; i++) {
+          buffer[i] = from[skipCount + i];
+        }
+        for (int i = start; i < end; i++) {
+          this[i] = buffer[i - start];
+        }
+        return;
+      }
     }
+    IterableMixinWorkaround.setRangeList(this, start,
+                                         end, from, skipCount);
   }
 
   void setAll(int index, Iterable iterable) {
@@ -567,12 +596,10 @@
     return this;
   }
 
-
   // Methods implementing the collection interface.
 
   int get length native "TypedData_length";
 
-
   // Internal utility methods.
 
   int _getInt8(int offsetInBytes) native "TypedData_GetInt8";
@@ -738,6 +765,7 @@
     return new _Uint8ClampedArrayView(buffer, offsetInBytes, length);
   }
 
+  bool _isClamped() { return true; }
 
   // Methods implementing List interface.
 
@@ -1503,6 +1531,7 @@
     return _new(length);
   }
 
+  bool _isClamped() { return true; }
 
   // Method(s) implementing the List interface.
 
@@ -2286,7 +2315,6 @@
       length = _length {
   }
 
-
   // Method(s) implementing the TypedData interface.
 
   int get lengthInBytes {
@@ -2419,6 +2447,8 @@
   }
 
 
+  bool _isClamped() { return true; }
+
   // Method(s) implementing List interface.
 
   int operator[](int index) {