mirrors/llvm-project
0
0
Fork 0
mirror of https://github.com/llvm/llvm-project.git synced 2025-04-25 12:36:10 +00:00
Code Issues Projects Releases Wiki Activity

Fix logic to detect cl::option equality. (#65754)

Browse Source 
This is a new attempt of https://reviews.llvm.org/D159481, this time as
GitHub PR.

`GenericOptionValue::compare()` should return `true` for a match.

- `OptionValueBase::compare()` always returns `false` and shouldn't
match anything.
- `OptionValueCopy::compare()` returns `false` if not `Valid` which
corresponds to no match.

Also adding some tests.
This commit is contained in:
Christian Sigg 2023-09-10 12:25:19 +02:00 committed by GitHub
parent 18b6e2139f
commit 710b5a1232
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 64 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
llvm/include/llvm/Support/CommandLine.h
View File

@ -552,6 +552,7 @@ struct OptionValueBase : public GenericOptionValue {
// Some options may take their value from a different data type.
template <class DT> 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 @@ public:
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<DataType> &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<ParserClass>(*this, Parser, this->getValue(),
this->getDefault(), GlobalWidth);
}

4
llvm/lib/Support/CommandLine.cpp
View File

@ -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;

49
llvm/unittests/Support/CommandLineTest.cpp
View File

@ -1294,7 +1294,8 @@ struct AutoDeleteFile {
}
};
class PrintOptionInfoTest : public ::testing::Test {
template <void (*Func)(const cl::Option &)>
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 @@ public:
StackOption<OptionValue> TestOption(Opt, cl::desc(HelpText),
OptionAttributes...);
printOptionInfo(TestOption, 26);
Func(TestOption);
outs().flush();
}
auto Buffer = MemoryBuffer::getFile(File.FilePath);
@ -1321,14 +1322,15 @@ public:
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<printOptionInfo>;
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"
" =<empty> - This is an unnamed enum value option\n"
" =<empty> - 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<printOptionValue>;
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 };

12
mlir/test/Pass/pipeline-options-parsing.mlir
View File

@ -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= })))

11
mlir/test/lib/Pass/TestPassManager.cpp
View File

@ -53,6 +53,8 @@ struct TestOptionsPass
: public PassWrapper<TestOptionsPass, OperationPass<func::FuncOp>> {
MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestOptionsPass)
enum Enum {};
struct Options : public PassPipelineOptions<Options> {
ListOption<int> 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<std::string> stringOption{*this, "string",
llvm::cl::desc("Example string option")};
Option<Enum> 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<std::string> stringOption{*this, "string",
llvm::cl::desc("Example string option")};
Option<Enum> 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