Use locale insensitive method to parse double literals.

Use locale insensitive method to parse double literals.

Using strtod does not work because Dart grammar does not necessary match grammar used by strtod. Some locales (e.g. Russian and Danish) use comma instead of dot for radix point.

R=iposva@google.com
BUG=

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

[Vyacheslav Egorov (Google), 2013-04-05 13:45:25]

PTAL

https://codereview.chromium.org/13529014/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/13529014/diff/1/runtime/vm/object.cc#newcode1...
runtime/vm/object.cc:10591: if (!StringToDouble(str.ToCString(), str.Length(),
&double_value)) {
I made it more strict than it was before. All tests still pass.

I am not sure what was the reason to allow white space after string
representation of a double.

[srdjan, 2013-04-05 16:42:02]

DBC
Do you see any performance difference in parsing doubles? Dart and JSON parsers
are probably the main customers of string2double.

https://codereview.chromium.org/13529014/diff/1/runtime/vm/double_conversion.h
File runtime/vm/double_conversion.h (right):

https://codereview.chromium.org/13529014/diff/1/runtime/vm/double_conversion....
runtime/vm/double_conversion.h:18: bool StringToDouble(const char* str, intptr_t
length, double* result);
s/StringToDouble/CStringToDouble/

[Vyacheslav Egorov (Google), 2013-04-05 17:01:05]

I did some pure Dart microbenchmarks but did not see any performance difference.
If there is any --- the overhead of transitioning to runtime, allocation and GC
hides it.

https://codereview.chromium.org/13529014/diff/1/runtime/vm/double_conversion.h
File runtime/vm/double_conversion.h (right):

https://codereview.chromium.org/13529014/diff/1/runtime/vm/double_conversion....
runtime/vm/double_conversion.h:18: bool StringToDouble(const char* str, intptr_t
length, double* result);
On 2013/04/05 16:42:02, srdjan wrote:
> s/StringToDouble/CStringToDouble/

Done.

[Ivan Posva, 2013-04-05 21:56:42]

LGTM with comments.

-Ivan

https://codereview.chromium.org/13529014/diff/1005/runtime/lib/double.cc
File runtime/lib/double.cc (right):

https://codereview.chromium.org/13529014/diff/1005/runtime/lib/double.cc#newc...
runtime/lib/double.cc:240: ASSERT(number_string->IsOneByteString());
Please explain here why you are allowed to make this assertion. It is rather
confusing as above we short-circuit for OneByteString.

https://codereview.chromium.org/13529014/diff/1005/runtime/lib/double.cc#newc...
runtime/lib/double.cc:245: USE(ok);
I would have thought USE is not necessary here.

[Vyacheslav Egorov (Google), 2013-04-08 14:25:03]

Thank you. Comments addressed. Landing.

https://codereview.chromium.org/13529014/diff/1005/runtime/lib/double.cc
File runtime/lib/double.cc (right):

https://codereview.chromium.org/13529014/diff/1005/runtime/lib/double.cc#newc...
runtime/lib/double.cc:240: ASSERT(number_string->IsOneByteString());
On 2013/04/05 21:56:42, Ivan Posva wrote:
> Please explain here why you are allowed to make this assertion. It is rather
> confusing as above we short-circuit for OneByteString.

Done.

https://codereview.chromium.org/13529014/diff/1005/runtime/lib/double.cc#newc...
runtime/lib/double.cc:245: USE(ok);
On 2013/04/05 21:56:42, Ivan Posva wrote:
> I would have thought USE is not necessary here.

Done.

[Vyacheslav Egorov (Google), 2013-04-08 14:32:58]

Committed patchset #2 manually as r21074 (presubmit successful).