[llvm] c5f34d1 - [CommandLine] Keep option default value unset if no cl::init() is used

[Yevgeny Rouban via llvm-commits]

Author: Yevgeny Rouban
Date: 2022-03-11T14:24:25+07:00
New Revision: c5f34d169244db3f321d75d036902751ec03fe42

URL: https://github.com/llvm/llvm-project/commit/c5f34d169244db3f321d75d036902751ec03fe42
DIFF: https://github.com/llvm/llvm-project/commit/c5f34d169244db3f321d75d036902751ec03fe42.diff

LOG: [CommandLine] Keep option default value unset if no cl::init() is used

Current declaration of cl::opt is incoherent between class and non-class
specializations of the opt_storage template. There is an inconsistency
in the initialization of the Default field: for inClass instances
the default constructor is used - it sets the Optional Default field to
None; though for non-inClass instances the Default field is set to
the type's default value. For non-inClass instances it is impossible
to know if the option is defined with cl::init() initializer or not:

cl::opt<int> i1("option-i1");
cl::opt<int> i2("option-i2", cl::init(0));
cl::opt<std::string> s1("option-s1");
cl::opt<std::string> s2("option-s2", cl::init(""));

assert(s1.Default.hasValue() != s2.Default.hasValue()); // Ok
assert(i1.Default.hasValue() != i2.Default.hasValue()); // Fails

This patch changes constructor of the non-class specializations to keep
the Default field unset (that is None) rather than initialize it with
DataType().

Reviewed By: lattner
Differential Revision: https://reviews.llvm.org/D114645

Added: 
    

Modified: 
    llvm/include/llvm/Support/CommandLine.h
    llvm/unittests/Support/CommandLineTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index 3dbc21b2d8c49..3765aca33066c 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -1366,7 +1366,7 @@ template <class DataType> class opt_storage<DataType, false, false> {
 
   // Make sure we initialize the value with the default constructor for the
   // type.
-  opt_storage() : Value(DataType()), Default(DataType()) {}
+  opt_storage() : Value(DataType()), Default() {}
 
   template <class T> void setValue(const T &V, bool initial = false) {
     Value = V;

diff  --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index c683f912779ac..7fe1cf46ce260 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -1972,4 +1972,60 @@ TEST(CommandLineTest, ResetAllOptionOccurrences) {
   EXPECT_EQ(0u, ExtraArgs.size());
 }
 
+TEST(CommandLineTest, DefaultValue) {
+  cl::ResetCommandLineParser();
+
+  StackOption<bool> BoolOption("bool-option");
+  StackOption<std::string> StrOption("str-option");
+  StackOption<bool> BoolInitOption("bool-init-option", cl::init(true));
+  StackOption<std::string> StrInitOption("str-init-option",
+                                         cl::init("str-default-value"));
+
+  const char *Args[] = {"prog"}; // no options
+
+  std::string Errs;
+  raw_string_ostream OS(Errs);
+  EXPECT_TRUE(cl::ParseCommandLineOptions(1, Args, StringRef(), &OS));
+  EXPECT_TRUE(OS.str().empty());
+
+  EXPECT_TRUE(!BoolOption);
+  EXPECT_FALSE(BoolOption.Default.hasValue());
+  EXPECT_EQ(0, BoolOption.getNumOccurrences());
+
+  EXPECT_EQ("", StrOption);
+  EXPECT_FALSE(StrOption.Default.hasValue());
+  EXPECT_EQ(0, StrOption.getNumOccurrences());
+
+  EXPECT_TRUE(BoolInitOption);
+  EXPECT_TRUE(BoolInitOption.Default.hasValue());
+  EXPECT_EQ(0, BoolInitOption.getNumOccurrences());
+
+  EXPECT_EQ("str-default-value", StrInitOption);
+  EXPECT_TRUE(StrInitOption.Default.hasValue());
+  EXPECT_EQ(0, StrInitOption.getNumOccurrences());
+
+  const char *Args2[] = {"prog", "-bool-option", "-str-option=str-value",
+                         "-bool-init-option=0",
+                         "-str-init-option=str-init-value"};
+
+  EXPECT_TRUE(cl::ParseCommandLineOptions(5, Args2, StringRef(), &OS));
+  EXPECT_TRUE(OS.str().empty());
+
+  EXPECT_TRUE(BoolOption);
+  EXPECT_FALSE(BoolOption.Default.hasValue());
+  EXPECT_EQ(1, BoolOption.getNumOccurrences());
+
+  EXPECT_EQ("str-value", StrOption);
+  EXPECT_FALSE(StrOption.Default.hasValue());
+  EXPECT_EQ(1, StrOption.getNumOccurrences());
+
+  EXPECT_FALSE(BoolInitOption);
+  EXPECT_TRUE(BoolInitOption.Default.hasValue());
+  EXPECT_EQ(1, BoolInitOption.getNumOccurrences());
+
+  EXPECT_EQ("str-init-value", StrInitOption);
+  EXPECT_TRUE(StrInitOption.Default.hasValue());
+  EXPECT_EQ(1, StrInitOption.getNumOccurrences());
+}
+
 } // anonymous namespace