Dart version of double.parse that works for numbers that can be represented

Dart version of double.parse that works for numbers that can be represented exactly as doubles.

BUG= http://dartbug.com/17711
R=vegorov@google.com

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

[Lasse Reichstein Nielsen, 2014-09-29 14:31:47]

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

[floitsch, 2014-09-29 14:51:33]

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

[floitsch, 2014-09-29 14:51:34]

DBC.

https://codereview.chromium.org/520693004/diff/140001/runtime/lib/double_patc...
File runtime/lib/double_patch.dart (right):

https://codereview.chromium.org/520693004/diff/140001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:112: if (length < 16) {
Given that exponents don't count in the limit, you could try for greater lengths
(16.0digits_e22) -> 16 digits + 1 dot + e + 2 digits.
(you could also have one character for the plus in the exponent "e+22", but
that's probably too rare.

[Lasse Reichstein Nielsen, 2014-09-30 05:45:30]

https://codereview.chromium.org/520693004/diff/140001/runtime/lib/double_patc...
File runtime/lib/double_patch.dart (right):

https://codereview.chromium.org/520693004/diff/140001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:112: if (length < 16) {
Ack, I forgot the limit was even there. I have a value limit in the function
instead, so when the value exceeds 2^53, it bails out. This test should be
removable.

[Lasse Reichstein Nielsen, 2014-10-01 12:02:14]

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

[Lasse Reichstein Nielsen, 2014-10-01 12:02:14]

+vegorov, -fschneider

[Vyacheslav Egorov (Google), 2014-10-02 14:27:41]

lgtm

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
File runtime/lib/double_patch.dart (right):

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:24: int digitsSeen = 0;
please change this to boolean

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:43: if (end == start + 3 && str.codeUnitAt(start +
1) == _a
1 line &&
1 line &&
1 line

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:61: if (doubleValue >= 9007199254740992.0) return
null;  // Maybe unprecise.
I would make this a const with a good name.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:64: } else if ((c == _DOT) && (exponentDelta ==
0)) {
there is an inconsistency when you parenthesize ==-comparisons and when you
don't. please either always write (a == b) && (c == d) or a == b && c == d.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:69: int expPart = int._tryParseSmi(str, i, end -
1);
I find this end - 1 confusing. Is it because our parsing functions being
inconsistent with end being inclusive/exclusive?

Can we settle on end being either inclusive or exclusive everywhere? At least
add a comment here.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:79: // Powers of 10 up to 10^22 are exact doubles.
It would be good to use something more elaborate instead of 'exact' it's a bit
confusing.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:106: if (exponent < -22) return null;
maybe use P10.length here instead of 22?

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:109: if (exponent > 22) return null;
Ditto

[Lasse Reichstein Nielsen, 2014-10-02 14:36:44]

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
File runtime/lib/double_patch.dart (right):

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:24: int digitsSeen = 0;
Done. Please optimize booleans :)

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:43: if (end == start + 3 && str.codeUnitAt(start +
1) == _a
On 2014/10/02 14:27:40, Vyacheslav Egorov (Google) wrote:
> 1 line &&
> 1 line &&
> 1 line

Done.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:61: if (doubleValue >= 9007199254740992.0) return
null;  // Maybe unprecise.
On 2014/10/02 14:27:41, Vyacheslav Egorov (Google) wrote:
> I would make this a const with a good name.

Done.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:64: } else if ((c == _DOT) && (exponentDelta ==
0)) {
Done, without the parentheses.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:69: int expPart = int._tryParseSmi(str, i, end -
1);
The _tryParseSmi is unconventional in having end inclusive (it made sense at the
time, because we calculate the "last non-whitespace index", so we can use that
as argument directly on the fast path. 

We should probably clean that up, now that I use it from elsewhere too.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:79: // Powers of 10 up to 10^22 are exact doubles.
Added comment.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:106: if (exponent < -22) return null;
That does appear to be just as fast. Done.

https://codereview.chromium.org/520693004/diff/260001/runtime/lib/double_patc...
runtime/lib/double_patch.dart:109: if (exponent > 22) return null;
On 2014/10/02 14:27:41, Vyacheslav Egorov (Google) wrote:
> Ditto

Done.

[Lasse Reichstein Nielsen, 2014-10-03 06:01:58]

Committed patchset #19 manually as r40891 (presubmit successful).