LGTM
http://codereview.chromium.org/5063003/diff/1/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/5063003/diff/1/src/parser.cc#newcode4671 src/parser.cc:4671: } How about using if ... else if ... else if ... else if ... else ... To indicate that we will never execute more than one? http://codereview.chromium.org/5063003/diff/1/src/prescanner.h File src/prescanner.h (right): http://codereview.chromium.org/5063003/diff/1/src/prescanner.h#newcode72 src/prescanner.h:72: template <typename UTF16Buffer, typename UTF8Buffer> Please explain these template parameters a bit. It is rather nasty to have these all over the place just to declare two member variables of these types. Also they are both named XXXBuffer, but one is for input and the other is for some output and their "interfaces" are quite different. http://codereview.chromium.org/5063003/diff/1/src/prescanner.h#newcode121 src/prescanner.h:121: Location location() const { return current_.location; } Maybe add a separate comment for peek_location. http://codereview.chromium.org/5063003/diff/1/src/prescanner.h#newcode349 src/prescanner.h:349: void Scanner<UTF16Buffer, UTF8Buffer>::AddChar(uc32 c) { AddChar -> AddLiteralChar? http://codereview.chromium.org/5063003/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
