Change getRange to sublist. Make getRange deprecated.

Change getRange to sublist. Make getRange deprecated.

This changes the exception behavior of getRange. It used to accept
a length of zero, no matter what start value. Now the start value
must be a valid list index.

Committed: https://code.google.com/p/dart/source/detail?r=20064

[Lasse Reichstein Nielsen, 2013-03-14 14:02:28]

There might be a bug in pub yet, but the rest seems solid.

[floitsch, 2013-03-14 15:46:06]

LGTM.

https://codereview.chromium.org/12817003/diff/1/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/12817003/diff/1/runtime/lib/array.dart#newcode82
runtime/lib/array.dart:82: Arrays.indicesCheck(this, start, end);
I would make this an IterableMixinWorkaround method.

https://codereview.chromium.org/12817003/diff/1/runtime/tests/vm/dart/byte_ar...
File runtime/tests/vm/dart/byte_array_test.dart (right):

https://codereview.chromium.org/12817003/diff/1/runtime/tests/vm/dart/byte_ar...
runtime/tests/vm/dart/byte_array_test.dart:81: var empty =
array.sublist(array.length, array.length);
you could also copy paste this line and remove the "array.length" (testing the
optional parameter).

https://codereview.chromium.org/12817003/diff/1/sdk/lib/core/list.dart
File sdk/lib/core/list.dart (right):

https://codereview.chromium.org/12817003/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:219: * If [end] is omitted, the end of the list is used.
length of the list ?

https://codereview.chromium.org/12817003/diff/1/tools/testing/dart/http_serve...
File tools/testing/dart/http_server.dart (right):

https://codereview.chromium.org/12817003/diff/1/tools/testing/dart/http_serve...
tools/testing/dart/http_server.dart:199: pathSegments.sublist(1).join('/'));
In theory pathSegments.skip(1).join('/') would be more efficient.
But ok for keeping.

[Lasse Reichstein Nielsen, 2013-03-15 09:13:09]

https://codereview.chromium.org/12817003/diff/1/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/12817003/diff/1/runtime/lib/array.dart#newcode82
runtime/lib/array.dart:82: Arrays.indicesCheck(this, start, end);
It would not be a method on a mixin when we get mixins, so I don't think it fits
there.

https://codereview.chromium.org/12817003/diff/1/runtime/tests/vm/dart/byte_ar...
File runtime/tests/vm/dart/byte_array_test.dart (right):

https://codereview.chromium.org/12817003/diff/1/runtime/tests/vm/dart/byte_ar...
runtime/tests/vm/dart/byte_array_test.dart:81: var empty =
array.sublist(array.length, array.length);
On 2013/03/14 15:46:06, floitsch wrote:
> you could also copy paste this line and remove the "array.length" (testing the
> optional parameter).

Done.

https://codereview.chromium.org/12817003/diff/1/sdk/lib/core/list.dart
File sdk/lib/core/list.dart (right):

https://codereview.chromium.org/12817003/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:219: * If [end] is omitted, the end of the list is used.
On 2013/03/14 15:46:06, floitsch wrote:
> length of the list ?

Done.

https://codereview.chromium.org/12817003/diff/1/tools/testing/dart/http_serve...
File tools/testing/dart/http_server.dart (right):

https://codereview.chromium.org/12817003/diff/1/tools/testing/dart/http_serve...
tools/testing/dart/http_server.dart:199: pathSegments.sublist(1).join('/'));
This code is in the testing directory, so I'll revert the change. I'll switch it
to skip(1) anyway, just as a good example.

[Lasse Reichstein Nielsen, 2013-03-15 09:16:18]

Committed patchset #3 manually as r20064 (presubmit successful).

[Lasse Reichstein Nielsen, 2013-03-15 12:30:49]

CCing Srdjan.

[Ivan Posva, 2013-03-18 03:17:39]

DBC -ip

https://codereview.chromium.org/12817003/diff/10001/runtime/lib/simd128.dart
File runtime/lib/simd128.dart (right):

https://codereview.chromium.org/12817003/diff/10001/runtime/lib/simd128.dart#...
runtime/lib/simd128.dart:586: 
Why are you changing white space here?

[srdjan, 2013-03-18 16:24:41]

DBC

https://codereview.chromium.org/12817003/diff/10001/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/12817003/diff/10001/runtime/lib/array.dart#ne...
runtime/lib/array.dart:87: Arrays.indicesCheck(this, start, end);
It seems that this version of sublist is considerably slower than deleted
getRange for small ranges (7 vs 4 tests). Also remember that using optional
arguments is slow and should not be used in high performance code.

https://codereview.chromium.org/12817003/diff/10001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/lib/js_array.dart (right):

https://codereview.chromium.org/12817003/diff/10001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:151: if (start is
!int) throw new ArgumentError(start);
Isn't this different behavior than in the VM libraries? Didn't we agree not to
do explicit checks?

[floitsch, 2013-03-18 17:02:14]

https://codereview.chromium.org/12817003/diff/10001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/lib/js_array.dart (right):

https://codereview.chromium.org/12817003/diff/10001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:151: if (start is
!int) throw new ArgumentError(start);
On 2013/03/18 16:24:41, srdjan wrote:
> Isn't this different behavior than in the VM libraries?
If you want to have the same behavior on the VM and dart2js we have to check
types everywhere, as otherwise the code would need look the same on the two
platforms.

> Didn't we agree not to do explicit checks?
The result is used in a JS. We must check it. If you prefer we can move it
several lines down, but all 'start' checks are done here, so it makes sense to
do the type check here too.