http://codereview.chromium.org/4112012/diff/3001/4001 File src/parser.cc (right):
http://codereview.chromium.org/4112012/diff/3001/4001#newcode47 src/parser.cc:47: #include "preparser.h" Can you all this after platform.h to alphabetize? http://codereview.chromium.org/4112012/diff/3001/4001#newcode4937 src/parser.cc:4937: class LogAdapter : public ParserLog { Why do you need to adapt instead of just changing the interface? http://codereview.chromium.org/4112012/diff/3001/4001#newcode4945 src/parser.cc:4945: void LogMessage(int start, blank line before LogMessage? http://codereview.chromium.org/4112012/diff/3001/4002 File src/parser.h (right): http://codereview.chromium.org/4112012/diff/3001/4002#newcode184 src/parser.h:184: // Records the occurrence of a function. Only two of the many methods here have comments. Either add comments to all of them or don't comment on any of them? http://codereview.chromium.org/4112012/diff/3001/4002#newcode188 src/parser.h:188: virtual void LogError() { } Am I missing an implementation of this? I don't see this implemented anywhere, shouldn't it be? http://codereview.chromium.org/4112012/diff/3001/4002#newcode204 src/parser.h:204: PartialParserRecorder(); Since you have spaces between the rest of the functions, should you have a blank line after the constructor? http://codereview.chromium.org/4112012/diff/3001/4002#newcode231 src/parser.h:231: bool is_recording() { blank line before? http://codereview.chromium.org/4112012/diff/3001/4002#newcode243 src/parser.h:243: int prev_start; Missing underscore: prev_start_ Where is this member used? http://codereview.chromium.org/4112012/diff/3001/4003 File src/preparser.h (right): http://codereview.chromium.org/4112012/diff/3001/4003#newcode75 src/preparser.h:75: Excessive spacing. http://codereview.chromium.org/4112012/diff/3001/4003#newcode80 src/preparser.h:80: class PreParser { Do we have tests that are good enough to catch if the real parser and the pre-parser do not evolve in the same way? http://codereview.chromium.org/4112012/diff/3001/4003#newcode253 src/preparser.h:253: template <typename S, typename L> S -> Scanner L -> Logger To improve readability? Also in the rest of the file. http://codereview.chromium.org/4112012/diff/3001/4004 File test/cctest/test-parsing.cc (right): http://codereview.chromium.org/4112012/diff/3001/4004#newcode267 test/cctest/test-parsing.cc:267: void LogMessage(int start, int end, const char* type, const char* arg_opt) { Blank line before LogMessage? http://codereview.chromium.org/4112012/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
