Addressed comments, please take another look.
http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc File preparser/preparser-process.cc (right): http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#newcode273 preparser/preparser-process.cc:273: while (argc > arg_index && strncmp("throws", argv[arg_index], 7)) On 2011/10/13 13:08:14, Lasse Reichstein wrote:
Braces around then-branch.
Done. http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#newcode274 preparser/preparser-process.cc:274: ++arg_index; On 2011/10/13 13:08:14, Lasse Reichstein wrote:
For consistency with the remaining code, just use postfix increment.
Done. http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#newcode278 preparser/preparser-process.cc:278: if (argc == arg_index || IsFlag(argv[arg_index])) On 2011/10/13 13:08:14, Lasse Reichstein wrote:
Always curly braces around then-branch on multi-line ifs.
Done. http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#newcode279 preparser/preparser-process.cc:279: fail(NULL, "ERROR: exception type must follow 'throws'.\n"); On 2011/10/13 13:08:14, Lasse Reichstein wrote:
This changes the command line to require the exception type, where
before it was
optional. If making this change (it's ok, makes for better tests, and we might
already
always pass that information anyway), update the comment above that
shows the
command line format (i.e., no "[" before "<exn-type>" - and matchine
"]" at
end). Also, update the test/preparser/testcfg.py to ensure that the
exception type is
required by the pyt file macros.
Alternatively, just require break command line parsing at this point,
without
failing, if the next argument is a flag. I think I prefer that
approach. Yep, didn't notice that it is optional. I prefer the second approach too. http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#newcode298 preparser/preparser-process.cc:298: // Any flags (except an initial -s) are ignored. On 2011/10/13 13:08:14, Lasse Reichstein wrote:
"-s" should probably be "-e". Mea culpa :)
Done. http://codereview.chromium.org/8268004/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
