[llvm] r351038 - Add support for prefix-only CLI options
Joel E. Denny via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 22 14:22:53 PST 2019
Hi Thomas, Paul,
On Mon, Jan 14, 2019 at 1:14 PM Thomas Preud'homme via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: thopre
> Date: Mon Jan 14 01:28:53 2019
> New Revision: 351038
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351038&view=rev
> Log:
> Add support for prefix-only CLI options
>
> Summary:
> Add support for options that always prefix their value, giving an error
> if the value is in the next argument or if the option is given a value
> assignment (ie. opt=val). This is the desired behavior for the -D option
> of FileCheck for instance.
>
> Copyright:
> - Linaro (changes in version 2 of revision D55940)
> - GraphCore (changes in later versions and introduced when creating
> D56549)
>
That copyright notice wasn't there when we reviewed. Likewise for D55940.
Was that an accident, or is this something people need to do now
sometimes? I don't know whether it has any legal significance.
Thanks.
Joel
>
> Reviewers: jdenny
>
> Subscribers: llvm-commits, probinson, kristina, hiraditya,
> JonChesterfield
>
> Differential Revision: https://reviews.llvm.org/D56549
>
> Modified:
> llvm/trunk/include/llvm/Support/CommandLine.h
> llvm/trunk/lib/Support/CommandLine.cpp
> llvm/trunk/unittests/Support/CommandLineTest.cpp
>
> Modified: llvm/trunk/include/llvm/Support/CommandLine.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=351038&r1=351037&r2=351038&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/CommandLine.h (original)
> +++ llvm/trunk/include/llvm/Support/CommandLine.h Mon Jan 14 01:28:53 2019
> @@ -156,6 +156,9 @@ enum OptionHidden { // Control whether
> // 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 @@ enum FormattingFlags {
> 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 @@ class Option {
> // 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.
>
> Modified: llvm/trunk/lib/Support/CommandLine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=351038&r1=351037&r2=351038&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
> +++ llvm/trunk/lib/Support/CommandLine.cpp Mon Jan 14 01:28:53 2019
> @@ -426,12 +426,17 @@ Option *CommandLineParser::LookupOption(
> 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
> allows
> + // non-prefix form (ie is not AlwaysPrefix), we match. If not, signal
> match
> + // failure by returning nullptr.
> 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 +544,9 @@ static inline bool ProvideOption(Option
> switch (Handler->getValueExpectedFlag()) {
> case ValueRequired:
> if (!Value.data()) { // No value specified?
> - if (i + 1 >= argc)
> + // If no other argument or the option only supports prefix form, we
> + // cannot look at the 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 +604,8 @@ static inline bool isGrouping(const Opti
> 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 +655,8 @@ HandlePrefixedOrGroupedOption(StringRef
> // 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);
>
> Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=351038&r1=351037&r2=351038&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
> +++ llvm/trunk/unittests/Support/CommandLineTest.cpp Mon Jan 14 01:28:53
> 2019
> @@ -840,4 +840,78 @@ TEST(CommandLineTest, GetCommandLineArgu
> }
> #endif
>
> -} // anonymous namespace
> +TEST(CommandLineTest, PrefixOptions) {
> + cl::ResetCommandLineParser();
> +
> + StackOption<std::string, cl::list<std::string>> 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<std::string, cl::list<std::string>> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190122/449f421d/attachment.html>
More information about the llvm-commits
mailing list