Change restrictions on start/end on String.fromCharCodes.

Change restrictions on start/end on String.fromCharCodes.

Must now be 0 <= start <= end <= iterable.length.

R=sgjesse@google.com

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

[Lasse Reichstein Nielsen, 2014-09-26 10:42:10]

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

[Lasse Reichstein Nielsen, 2014-09-26 10:42:11]

By popular demand.

[Søren Gjesse, 2014-09-26 11:43:00]

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

https://codereview.chromium.org/605213002/diff/1/runtime/lib/string_patch.dar...
runtime/lib/string_patch.dart:69: charCodes = new List.from(charCodes, growable:
false);
If the iterable is long (maybe even infinite) then this will create a long
list/go OOM even if only a small portion in the begining is requested.

Use skip/take pattern as in the dart2js code.

https://codereview.chromium.org/605213002/diff/1/sdk/lib/_internal/compiler/j...
File sdk/lib/_internal/compiler/js_lib/core_patch.dart (right):

https://codereview.chromium.org/605213002/diff/1/sdk/lib/_internal/compiler/j...
sdk/lib/_internal/compiler/js_lib/core_patch.dart:280: if (end < start) throw
new RangeError.value(end);
As you mentioned yourself end can be null here.

https://codereview.chromium.org/605213002/diff/1/sdk/lib/_internal/compiler/j...
sdk/lib/_internal/compiler/js_lib/core_patch.dart:282: list = new
List.from(charCodes.take(end).skip(start));
skip before take?

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

https://codereview.chromium.org/605213002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:112: * `0 <= start <= end <= charCodes.length`.
There might no be any charCodes.length.

[Lasse Reichstein Nielsen, 2014-09-29 07:49:28]

Update, ptal

[Lasse Reichstein Nielsen, 2014-09-29 07:51:07]

lrn@google.com changed reviewers:
+ fschneider@google.com

[Lasse Reichstein Nielsen, 2014-09-29 07:51:08]

https://codereview.chromium.org/605213002/diff/20001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (left):

https://codereview.chromium.org/605213002/diff/20001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:67: if ((charCodes is Uint8List) || (charCodes is
Int8List)) {
Negative values in an Int8List are not caught, so
  new String.fromCharCodes(new Int8List(1)..[0] = -1).codeUnitAt(0)
returns 255.

[Søren Gjesse, 2014-09-29 07:54:41]

lgtm

https://codereview.chromium.org/605213002/diff/20001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/605213002/diff/20001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:311: return (codeUnit == 0x85) || (codeUnit ==
0xA0);
Keep the NEL, NBSP comment.

https://codereview.chromium.org/605213002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/js_lib/core_patch.dart (right):

https://codereview.chromium.org/605213002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/core_patch.dart:308: static String
_stringFromIterable(Iterable<int> charCodes, int start, int end) {
Long line.

[Lasse Reichstein Nielsen, 2014-09-29 08:15:56]

Committed patchset #3 (id:40001) manually as 40755 (presubmit successful).

[Lasse Reichstein Nielsen, 2014-09-29 08:16:19]

https://codereview.chromium.org/605213002/diff/20001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/605213002/diff/20001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:311: return (codeUnit == 0x85) || (codeUnit ==
0xA0);
On 2014/09/29 07:54:40, Søren Gjesse wrote:
> Keep the NEL, NBSP comment.

Done.

https://codereview.chromium.org/605213002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/js_lib/core_patch.dart (right):

https://codereview.chromium.org/605213002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/core_patch.dart:308: static String
_stringFromIterable(Iterable<int> charCodes, int start, int end) {
On 2014/09/29 07:54:40, Søren Gjesse wrote:
> Long line.

Done.