Ignore flags with arguments in preparser-process.

Ignore flags with arguments in preparser-process.

Currently, preparser-process crashes when given flags with arguments (e.g. --gc_interval 10). It can be fixed by ignoring everything in the command line except the "throws" command and its immediate arguments. This assumes that no flags appear between "throws" and its arguments.

TEST=make ia32.release.check TESTFLAGS="preparser --special-command=\"@ --gc_interval 10\""

Committed: http://code.google.com/p/v8/source/detail?r=9617

[ulan, 2011-10-13 12:15:43]

Please take a look.

[Lasse Reichstein, 2011-10-13 13:08:14]

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#...
preparser/preparser-process.cc:273: while (argc > arg_index && strncmp("throws",
argv[arg_index], 7))
Braces around then-branch.

http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#...
preparser/preparser-process.cc:274: ++arg_index;
For consistency with the remaining code, just use postfix increment.

http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#...
preparser/preparser-process.cc:278: if (argc == arg_index ||
IsFlag(argv[arg_index]))
Always curly braces around then-branch on multi-line ifs.

http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#...
preparser/preparser-process.cc:279: fail(NULL, "ERROR: exception type must
follow 'throws'.\n");
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.

http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#...
preparser/preparser-process.cc:298: // Any flags (except an initial -s) are
ignored.
"-s" should probably be "-e". Mea culpa :)

http://codereview.chromium.org/8268004/diff/1/preparser/preparser-process.cc#...
preparser/preparser-process.cc:299: // Flags must not separate "throws" and its
arguments.
Yes, that's perfectly fine. No good reason to allow it.

[ulan, 2011-10-13 13:43:18]

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#...
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#...
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#...
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#...
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#...
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.

[Lasse Reichstein, 2011-10-13 13:45:02]

lgtm