[llvm] r352310 - Revert "Add support for prefix-only CLI options"
Thomas Preud'homme via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 27 01:11:20 PST 2019
Hi Ronan,
On 27/01/2019, 09:05, "Roman Lebedev" <lebedev.ri at gmail.com> wrote:
On Sun, Jan 27, 2019 at 12:02 PM Thomas Preud'homme via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: thopre
> Date: Sun Jan 27 01:02:46 2019
> New Revision: 352310
>
> URL: http://llvm.org/viewvc/llvm-project?rev=352310&view=rev
> Log:
> Revert "Add support for prefix-only CLI options"
>
> This reverts commit r351038.
It is usually a good idea to not just state *what* happens, but *why*.
Fair enough. Will keep this in mind in the future. Revert happened due to an ongoing discussion about the copyright that applies to these patches.
Best regards,
Thomas
> 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=352310&r1=352309&r2=352310&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/CommandLine.h (original)
> +++ llvm/trunk/include/llvm/Support/CommandLine.h Sun Jan 27 01:02:46 2019
> @@ -155,9 +155,6 @@ 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
> @@ -167,8 +164,7 @@ enum FormattingFlags {
> NormalFormatting = 0x00, // Nothing special
> Positional = 0x01, // Is a positional argument, no '-' required
> Prefix = 0x02, // Can this option directly prefix its value?
> - AlwaysPrefix = 0x03, // Can this option only directly prefix its value?
> - Grouping = 0x04 // Can this option group with other options?
> + Grouping = 0x03 // Can this option group with other options?
> };
>
> enum MiscFlags { // Miscellaneous flags to adjust argument
> @@ -268,7 +264,7 @@ class Option {
> // detail representing the non-value
> unsigned Value : 2;
> unsigned HiddenFlag : 2; // enum OptionHidden
> - unsigned Formatting : 3; // enum FormattingFlags
> + unsigned Formatting : 2; // 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=352310&r1=352309&r2=352310&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
> +++ llvm/trunk/lib/Support/CommandLine.cpp Sun Jan 27 01:02:46 2019
> @@ -425,17 +425,12 @@ Option *CommandLineParser::LookupOption(
> return I != Sub.OptionsMap.end() ? I->second : nullptr;
> }
>
> - // 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.
> + // If the argument before the = is a valid option name, 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;
> @@ -543,9 +538,7 @@ static inline bool ProvideOption(Option
> switch (Handler->getValueExpectedFlag()) {
> case ValueRequired:
> if (!Value.data()) { // No value specified?
> - // 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)
> + if (i + 1 >= argc)
> return Handler->error("requires a value!");
> // Steal the next argument, like for '-o filename'
> assert(argv && "null check");
> @@ -603,8 +596,7 @@ 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 ||
> - O->getFormattingFlag() == cl::AlwaysPrefix;
> + return isGrouping(O) || O->getFormattingFlag() == cl::Prefix;
> }
>
> // getOptionPred - Check to see if there are any options that satisfy the
> @@ -654,8 +646,7 @@ 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 ||
> - PGOpt->getFormattingFlag() == cl::AlwaysPrefix) {
> + if (PGOpt->getFormattingFlag() == cl::Prefix) {
> 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=352310&r1=352309&r2=352310&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
> +++ llvm/trunk/unittests/Support/CommandLineTest.cpp Sun Jan 27 01:02:46 2019
> @@ -839,78 +839,4 @@ TEST(CommandLineTest, GetCommandLineArgu
> }
> #endif
>
> -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
> +} // anonymous namespace
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
** We have updated our privacy policy, which contains important information about how we collect and process your personal data. To read the policy, please click here<http://www.graphcore.ai/privacy> **
This email and its attachments are intended solely for the addressed recipients and may contain confidential or legally privileged information.
If you are not the intended recipient you must not copy, distribute or disseminate this email in any way; to do so may be unlawful.
Any personal data/special category personal data herein are processed in accordance with UK data protection legislation.
All associated feasible security measures are in place. Further details are available from the Privacy Notice on the website and/or from the Company.
Graphcore Limited (registered in England and Wales with registration number 10185006) is registered at 107 Cheapside, London, UK, EC2V 6DN.
This message was scanned for viruses upon transmission. However Graphcore accepts no liability for any such transmission.
More information about the llvm-commits
mailing list