Driver: Implement 'missing argument' error.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@67490 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/DiagnosticDriverKinds.td

@@ -31,6 +31,8 @@ def err_drv_command_failure : Error<
   "unable to execute command: %0">;
 def err_drv_invalid_darwin_version : Error<
   "invalid Darwin version number: %0">;
+def err_drv_missing_argument : Error<
+  "argument to '%0' is missing (expected %1 %plural{1:value|:values}1)">;
 
 def warn_drv_input_file_unused : Warning<
   "%0: '%1' input file unused when '%2' is present">;

include/clang/Driver/ArgList.h

@@ -51,6 +51,9 @@ namespace driver {
     /// The full list of arguments.
     arglist_type Args;
 
+    /// The number of original input argument strings.
+    unsigned NumInputArgStrings;
+
   public:
     ArgList(const char **ArgBegin, const char **ArgEnd);
     ArgList(const ArgList &);
@@ -70,6 +73,10 @@ namespace driver {
     /// getArgString - Return the input argument string at \arg Index.
     const char *getArgString(unsigned Index) const { return ArgStrings[Index]; }
 
+    /// getNumInputArgStrings - Return the number of original input
+    /// argument strings.
+    unsigned getNumInputArgStrings() const { return NumInputArgStrings; }
+
     /// hasArg - Does the arg list contain any option matching \arg Id.
     ///
     /// \arg Claim Whether the argument should be claimed, if it exists.

include/clang/Driver/Option.h

@@ -135,9 +135,11 @@ namespace driver {
 
     /// accept - Potentially accept the current argument, returning a
     /// new Arg instance, or 0 if the option does not accept this
-    /// argument.
+    /// argument (or the argument is missing values).
     ///
-    /// May issue a missing argument error.
+    /// If the option accepts the current argument, accept() sets
+    /// Index to the position where argument parsing should resume
+    /// (even if the argument is missing values).
     virtual Arg *accept(const ArgList &Args, unsigned &Index) const = 0;
     
     void dump() const;

include/clang/Driver/Options.h

@@ -58,12 +58,12 @@ namespace options {
     ///
     /// \param [in] [out] Index - The current parsing position in the
     /// argument string list; on return this will be the index of the
-    /// next option to parse.
+    /// next argument string to parse.
     ///
-    /// \param IndexEnd - The last argument string index to consider
-    /// when parsing.
-    Arg *ParseOneArg(const ArgList &Args, unsigned &Index, 
-                     unsigned IndexEnd) const;
+    /// \return - The parsed argument, or 0 if the argument is missing
+    /// values (in which case Index still points at the conceptual
+    /// next argument string to parse).
+    Arg *ParseOneArg(const ArgList &Args, unsigned &Index) const;
   };
 }
 }

lib/Driver/ArgList.cpp

@@ -13,7 +13,9 @@
 
 using namespace clang::driver;
 
-ArgList::ArgList(const char **ArgBegin, const char **ArgEnd) {
+ArgList::ArgList(const char **ArgBegin, const char **ArgEnd) 
+  : NumInputArgStrings(ArgEnd - ArgBegin) 
+{
   ArgStrings.append(ArgBegin, ArgEnd);
 }
 

lib/Driver/Driver.cpp

@@ -72,18 +72,23 @@ ArgList *Driver::ParseArgStrings(const char **ArgBegin, const char **ArgEnd) {
     }
 
     unsigned Prev = Index;
-    Arg *A = getOpts().ParseOneArg(*Args, Index, End);
-    if (A) {
-      if (A->getOption().isUnsupported()) {
-        Diag(clang::diag::err_drv_unsupported_opt) << A->getAsString(*Args);
-        continue;
-      }
+    Arg *A = getOpts().ParseOneArg(*Args, Index);
+    assert(Index > Prev && "Parser failed to consume argument.");
 
-      Args->append(A);
+    // Check for missing argument error.
+    if (!A) {
+      assert(Index >= End && "Unexpected parser error.");
+      Diag(clang::diag::err_drv_missing_argument)
+        << Args->getArgString(Prev)
+        << (Index - Prev - 1);
+      break;
     }
 
-    assert(Index > Prev && "Parser failed to consume argument.");
-    (void) Prev;
+    if (A->getOption().isUnsupported()) {
+      Diag(clang::diag::err_drv_unsupported_opt) << A->getAsString(*Args);
+      continue;
+    }
+    Args->append(A);
   }
 
   return Args;

lib/Driver/OptTable.cpp

@@ -128,21 +128,28 @@ Option *OptTable::constructOption(options::ID id) const {
   return Opt;
 }
 
-Arg *OptTable::ParseOneArg(const ArgList &Args, unsigned &Index, 
-                           unsigned IndexEnd) const {
+Arg *OptTable::ParseOneArg(const ArgList &Args, unsigned &Index) const {
+  unsigned Prev = Index;
   const char *Str = Args.getArgString(Index);
 
   // Anything that doesn't start with '-' is an input, as is '-' itself.
   if (Str[0] != '-' || Str[1] == '\0')
     return new PositionalArg(getOption(OPT_INPUT), Index++);
 
+  // FIXME: Make this fast, and avoid looking through option
+  // groups. Maybe we should declare them separately?
   for (unsigned j = OPT_UNKNOWN + 1; j < LastOption; ++j) {
     const char *OptName = getOptionName((options::ID) j);
     
     // Arguments are only accepted by options which prefix them.
-    if (memcmp(Str, OptName, strlen(OptName)) == 0)
+    if (memcmp(Str, OptName, strlen(OptName)) == 0) {
       if (Arg *A = getOption((options::ID) j)->accept(Args, Index))
         return A;
+
+      // Otherwise, see if this argument was missing values.
+      if (Prev != Index)
+        return 0;
+    }
   }
 
   return new PositionalArg(getOption(OPT_UNKNOWN), Index++);

lib/Driver/Option.cpp

@@ -108,7 +108,7 @@ OptionGroup::OptionGroup(options::ID ID, const char *Name,
 }
 
 Arg *OptionGroup::accept(const ArgList &Args, unsigned &Index) const {
-  assert(0 && "FIXME");
+  assert(0 && "accept() should never be called on an OptionGroup");
   return 0;
 }
 
@@ -117,7 +117,7 @@ InputOption::InputOption()
 }
 
 Arg *InputOption::accept(const ArgList &Args, unsigned &Index) const {
-  assert(0 && "FIXME");
+  assert(0 && "accept() should never be called on an InputOption");
   return 0;
 }
 
@@ -126,7 +126,7 @@ UnknownOption::UnknownOption()
 }
 
 Arg *UnknownOption::accept(const ArgList &Args, unsigned &Index) const {
-  assert(0 && "FIXME");
+  assert(0 && "accept() should never be called on an UnknownOption");
   return 0;
 }
 
@@ -181,8 +181,10 @@ Arg *SeparateOption::accept(const ArgList &Args, unsigned &Index) const {
   if (strlen(getName()) != strlen(Args.getArgString(Index)))
     return 0;
 
-  // FIXME: Missing argument error.
   Index += 2;
+  if (Index > Args.getNumInputArgStrings())
+    return 0;
+
   return new SeparateArg(this, Index - 2, 1);
 }
 
@@ -199,8 +201,10 @@ Arg *MultiArgOption::accept(const ArgList &Args, unsigned &Index) const {
   if (strlen(getName()) != strlen(Args.getArgString(Index)))
     return 0;
 
-  // FIXME: Missing argument error.
   Index += 1 + NumArgs;
+  if (Index > Args.getNumInputArgStrings())
+    return 0;
+
   return new SeparateArg(this, Index - 1 - NumArgs, NumArgs);
 }
 
@@ -210,15 +214,18 @@ JoinedOrSeparateOption::JoinedOrSeparateOption(options::ID ID, const char *Name,
   : Option(Option::JoinedOrSeparateClass, ID, Name, Group, Alias) {
 }
 
-Arg *JoinedOrSeparateOption::accept(const ArgList &Args, unsigned &Index) const {
+Arg *JoinedOrSeparateOption::accept(const ArgList &Args, 
+                                    unsigned &Index) const {
   // If this is not an exact match, it is a joined arg.
   // FIXME: Avoid strlen.
   if (strlen(getName()) != strlen(Args.getArgString(Index)))
     return new JoinedArg(this, Index++);
 
   // Otherwise it must be separate.
-  // FIXME: Missing argument error.
   Index += 2;
+  if (Index > Args.getNumInputArgStrings())
+    return 0;
+
   return new SeparateArg(this, Index - 2, 1);  
 }
 
@@ -229,11 +236,14 @@ JoinedAndSeparateOption::JoinedAndSeparateOption(options::ID ID,
   : Option(Option::JoinedAndSeparateClass, ID, Name, Group, Alias) {
 }
 
-Arg *JoinedAndSeparateOption::accept(const ArgList &Args, unsigned &Index) const {
+Arg *JoinedAndSeparateOption::accept(const ArgList &Args, 
+                                     unsigned &Index) const {
   // Always matches.
 
-  // FIXME: Missing argument error.
   Index += 2;
+  if (Index > Args.getNumInputArgStrings())
+    return 0;
+
   return new JoinedAndSeparateArg(this, Index - 2);
 }
 

test/Driver/parsing.c

@@ -10,6 +10,11 @@
 // RUN: grep 'Option 8 - Name: "-Xarch_", Values: {"joined", "AndSeparate"}' %t &&
 // RUN: grep 'Option 9 - Name: "-sectalign", Values: {"1", "2", "3"}' %t &&
 
+// RUN: ! clang-driver -V &> %t &&
+// RUN: grep "error: argument to '-V' is missing (expected 1 value)" %t &&
+// RUN: ! clang-driver -sectalign 1 2 &> %t &&
+// RUN: grep "error: argument to '-sectalign' is missing (expected 3 values)" %t &&
+
 // RUN: true