diff --git a/include/clang/Basic/DiagnosticDriverKinds.td b/include/clang/Basic/DiagnosticDriverKinds.td index db64b0cb3d..e8cfa371ee 100644 --- a/include/clang/Basic/DiagnosticDriverKinds.td +++ b/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">; diff --git a/include/clang/Driver/ArgList.h b/include/clang/Driver/ArgList.h index 3fbeec7f90..cf67295241 100644 --- a/include/clang/Driver/ArgList.h +++ b/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. diff --git a/include/clang/Driver/Option.h b/include/clang/Driver/Option.h index 271585f0aa..57e15cb00f 100644 --- a/include/clang/Driver/Option.h +++ b/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; diff --git a/include/clang/Driver/Options.h b/include/clang/Driver/Options.h index 9690d3afb3..1268c30818 100644 --- a/include/clang/Driver/Options.h +++ b/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; }; } } diff --git a/lib/Driver/ArgList.cpp b/lib/Driver/ArgList.cpp index 30da136e04..7653daf7ee 100644 --- a/lib/Driver/ArgList.cpp +++ b/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); } diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index 40c6e50063..db084262e9 100644 --- a/lib/Driver/Driver.cpp +++ b/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; diff --git a/lib/Driver/OptTable.cpp b/lib/Driver/OptTable.cpp index 7b7e2a7bbc..07e3511719 100644 --- a/lib/Driver/OptTable.cpp +++ b/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++); diff --git a/lib/Driver/Option.cpp b/lib/Driver/Option.cpp index 12d501b3cb..dc681c7295 100644 --- a/lib/Driver/Option.cpp +++ b/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); } diff --git a/test/Driver/parsing.c b/test/Driver/parsing.c index 3171cb7395..c94af8e213 100644 --- a/test/Driver/parsing.c +++ b/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