Ignore flags with arguments in preparser-process.

preparser/preparser-process.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 249 matching lines @@
     }
   } else if (reader.throws()) {
     const char* message = reader.message();
     fail(data, "Throws unexpectedly with message: %s at location %d-%d\n",
          message, reader.beg_pos(), reader.end_pos());
   }
 }
 
 
 ExceptionExpectation ParseExpectation(int argc, const char* argv[]) {
+  // Parse ["throws" [<exn-type> [<start> [<end>]]]].
   ExceptionExpectation expects;
-
-  // Parse exception expectations from (the remainder of) the command line.
   int arg_index = 0;
-  // Skip any flags.
-  while (argc > arg_index && IsFlag(argv[arg_index])) arg_index++;
+  while (argc > arg_index && strncmp("throws", argv[arg_index], 7))

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

Braces around then-branch.

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

Done.

+    ++arg_index;

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

For consistency with the remaining code, just use postfix increment.

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

Done.

   if (argc > arg_index) {
-    if (strncmp("throws", argv[arg_index], 7)) {
-      // First argument after filename, if present, must be the verbatim
-      // "throws", marking that the preparsing should fail with an exception.
-      fail(NULL, "ERROR: Extra arguments not prefixed by \"throws\".\n");
-    }
     expects.throws = true;
-    do {
-      arg_index++;
-    } while (argc > arg_index && IsFlag(argv[arg_index]));
-    if (argc > arg_index) {
-      // Next argument is the exception type identifier.
-      expects.type = argv[arg_index];
-      do {
-        arg_index++;
-      } while (argc > arg_index && IsFlag(argv[arg_index]));
-      if (argc > arg_index) {
-        expects.beg_pos = atoi(argv[arg_index]);  // NOLINT
-        do {
-          arg_index++;
-        } while (argc > arg_index && IsFlag(argv[arg_index]));
-        if (argc > arg_index) {
-          expects.end_pos = atoi(argv[arg_index]);  // NOLINT
-        }
+    ++arg_index;
+    if (argc == arg_index || IsFlag(argv[arg_index]))

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

Always curly braces around then-branch on multi-line ifs.

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

Done.

+      fail(NULL, "ERROR: exception type must follow 'throws'.\n");

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

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.

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

Yep, didn't notice that it is optional.
I prefer the second approach too.

+    expects.type = argv[arg_index];
+    ++arg_index;
+    if (argc > arg_index && !IsFlag(argv[arg_index])) {
+      expects.beg_pos = atoi(argv[arg_index]);  // NOLINT
+      ++arg_index;
+      if (argc > arg_index && !IsFlag(argv[arg_index])) {
+        expects.end_pos = atoi(argv[arg_index]);  // NOLINT
       }
     }
   }
   return expects;
 }
 
 
 int main(int argc, const char* argv[]) {
   // Parse command line.
   // Format:  preparser (<scriptfile> | -e "<source>")
   //                    ["throws" [<exn-type> [<start> [<end>]]]]
   // Any flags (except an initial -s) are ignored.

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

"-s" should probably be "-e". Mea culpa :)

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

Done.

+  // Flags must not separate "throws" and its arguments.

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

Yes, that's perfectly fine. No good reason to allow it.

 
   // Check for mandatory filename argument.
   int arg_index = 1;
   if (argc <= arg_index) {
     fail(NULL, "ERROR: No filename on command line.\n");
   }
   const uint8_t* source = NULL;
   const char* filename = argv[arg_index];
   if (!strcmp(filename, "-e")) {
     arg_index++;
@@ skipping 48 matching lines @@
   if (data.stack_overflow()) {
     fail(&data, "ERROR: Stack overflow\n");
   }
 
   // Check that the expected exception is thrown, if an exception is
   // expected.
   CheckException(&data, &expects);
 
   return EXIT_SUCCESS;
 }