Change RangeError instances to use RangeError.range.

Change RangeError instances to use RangeError.range.
This avoids using the (sometimes confusing) "[...)" notation for half-open ranges.
Also change argument tests to simpler interval tests, and move error handling to the end of the functions.

Mostly in VM typed-data libraries.

R=sgjesse@google.com, srdjan@google.com

Committed: https://github.com/dart-lang/sdk/commit/932bcc6901d70492c6f1b8d000b9555a0db62f4b

Committed: https://github.com/dart-lang/sdk/commit/cf4eaae5a689132369e8ebfbbe52fd71aa5060e6

[Lasse Reichstein Nielsen, 2015-05-13 10:29:14]

lrn@google.com changed reviewers:
+ iposva@google.com, sgjesse@google.com

[Lasse Reichstein Nielsen, 2015-05-13 10:29:14]

lrn@google.com changed required reviewers:
+ sgjesse@google.com

[Søren Gjesse, 2015-05-13 11:22:08]

lgtm

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (right):

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:512: // TODO(srdjan): Optimize to skip copying if
the range does not overlap.
Long line.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:539: assert(end != null);
assert(start != null) as well for completeness.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:541: RangeError.checkValidRange(start, end, length);
Add assert(false)?

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:2673: _typedData._setUint8(offsetInBytes + (index *
Uint8List.BYTES_PER_ELEMENT),
Long line - more below.

[Ivan Posva, 2015-05-14 18:13:18]

Does not look good to me due to subtle changes in behaviour and style.

Please fix...

Thanks,
-Ivan

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (left):

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:499: if (skipCount < 0) {
This part is missed in the new code.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:504: if ((from.length - skipCount) < count) {
This part will be missed if count is 0 in the new code.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (right):

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:489: if (0 <= start && start <= end && end <=
length) {
() around comparisons is generally the style we are using.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:936: if (index >= 0 && index < length) {
Please do not change the style of the functions! Here and across the change.
Preference is on ending with a return.

[Lasse Reichstein Nielsen, 2015-05-18 12:21:09]

PTAL

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (left):

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:499: if (skipCount < 0) {
On 2015/05/14 18:13:18, Ivan Posva wrote:
> This part is missed in the new code.

Acknowledged.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (right):

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:512: // TODO(srdjan): Optimize to skip copying if
the range does not overlap.
On 2015/05/13 11:22:08, Søren Gjesse wrote:
> Long line.

Done.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:539: assert(end != null);
The assert is only relevant because RangeError.checkValidRange accepts a null
end argument. 
The tests at the top of the method prevents this:
  0 <= start   implies start is a num (or it would have thrown)
  && start <= end   implies end is also a num, so the assert is just here as a
reminder.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:541: RangeError.checkValidRange(start, end, length);
Slightly icky, but ok.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:936: if (index >= 0 && index < length) {
This was a, possibly premature, attempt to improve performance. The only
benchmark that was actually affected was TypedDataCopy.ByteSwap by 8.379%, the
remaining benchmarks were generally unaffected.

I'll undo these changes for this CL, and do some more experiments.

https://codereview.chromium.org/1132603003/diff/40001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:2673: _typedData._setUint8(offsetInBytes + (index *
Uint8List.BYTES_PER_ELEMENT),
On 2015/05/13 11:22:08, Søren Gjesse wrote:
> Long line - more below.

Done.

[srdjan, 2015-05-18 16:21:59]

srdjan@google.com changed reviewers:
+ srdjan@google.com

[srdjan, 2015-05-18 16:22:00]

DBC

https://codereview.chromium.org/1132603003/diff/80001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (right):

https://codereview.chromium.org/1132603003/diff/80001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:492: assert(false);
Why is this assert needed?

[Lasse Reichstein Nielsen, 2015-05-19 13:05:07]

https://codereview.chromium.org/1132603003/diff/80001/runtime/lib/typed_data....
File runtime/lib/typed_data.dart (right):

https://codereview.chromium.org/1132603003/diff/80001/runtime/lib/typed_data....
runtime/lib/typed_data.dart:492: assert(false);
It's not *needed* (that's why it's an assert).
It's just stating an assumption - that control never reaches that point, and
stating it a little more strongly than just the comment above.

[Søren Gjesse, 2015-05-27 09:27:45]

lgtm

[Lasse Reichstein Nielsen, 2015-05-28 08:34:09]

Committed patchset #5 (id:80001) manually as
932bcc6901d70492c6f1b8d000b9555a0db62f4b (presubmit successful).

[Lasse Reichstein Nielsen, 2015-05-28 19:33:43]

Updated fingerprints.
PTAL

[srdjan, 2015-05-28 19:51:37]

lgtm

[Lasse Reichstein Nielsen, 2015-05-29 08:50:25]

Committed patchset #6 (id:100001) manually as
cf4eaae5a689132369e8ebfbbe52fd71aa5060e6 (presubmit successful).