[llvm] a4deb14 - [LLVM][Support] Support for `llvm::cl::list`'s default values
Son Tuan Vu via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 6 10:50:56 PDT 2022
Author: Son Tuan Vu
Date: 2022-10-06T17:50:40Z
New Revision: a4deb14fdfaa183da938e40b54954afa9c7a11c1
URL: https://github.com/llvm/llvm-project/commit/a4deb14fdfaa183da938e40b54954afa9c7a11c1
DIFF: https://github.com/llvm/llvm-project/commit/a4deb14fdfaa183da938e40b54954afa9c7a11c1.diff
LOG: [LLVM][Support] Support for `llvm::cl::list`'s default values
This patch introduces support for default values of list of CL options.
It fixes the issue in https://github.com/llvm/llvm-project/issues/52667
Reviewed By: bkramer
Differential Revision: https://reviews.llvm.org/D135311
Added:
Modified:
llvm/include/llvm/Support/CommandLine.h
llvm/unittests/Support/CommandLineTest.cpp
mlir/include/mlir/Pass/PassOptions.h
mlir/lib/Pass/PassRegistry.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h
index 15deb8a0b027a..0788308577f59 100644
--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -437,10 +437,22 @@ template <class Ty> struct initializer {
template <class Opt> void apply(Opt &O) const { O.setInitialValue(Init); }
};
+template <class Ty> struct list_initializer {
+ ArrayRef<Ty> Inits;
+ list_initializer(ArrayRef<Ty> Vals) : Inits(Vals) {}
+
+ template <class Opt> void apply(Opt &O) const { O.setInitialValues(Inits); }
+};
+
template <class Ty> initializer<Ty> init(const Ty &Val) {
return initializer<Ty>(Val);
}
+template <class Ty>
+list_initializer<Ty> list_init(ArrayRef<Ty> Vals) {
+ return list_initializer<Ty>(Vals);
+}
+
// Allow the user to specify which external variable they want to store the
// results of the command line argument processing into, if they don't want to
// store it in the option itself.
@@ -1504,6 +1516,9 @@ extern template class opt<bool>;
//
template <class DataType, class StorageClass> class list_storage {
StorageClass *Location = nullptr; // Where to store the object...
+ std::vector<OptionValue<DataType>> Default =
+ std::vector<OptionValue<DataType>>();
+ bool DefaultAssigned = false;
public:
list_storage() = default;
@@ -1517,12 +1532,22 @@ template <class DataType, class StorageClass> class list_storage {
return false;
}
- template <class T> void addValue(const T &V) {
+ template <class T> void addValue(const T &V, bool initial = false) {
assert(Location != nullptr &&
"cl::location(...) not specified for a command "
"line option with external storage!");
Location->push_back(V);
+ if (initial)
+ Default.push_back(V);
+ }
+
+ const std::vector<OptionValue<DataType>> &getDefault() const {
+ return Default;
}
+
+ void assignDefault() { DefaultAssigned = true; }
+ void overwriteDefault() { DefaultAssigned = false; }
+ bool isDefaultAssigned() { return DefaultAssigned; }
};
// Define how to hold a class type object, such as a string.
@@ -1535,6 +1560,8 @@ template <class DataType, class StorageClass> class list_storage {
//
template <class DataType> class list_storage<DataType, bool> {
std::vector<DataType> Storage;
+ std::vector<OptionValue<DataType>> Default;
+ bool DefaultAssigned = false;
public:
using iterator = typename std::vector<DataType>::iterator;
@@ -1598,7 +1625,19 @@ template <class DataType> class list_storage<DataType, bool> {
std::vector<DataType> *operator&() { return &Storage; }
const std::vector<DataType> *operator&() const { return &Storage; }
- template <class T> void addValue(const T &V) { Storage.push_back(V); }
+ template <class T> void addValue(const T &V, bool initial = false) {
+ Storage.push_back(V);
+ if (initial)
+ Default.push_back(OptionValue<DataType>(V));
+ }
+
+ const std::vector<OptionValue<DataType>> &getDefault() const {
+ return Default;
+ }
+
+ void assignDefault() { DefaultAssigned = true; }
+ void overwriteDefault() { DefaultAssigned = false; }
+ bool isDefaultAssigned() { return DefaultAssigned; }
};
//===----------------------------------------------------------------------===//
@@ -1622,6 +1661,10 @@ class list : public Option, public list_storage<DataType, StorageClass> {
StringRef Arg) override {
typename ParserClass::parser_data_type Val =
typename ParserClass::parser_data_type();
+ if (list_storage<DataType, StorageClass>::isDefaultAssigned()) {
+ clear();
+ list_storage<DataType, StorageClass>::overwriteDefault();
+ }
if (Parser.parse(*this, ArgName, Arg, Val))
return true; // Parse Error!
list_storage<DataType, StorageClass>::addValue(Val);
@@ -1647,6 +1690,8 @@ class list : public Option, public list_storage<DataType, StorageClass> {
void setDefault() override {
Positions.clear();
list_storage<DataType, StorageClass>::clear();
+ for (auto &Val : list_storage<DataType, StorageClass>::getDefault())
+ list_storage<DataType, StorageClass>::addValue(Val.getValue());
}
void done() {
@@ -1666,6 +1711,20 @@ class list : public Option, public list_storage<DataType, StorageClass> {
return Positions[optnum];
}
+ void clear() {
+ Positions.clear();
+ list_storage<DataType, StorageClass>::clear();
+ }
+
+ // setInitialValues - Used by the cl::list_init modifier...
+ void setInitialValues(ArrayRef<DataType> Vs) {
+ assert(!(list_storage<DataType, StorageClass>::isDefaultAssigned()) &&
+ "Cannot have two default values");
+ list_storage<DataType, StorageClass>::assignDefault();
+ for (auto &Val : Vs)
+ list_storage<DataType, StorageClass>::addValue(Val, true);
+ }
+
void setNumAdditionalVals(unsigned n) { Option::setNumAdditionalVals(n); }
template <class... Mods>
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 7af8c89e90e0f..dd6e12223dacb 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -1038,7 +1038,7 @@ TEST(CommandLineTest, ResponseFileEOLs) {
}
}
-TEST(CommandLineTest, SetDefautValue) {
+TEST(CommandLineTest, SetDefaultValue) {
cl::ResetCommandLineParser();
StackOption<std::string> Opt1("opt1", cl::init("true"));
@@ -1046,15 +1046,32 @@ TEST(CommandLineTest, SetDefautValue) {
cl::alias Alias("alias", llvm::cl::aliasopt(Opt2));
StackOption<int> Opt3("opt3", cl::init(3));
- const char *args[] = {"prog", "-opt1=false", "-opt2", "-opt3"};
+ llvm::SmallVector<int, 3> IntVals = {1, 2, 3};
+ llvm::SmallVector<std::string, 3> StrVals = {"foo", "bar", "baz"};
+
+ StackOption<int, cl::list<int>> List1(
+ "list1", cl::list_init<int>(llvm::ArrayRef<int>(IntVals)),
+ cl::CommaSeparated);
+ StackOption<std::string, cl::list<std::string>> List2(
+ "list2", cl::list_init<std::string>(llvm::ArrayRef<std::string>(StrVals)),
+ cl::CommaSeparated);
+ cl::alias ListAlias("list-alias", llvm::cl::aliasopt(List2));
+
+ const char *args[] = {"prog", "-opt1=false", "-list1", "4",
+ "-list1", "5,6", "-opt2", "-opt3"};
EXPECT_TRUE(
- cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
+ cl::ParseCommandLineOptions(7, args, StringRef(), &llvm::nulls()));
EXPECT_EQ(Opt1, "false");
EXPECT_TRUE(Opt2);
EXPECT_EQ(Opt3, 3);
+ for (size_t I = 0, E = IntVals.size(); I < E; ++I) {
+ EXPECT_EQ(IntVals[I] + 3, List1[I]);
+ EXPECT_EQ(StrVals[I], List2[I]);
+ }
+
Opt2 = false;
Opt3 = 1;
@@ -1071,7 +1088,13 @@ TEST(CommandLineTest, SetDefautValue) {
EXPECT_EQ(Opt1, "true");
EXPECT_TRUE(Opt2);
EXPECT_EQ(Opt3, 3);
+ for (size_t I = 0, E = IntVals.size(); I < E; ++I) {
+ EXPECT_EQ(IntVals[I], List1[I]);
+ EXPECT_EQ(StrVals[I], List2[I]);
+ }
+
Alias.removeArgument();
+ ListAlias.removeArgument();
}
TEST(CommandLineTest, ReadConfigFile) {
diff --git a/mlir/include/mlir/Pass/PassOptions.h b/mlir/include/mlir/Pass/PassOptions.h
index 3ee91c7d463be..67c785c3633ce 100644
--- a/mlir/include/mlir/Pass/PassOptions.h
+++ b/mlir/include/mlir/Pass/PassOptions.h
@@ -229,6 +229,10 @@ class PassOptions : protected llvm::cl::SubCommand {
bool handleOccurrence(unsigned pos, StringRef argName,
StringRef arg) override {
+ if (this->isDefaultAssigned()) {
+ this->clear();
+ this->overwriteDefault();
+ }
this->optHasValue = true;
return failed(detail::pass_options::parseCommaSeparatedList(
*this, argName, arg, elementParser,
@@ -418,6 +422,7 @@ struct OptionValue<mlir::OpPassManager> final : GenericOptionValue {
using WrapperType = mlir::OpPassManager;
OptionValue();
+ OptionValue(const OptionValue<mlir::OpPassManager> &rhs);
OptionValue(const mlir::OpPassManager &value);
OptionValue<mlir::OpPassManager> &operator=(const mlir::OpPassManager &rhs);
~OptionValue();
diff --git a/mlir/lib/Pass/PassRegistry.cpp b/mlir/lib/Pass/PassRegistry.cpp
index 9f0ce23c5b817..2d6f3417b2c9e 100644
--- a/mlir/lib/Pass/PassRegistry.cpp
+++ b/mlir/lib/Pass/PassRegistry.cpp
@@ -348,6 +348,11 @@ llvm::cl::OptionValue<OpPassManager>::OptionValue(
const mlir::OpPassManager &value) {
setValue(value);
}
+llvm::cl::OptionValue<OpPassManager>::OptionValue(
+ const llvm::cl::OptionValue<mlir::OpPassManager> &rhs) {
+ if (rhs.hasValue())
+ setValue(rhs.getValue());
+}
llvm::cl::OptionValue<OpPassManager> &
llvm::cl::OptionValue<OpPassManager>::operator=(
const mlir::OpPassManager &rhs) {
More information about the llvm-commits
mailing list