diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index d2079fead6680..49f4a668ae416 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -552,6 +552,7 @@ struct OptionValueBase : public GenericOptionValue { // Some options may take their value from a different data type. template void setValue(const DT & /*V*/) {} + // Returns whether this instance matches the argument. bool compare(const DataType & /*V*/) const { return false; } bool compare(const GenericOptionValue & /*V*/) const override { @@ -587,7 +588,8 @@ template class OptionValueCopy : public GenericOptionValue { Value = V; } - bool compare(const DataType &V) const { return Valid && (Value != V); } + // Returns whether this instance matches V. + bool compare(const DataType &V) const { return Valid && (Value == V); } bool compare(const GenericOptionValue &V) const override { const OptionValueCopy &VC = @@ -1442,7 +1444,7 @@ class opt } void printOptionValue(size_t GlobalWidth, bool Force) const override { - if (Force || this->getDefault().compare(this->getValue())) { + if (Force || !this->getDefault().compare(this->getValue())) { cl::printOptionDiff(*this, Parser, this->getValue(), this->getDefault(), GlobalWidth); } diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index d3efb8b67be5c..55633d7cafa47 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -2181,7 +2181,7 @@ void generic_parser_base::printGenericOptionDiff( unsigned NumOpts = getNumOptions(); for (unsigned i = 0; i != NumOpts; ++i) { - if (Value.compare(getOptionValue(i))) + if (!Value.compare(getOptionValue(i))) continue; outs() << "= " << getOption(i); @@ -2189,7 +2189,7 @@ void generic_parser_base::printGenericOptionDiff( size_t NumSpaces = MaxOptWidth > L ? MaxOptWidth - L : 0; outs().indent(NumSpaces) << " (default: "; for (unsigned j = 0; j != NumOpts; ++j) { - if (Default.compare(getOptionValue(j))) + if (!Default.compare(getOptionValue(j))) continue; outs() << getOption(j); break; diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index 4f7cb40263b37..41cc8260acfed 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -1294,7 +1294,8 @@ struct AutoDeleteFile { } }; -class PrintOptionInfoTest : public ::testing::Test { +template +class PrintOptionTestBase : public ::testing::Test { public: // Return std::string because the output of a failing EXPECT check is // unreadable for StringRef. It also avoids any lifetime issues. @@ -1309,7 +1310,7 @@ class PrintOptionInfoTest : public ::testing::Test { StackOption TestOption(Opt, cl::desc(HelpText), OptionAttributes...); - printOptionInfo(TestOption, 26); + Func(TestOption); outs().flush(); } auto Buffer = MemoryBuffer::getFile(File.FilePath); @@ -1321,14 +1322,15 @@ class PrintOptionInfoTest : public ::testing::Test { enum class OptionValue { Val }; const StringRef Opt = "some-option"; const StringRef HelpText = "some help"; +}; -private: // This is a workaround for cl::Option sub-classes having their // printOptionInfo functions private. - void printOptionInfo(const cl::Option &O, size_t Width) { - O.printOptionInfo(Width); - } -}; +void printOptionInfo(const cl::Option &O) { + O.printOptionInfo(/*GlobalWidth=*/26); +} + +using PrintOptionInfoTest = PrintOptionTestBase; TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithoutSentinel) { std::string Output = @@ -1402,7 +1404,7 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoMultilineValueDescription) { "which has a really long description\n" "thus it is multi-line."), clEnumValN(OptionValue::Val, "", - "This is an unnamed enum value option\n" + "This is an unnamed enum value\n" "Should be indented as well"))); // clang-format off @@ -1411,11 +1413,40 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoMultilineValueDescription) { " =v1 - This is the first enum value\n" " which has a really long description\n" " thus it is multi-line.\n" - " = - This is an unnamed enum value option\n" + " = - This is an unnamed enum value\n" " Should be indented as well\n").str()); // clang-format on } +void printOptionValue(const cl::Option &O) { + O.printOptionValue(/*GlobalWidth=*/12, /*Force=*/true); +} + +using PrintOptionValueTest = PrintOptionTestBase; + +TEST_F(PrintOptionValueTest, PrintOptionDefaultValue) { + std::string Output = + runTest(cl::init(OptionValue::Val), + cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"))); + + EXPECT_EQ(Output, (" --" + Opt + " = v1 (default: v1)\n").str()); +} + +TEST_F(PrintOptionValueTest, PrintOptionNoDefaultValue) { + std::string Output = + runTest(cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"))); + + // Note: the option still has a (zero-initialized) value, but the default + // is invalid and doesn't match any value. + EXPECT_EQ(Output, (" --" + Opt + " = v1 (default: )\n").str()); +} + +TEST_F(PrintOptionValueTest, PrintOptionUnknownValue) { + std::string Output = runTest(cl::init(OptionValue::Val)); + + EXPECT_EQ(Output, (" --" + Opt + " = *unknown option value*\n").str()); +} + class GetOptionWidthTest : public ::testing::Test { public: enum class OptionValue { Val }; diff --git a/mlir/test/Pass/pipeline-options-parsing.mlir b/mlir/test/Pass/pipeline-options-parsing.mlir index 33bef75ee94a2..34313cd556603 100644 --- a/mlir/test/Pass/pipeline-options-parsing.mlir +++ b/mlir/test/Pass/pipeline-options-parsing.mlir @@ -2,16 +2,18 @@ // RUN: not mlir-opt %s -pass-pipeline='builtin.module(builtin.module(test-module-pass{test-option=3}))' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_2 %s // RUN: not mlir-opt %s -pass-pipeline='builtin.module(builtin.module(func.func(test-options-pass{list=3}), test-module-pass{invalid-option=3}))' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_3 %s // RUN: not mlir-opt %s -pass-pipeline='builtin.module(test-options-pass{list=3 list=notaninteger})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_4 %s +// RUN: not mlir-opt %s -pass-pipeline='builtin.module(test-options-pass{enum=invalid})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_5 %s // RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-options-pass{list=1,2,3,4 list=5 string=value1 string=value2}))' // RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(func.func(test-options-pass{string-list=a list=1,2,3,4 string-list=b,c list=5 string-list=d string=nested_pipeline{arg1=10 arg2=" {} " arg3=true}}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_1 %s -// RUN: mlir-opt %s -verify-each=false -test-options-pass-pipeline='list=1 string-list=a,b' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_2 %s -// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(builtin.module(func.func(test-options-pass{list=3}), func.func(test-options-pass{list=1,2,3,4})))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_3 %s +// RUN: mlir-opt %s -verify-each=false -test-options-pass-pipeline='list=1 string-list=a,b enum=one' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_2 %s +// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(builtin.module(func.func(test-options-pass{list=3}), func.func(test-options-pass{enum=one list=1,2,3,4})))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_3 %s // CHECK_ERROR_1: missing closing '}' while processing pass options // CHECK_ERROR_2: no such option test-option // CHECK_ERROR_3: no such option invalid-option // CHECK_ERROR_4: 'notaninteger' value invalid for integer argument +// CHECK_ERROR_5: for the --enum option: Cannot find option named 'invalid'! -// CHECK_1: test-options-pass{list=1,2,3,4,5 string=nested_pipeline{arg1=10 arg2=" {} " arg3=true} string-list=a,b,c,d} -// CHECK_2: test-options-pass{list=1 string= string-list=a,b} -// CHECK_3: builtin.module(builtin.module(func.func(test-options-pass{list=3 string= }),func.func(test-options-pass{list=1,2,3,4 string= }))) +// CHECK_1: test-options-pass{enum=zero list=1,2,3,4,5 string=nested_pipeline{arg1=10 arg2=" {} " arg3=true} string-list=a,b,c,d} +// CHECK_2: test-options-pass{enum=one list=1 string= string-list=a,b} +// CHECK_3: builtin.module(builtin.module(func.func(test-options-pass{enum=zero list=3 string= }),func.func(test-options-pass{enum=one list=1,2,3,4 string= }))) diff --git a/mlir/test/lib/Pass/TestPassManager.cpp b/mlir/test/lib/Pass/TestPassManager.cpp index b9cd217839bb9..3b24cf44e2110 100644 --- a/mlir/test/lib/Pass/TestPassManager.cpp +++ b/mlir/test/lib/Pass/TestPassManager.cpp @@ -53,6 +53,8 @@ struct TestOptionsPass : public PassWrapper> { MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestOptionsPass) + enum Enum {}; + struct Options : public PassPipelineOptions { ListOption listOption{*this, "list", llvm::cl::desc("Example list option")}; @@ -60,6 +62,10 @@ struct TestOptionsPass *this, "string-list", llvm::cl::desc("Example string list option")}; Option stringOption{*this, "string", llvm::cl::desc("Example string option")}; + Option enumOption{ + *this, "enum", llvm::cl::desc("Example enum option"), + llvm::cl::values(clEnumValN(0, "zero", "Example zero value"), + clEnumValN(1, "one", "Example one value"))}; }; TestOptionsPass() = default; TestOptionsPass(const TestOptionsPass &) : PassWrapper() {} @@ -67,6 +73,7 @@ struct TestOptionsPass listOption = options.listOption; stringOption = options.stringOption; stringListOption = options.stringListOption; + enumOption = options.enumOption; } void runOnOperation() final {} @@ -81,6 +88,10 @@ struct TestOptionsPass *this, "string-list", llvm::cl::desc("Example string list option")}; Option stringOption{*this, "string", llvm::cl::desc("Example string option")}; + Option enumOption{ + *this, "enum", llvm::cl::desc("Example enum option"), + llvm::cl::values(clEnumValN(0, "zero", "Example zero value"), + clEnumValN(1, "one", "Example one value"))}; }; /// A test pass that always aborts to enable testing the crash recovery