From c5f34d169244db3f321d75d036902751ec03fe42 Mon Sep 17 00:00:00 2001 From: Yevgeny Rouban Date: Fri, 11 Mar 2022 14:21:45 +0700 Subject: [PATCH] [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 i1("option-i1"); cl::opt i2("option-i2", cl::init(0)); cl::opt s1("option-s1"); cl::opt 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 --- llvm/include/llvm/Support/CommandLine.h | 2 +- llvm/unittests/Support/CommandLineTest.cpp | 56 ++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index 3dbc21b..3765aca 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -1366,7 +1366,7 @@ public: // 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 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 c683f91..7fe1cf4 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 BoolOption("bool-option"); + StackOption StrOption("str-option"); + StackOption BoolInitOption("bool-init-option", cl::init(true)); + StackOption 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 -- 2.7.4