Index: llvm/include/llvm/Support/CommandLine.h =================================================================== --- llvm/include/llvm/Support/CommandLine.h +++ llvm/include/llvm/Support/CommandLine.h @@ -156,6 +156,9 @@ // enabled, and used, the value for the flag comes from the suffix of the // argument. // +// AlwaysPrefix - Only allow the behavior enabled by the Prefix flag and reject +// the Option=Value form. +// // Grouping - With this option enabled, multiple letter options are allowed to // bunch together with only a single hyphen for the whole group. This allows // emulation of the behavior that ls uses for example: ls -la === ls -l -a @@ -165,7 +168,8 @@ NormalFormatting = 0x00, // Nothing special Positional = 0x01, // Is a positional argument, no '-' required Prefix = 0x02, // Can this option directly prefix its value? - Grouping = 0x03 // Can this option group with other options? + AlwaysPrefix = 0x03, // Can this option only directly prefix its value? + Grouping = 0x04 // Can this option group with other options? }; enum MiscFlags { // Miscellaneous flags to adjust argument @@ -265,7 +269,7 @@ // detail representing the non-value unsigned Value : 2; unsigned HiddenFlag : 2; // enum OptionHidden - unsigned Formatting : 2; // enum FormattingFlags + unsigned Formatting : 3; // enum FormattingFlags unsigned Misc : 3; unsigned Position = 0; // Position of last occurrence of the option unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option. Index: llvm/lib/Support/CommandLine.cpp =================================================================== --- llvm/lib/Support/CommandLine.cpp +++ llvm/lib/Support/CommandLine.cpp @@ -426,12 +426,16 @@ return I != Sub.OptionsMap.end() ? I->second : nullptr; } - // If the argument before the = is a valid option name, we match. If not, - // return Arg unmolested. + // If the argument before the = is a valid option name and the option does + // support only the prefix form, we match. If not, return Arg unmolested. auto I = Sub.OptionsMap.find(Arg.substr(0, EqualPos)); if (I == Sub.OptionsMap.end()) return nullptr; + auto O = I->second; + if (O->getFormattingFlag() == cl::AlwaysPrefix) + return nullptr; + Value = Arg.substr(EqualPos + 1); Arg = Arg.substr(0, EqualPos); return I->second; @@ -539,7 +543,9 @@ switch (Handler->getValueExpectedFlag()) { case ValueRequired: if (!Value.data()) { // No value specified? - if (i + 1 >= argc) + // If no other argument or option only support prefix form, we cannot + // look at next argument. + if (i + 1 >= argc || Handler->getFormattingFlag() == cl::AlwaysPrefix) return Handler->error("requires a value!"); // Steal the next argument, like for '-o filename' assert(argv && "null check"); @@ -597,7 +603,8 @@ return O->getFormattingFlag() == cl::Grouping; } static inline bool isPrefixedOrGrouping(const Option *O) { - return isGrouping(O) || O->getFormattingFlag() == cl::Prefix; + return isGrouping(O) || O->getFormattingFlag() == cl::Prefix || + O->getFormattingFlag() == cl::AlwaysPrefix; } // getOptionPred - Check to see if there are any options that satisfy the @@ -647,7 +654,8 @@ // If the option is a prefixed option, then the value is simply the // rest of the name... so fall through to later processing, by // setting up the argument name flags and value fields. - if (PGOpt->getFormattingFlag() == cl::Prefix) { + if (PGOpt->getFormattingFlag() == cl::Prefix || + PGOpt->getFormattingFlag() == cl::AlwaysPrefix) { Value = Arg.substr(Length); Arg = Arg.substr(0, Length); assert(OptionsMap.count(Arg) && OptionsMap.find(Arg)->second == PGOpt); Index: llvm/test/FileCheck/defines.txt =================================================================== --- llvm/test/FileCheck/defines.txt +++ llvm/test/FileCheck/defines.txt @@ -3,16 +3,32 @@ ; ; RUN: not FileCheck -DVALUE=10 -check-prefix NOT -input-file %s %s 2>&1 | FileCheck %s -check-prefix NOT-ERRMSG ; RUN: FileCheck -DVALUE=20 -check-prefix NOT -input-file %s %s +; RUN: not FileCheck -DVALUE10 -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIEQ1 +; RUN: not FileCheck -D -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIEQ2 +; RUN: not FileCheck -D=10 -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIVAR1 +; RUN: not FileCheck -D= -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIVAR2 +; RUN: FileCheck -DVALUE= -check-prefix EMPTY -input-file %s %s 2>&1 Value = 10 ; CHECK: Value = [[VALUE]] ; NOT-NOT: Value = [[VALUE]] -; ERRMSG: defines.txt:8:10: error: CHECK: expected string not found in input +; ERRMSG: defines.txt:[[@LINE-3]]:10: error: CHECK: expected string not found in input ; ERRMSG: defines.txt:1:1: note: scanning from here ; ERRMSG: defines.txt:1:1: note: with variable "VALUE" equal to "20" -; ERRMSG: defines.txt:7:1: note: possible intended match here +; ERRMSG: defines.txt:[[@LINE-7]]:1: note: possible intended match here -; NOT-ERRMSG: defines.txt:9:12: error: {{NOT}}-NOT: excluded string found in input -; NOT-ERRMSG: defines.txt:7:1: note: found here -; NOT-ERRMSG: defines.txt:7:1: note: with variable "VALUE" equal to "10" \ No newline at end of file +; NOT-ERRMSG: defines.txt:[[@LINE-7]]:12: error: {{NOT}}-NOT: excluded string found in input +; NOT-ERRMSG: defines.txt:[[@LINE-10]]:1: note: found here +; NOT-ERRMSG: defines.txt:[[@LINE-11]]:1: note: with variable "VALUE" equal to "10" + +; ERRCLIEQ1: Missing equal sign in command-line definition '-DVALUE10' + +; ERRCLIEQ2: FileCheck: for the -D option: requires a value! + +; ERRCLIVAR1: Missing pattern variable name in command-line definition '-D=10' + +; ERRCLIVAR2: Missing pattern variable name in command-line definition '-D=' + +Empty value = @@ +; EMPTY: Empty value = @[[VALUE]]@ Index: llvm/unittests/Support/CommandLineTest.cpp =================================================================== --- llvm/unittests/Support/CommandLineTest.cpp +++ llvm/unittests/Support/CommandLineTest.cpp @@ -840,4 +840,79 @@ } #endif +TEST(CommandLineTest, PrefixOptions) { + cl::ResetCommandLineParser(); + + StackOption> IncludeDirs( + "I", cl::Prefix, cl::desc("Declare an include directory")); + + // Test non-prefixed variant works with cl::Prefix options. + EXPECT_TRUE(IncludeDirs.empty()); + const char *args[] = {"prog", "-I=/usr/include"}; + EXPECT_TRUE( + cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls())); + EXPECT_TRUE(IncludeDirs.size() == 1); + EXPECT_TRUE(IncludeDirs.front().compare("/usr/include") == 0); + + IncludeDirs.erase(IncludeDirs.begin()); + cl::ResetAllOptionOccurrences(); + + // Test non-prefixed variant works with cl::Prefix options when value is + // passed in following argument. + EXPECT_TRUE(IncludeDirs.empty()); + const char *args2[] = {"prog", "-I", "/usr/include"}; + EXPECT_TRUE( + cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls())); + EXPECT_TRUE(IncludeDirs.size() == 1); + EXPECT_TRUE(IncludeDirs.front().compare("/usr/include") == 0); + + IncludeDirs.erase(IncludeDirs.begin()); + cl::ResetAllOptionOccurrences(); + + // Test prefixed variant works with cl::Prefix options. + EXPECT_TRUE(IncludeDirs.empty()); + const char *args3[] = {"prog", "-I/usr/include"}; + EXPECT_TRUE( + cl::ParseCommandLineOptions(2, args3, StringRef(), &llvm::nulls())); + EXPECT_TRUE(IncludeDirs.size() == 1); + EXPECT_TRUE(IncludeDirs.front().compare("/usr/include") == 0); + + + StackOption> MacroDefs( + "D", cl::AlwaysPrefix, cl::desc("Define a macro"), + cl::value_desc("MACRO[=VALUE]")); + + cl::ResetAllOptionOccurrences(); + + // Test non-prefixed variant does not work with cl::AlwaysPrefix options: + // equal sign is part of the value. + EXPECT_TRUE(MacroDefs.empty()); + const char *args4[] = {"prog", "-D=HAVE_FOO"}; + EXPECT_TRUE( + cl::ParseCommandLineOptions(2, args4, StringRef(), &llvm::nulls())); + EXPECT_TRUE(MacroDefs.size() == 1); + EXPECT_TRUE(MacroDefs.front().compare("=HAVE_FOO") == 0); + + MacroDefs.erase(MacroDefs.begin()); + cl::ResetAllOptionOccurrences(); + + // Test non-prefixed variant does not allow value to be passed in following + // argument with cl::AlwaysPrefix options. + EXPECT_TRUE(MacroDefs.empty()); + const char *args5[] = {"prog", "-D", "HAVE_FOO"}; + EXPECT_FALSE( + cl::ParseCommandLineOptions(3, args5, StringRef(), &llvm::nulls())); + EXPECT_TRUE(MacroDefs.empty()); + + cl::ResetAllOptionOccurrences(); + + // Test prefixed variant works with cl::AlwaysPrefix options. + EXPECT_TRUE(MacroDefs.empty()); + const char *args6[] = {"prog", "-DHAVE_FOO"}; + EXPECT_TRUE( + cl::ParseCommandLineOptions(2, args6, StringRef(), &llvm::nulls())); + EXPECT_TRUE(MacroDefs.size() == 1); + EXPECT_TRUE(MacroDefs.front().compare("HAVE_FOO") == 0); +} + } // anonymous namespace Index: llvm/utils/FileCheck/FileCheck.cpp =================================================================== --- llvm/utils/FileCheck/FileCheck.cpp +++ llvm/utils/FileCheck/FileCheck.cpp @@ -51,9 +51,10 @@ "this pattern occur which are not matched by a positive pattern"), cl::value_desc("pattern")); -static cl::list GlobalDefines("D", cl::Prefix, - cl::desc("Define a variable to be used in capture patterns."), - cl::value_desc("VAR=VALUE")); +static cl::list + GlobalDefines("D", cl::AlwaysPrefix, + cl::desc("Define a variable to be used in capture patterns."), + cl::value_desc("VAR=VALUE")); static cl::opt AllowEmptyInput( "allow-empty", cl::init(false), @@ -523,8 +524,24 @@ for (auto CheckNot : ImplicitCheckNot) Req.ImplicitCheckNot.push_back(CheckNot); - for (auto G : GlobalDefines) + bool GlobalDefineError = false; + for (auto G : GlobalDefines) { + size_t EqIdx = G.find('='); + if (EqIdx == std::string::npos) { + errs() << "Missing equal sign in command-line definition '-D" << G << "'\n"; + GlobalDefineError = true; + continue; + } + if (EqIdx == 0) { + errs() << "Missing pattern variable name in command-line definition '-D" + << G << "'\n"; + GlobalDefineError = true; + continue; + } Req.GlobalDefines.push_back(G); + } + if (GlobalDefineError) + return 2; Req.AllowEmptyInput = AllowEmptyInput; Req.EnableVarScope = EnableVarScope;