I rewrote things and addressed all but one of your comments.
Please have another look.
On 2011/06/24 13:56:00, Lasse Reichstein wrote:
No capes^H^H^H^H^Hgotos!
no gotos anymore.
The logic is too complex, with the same test being used to both exit the
loop
and later assume that it wasn't a clean exit.
complex logic is gone.
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h
File src/json-parser.h (right):
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode418
src/json-parser.h:418: int length = kInitialSpecialStringSize;
Change ...StringSize to ...StringLength. Size sounds like it's the number
of
bytes, not the number of characters.
done
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode420
src/json-parser.h:420: isolate()->factory()->NewRawTwoByteString(length,
NOT_TENURED);
If NewRawTwoByteString doesn't return a Handle<SeqTwoByteString>, you
should
change it to do so.
done (by someone else?)
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode504
src/json-parser.h:504: Handle<String>
JsonParser<seq_ascii>::SlowScanJsonAsciiString(
Sounds like it scans an ASCII string, not that it creates one.
How about SlowScanJsonIntoAsciiString?
(Otherwise I start questioning why we don't know that c0_ is ASCII inside
the
loop).
Both the Ascii and TwoByte case are now handled in one template.
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode571
src/json-parser.h:571: goto outer_loop; // break out of while loop.
I'm really not too keen on using gotos. Really not.
But the style guide doesn't say it's not allowed, so I guess I can't
prevent
it.
goto is gone.
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode587
src/json-parser.h:587: int string_size = SeqAsciiString::SizeFor(count);
If the truncation would turn a lot of space into filler, why not reuse it
for
the two-byte string?
I think this branch is mostly handling cases where we're allocated in large
object space in which case there is not much to do anyway.
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode593
src/json-parser.h:593: }
If you move the shrinking to a separate function, you can call it from
where
you
do the goto, and call directly to the two-byte scan, instead of this
torturous
logic.
Torturous logic gone.
http://codereview.chromium.org/7241023/diff/3001/src/json-parser.h#newcode618
src/json-parser.h:618: break;
Again, you can abstract the part below into a function, so you can call it
both
here and after the loop, and then call and return the result of
SlowScanJsonAsciiString from here only.
(sort of) done.
http://codereview.chromium.org/7241023/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
