[v8-dev] Re: StringToDouble rewritten not using String::Get and memory allocations.... (issue1096002)

[serya]

Buffer size enlarged to 772. Added code checking if all of the dropped  
digits
are zeros and appenging '1' (or '.1') if not. Added a test for 1000-digit
number.

http://codereview.chromium.org/1096002/diff/21004/27003
File src/conversions.cc (right):

http://codereview.chromium.org/1096002/diff/21004/27003#newcode123
src/conversions.cc:123: int StringInputBufferIterator::operator * ()
const {
On 2010/03/23 12:34:54, Florian Loitsch wrote:
Convention v8 seems to be to put the attach the operator: operator*()

Done.

http://codereview.chromium.org/1096002/diff/21004/27003#newcode298
src/conversions.cc:298: if (*current == '+') {
On 2010/03/20 16:56:51, Florian Loitsch wrote:
Are we sure there is at least one character?
If yes assert it.
If not, and it is legal to access current[0] of empty string, explain.

If string is empty this line wouldn't be reached (because of 'if
(!AdvanceToNonspace(&current, end)) return empty_string_val;').

Added explanation as a comment.

http://codereview.chromium.org/1096002/diff/21004/27003#newcode330
src/conversions.cc:330: if ((flags & ALLOW_HEX) && (*current == 'x' ||
*current == 'X')) {
On 2010/03/20 16:56:51, Florian Loitsch wrote:
It might make sense to move the hexadecimal reading into a separate
function.

Done.

http://codereview.chromium.org/1096002/diff/21004/27003#newcode405
src/conversions.cc:405: if (significant_digits == 0) {
On 2010/03/20 16:56:51, Florian Loitsch wrote:
I think it makes more sense to structure as follows:
if (current == end) {
   if (significant_digits == 0 && !leading_zero) {
     // String was ".".
     return JUNK_STRING_VALUE;
   } else {
     goto parsing_done;
   }
}
if (significant_digits == 0) {
   octal = false;
   ...
}

Done.

http://codereview.chromium.org/1096002/diff/21004/27003#newcode451
src/conversions.cc:451: if (current == end) return JUNK_STRING_VALUE;
On 2010/03/20 16:56:51, Florian Loitsch wrote:
How should "123e" be parsed when "trailing junk is enabled?
as "123" or as JUNK_STRING_VALUE?
If it's the latter, then this is fine.

Fixed. Test added.

http://codereview.chromium.org/1096002/diff/21004/27003#newcode456
src/conversions.cc:456: if (current == end) return JUNK_STRING_VALUE;
On 2010/03/20 16:56:51, Florian Loitsch wrote:
As before: should 123e+ be parsed as 123 or JUNK_STRING_VALUE when
trailing junk
is enabled.

Fixed. Test added.

http://codereview.chromium.org/1096002

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply 
to this email with the words "REMOVE ME" as the subject.