hintonda updated this revision to Diff 208736.
hintonda added a comment.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.
Make GeneralCategory a ManagedStatic.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62105/new/
https://reviews.llvm.org/D62105
Files:
clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
clang-tools-extra/clangd/indexer/IndexerMain.cpp
clang/tools/clang-refactor/ClangRefactor.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===================================================================
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -95,16 +95,16 @@
cl::Option *Retrieved = Map["test-option"];
ASSERT_EQ(&TestOption, Retrieved) << "Retrieved wrong option.";
- ASSERT_NE(Retrieved->Categories.end(),
- find_if(Retrieved->Categories,
+ ASSERT_NE(Retrieved->getCategories().end(),
+ find_if(Retrieved->getCategories(),
[&](const llvm::cl::OptionCategory *Cat) {
- return Cat == &cl::GeneralCategory;
+ return Cat == &*cl::GeneralCategory;
}))
<< "Incorrect default option category.";
Retrieved->addCategory(TestCategory);
- ASSERT_NE(Retrieved->Categories.end(),
- find_if(Retrieved->Categories,
+ ASSERT_NE(Retrieved->getCategories().end(),
+ find_if(Retrieved->getCategories(),
[&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
}))
@@ -160,8 +160,8 @@
TEST(CommandLineTest, UseOptionCategory) {
StackOption<int> TestOption2("test-option", cl::cat(TestCategory));
- ASSERT_NE(TestOption2.Categories.end(),
- find_if(TestOption2.Categories,
+ ASSERT_NE(TestOption2.getCategories().end(),
+ find_if(TestOption2.getCategories(),
[&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
}))
@@ -170,42 +170,44 @@
TEST(CommandLineTest, UseMultipleCategories) {
StackOption<int> TestOption2("test-option2", cl::cat(TestCategory),
- cl::cat(cl::GeneralCategory),
- cl::cat(cl::GeneralCategory));
+ cl::cat(*cl::GeneralCategory),
+ cl::cat(*cl::GeneralCategory));
+ auto TestOption2Categories = TestOption2.getCategories();
// Make sure cl::GeneralCategory wasn't added twice.
- ASSERT_EQ(TestOption2.Categories.size(), 2U);
+ ASSERT_EQ(TestOption2Categories.size(), 2U);
- ASSERT_NE(TestOption2.Categories.end(),
- find_if(TestOption2.Categories,
+ ASSERT_NE(TestOption2Categories.end(),
+ find_if(TestOption2Categories,
[&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
}))
<< "Failed to assign Option Category.";
- ASSERT_NE(TestOption2.Categories.end(),
- find_if(TestOption2.Categories,
+ ASSERT_NE(TestOption2Categories.end(),
+ find_if(TestOption2Categories,
[&](const llvm::cl::OptionCategory *Cat) {
- return Cat == &cl::GeneralCategory;
+ return Cat == &*cl::GeneralCategory;
}))
<< "Failed to assign General Category.";
cl::OptionCategory AnotherCategory("Additional test Options", "Description");
StackOption<int> TestOption("test-option", cl::cat(TestCategory),
cl::cat(AnotherCategory));
- ASSERT_EQ(TestOption.Categories.end(),
- find_if(TestOption.Categories,
+ auto TestOptionCategories = TestOption.getCategories();
+ ASSERT_EQ(TestOptionCategories.end(),
+ find_if(TestOptionCategories,
[&](const llvm::cl::OptionCategory *Cat) {
- return Cat == &cl::GeneralCategory;
+ return Cat == &*cl::GeneralCategory;
}))
<< "Failed to remove General Category.";
- ASSERT_NE(TestOption.Categories.end(),
- find_if(TestOption.Categories,
+ ASSERT_NE(TestOptionCategories.end(),
+ find_if(TestOptionCategories,
[&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
}))
<< "Failed to assign Option Category.";
- ASSERT_NE(TestOption.Categories.end(),
- find_if(TestOption.Categories,
+ ASSERT_NE(TestOptionCategories.end(),
+ find_if(TestOptionCategories,
[&](const llvm::cl::OptionCategory *Cat) {
return Cat == &AnotherCategory;
}))
@@ -378,6 +380,30 @@
testAliasRequired(array_lengthof(opts2), opts2);
}
+TEST(CommandLineTest, AliasWithSubCommand) {
+ StackSubCommand SC1("sc1", "Subcommand 1");
+ StackOption<std::string> Option1("option", cl::value_desc("output file"),
+ cl::init("-"), cl::desc("Option"),
+ cl::sub(SC1));
+ StackOption<std::string, cl::alias> Alias1("o", llvm::cl::aliasopt(Option1),
+ cl::desc("Alias for --option"),
+ cl::sub(SC1));
+}
+
+TEST(CommandLineTest, AliasWithMultipleSubCommandsWithSameOption) {
+ StackSubCommand SC1("sc1", "Subcommand 1");
+ StackOption<std::string> Option1("option", cl::value_desc("output file"),
+ cl::init("-"), cl::desc("Option"),
+ cl::sub(SC1));
+ StackSubCommand SC2("sc2", "Subcommand 2");
+ StackOption<std::string> Option2("option", cl::value_desc("output file"),
+ cl::init("-"), cl::desc("Option"),
+ cl::sub(SC2));
+
+ StackOption<std::string, cl::alias> Alias1("o", llvm::cl::aliasopt(Option1),
+ cl::desc("Alias for --option"));
+}
+
TEST(CommandLineTest, HideUnrelatedOptions) {
StackOption<int> TestOption1("hide-option-1");
StackOption<int> TestOption2("hide-option-2", cl::cat(TestCategory));
Index: llvm/lib/Support/CommandLine.cpp
===================================================================
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -142,7 +142,7 @@
// This collects Options added with the cl::DefaultOption flag. Since they can
// be overridden, they are not added to the appropriate SubCommands until
// ParseCommandLineOptions actually runs.
- SmallVector<Option*, 4> DefaultOptions;
+ SmallVector<std::pair<Option*, SubCommand*>, 4> DefaultOptions;
// This collects the different option categories that have been registered.
SmallPtrSet<OptionCategory *, 16> RegisteredOptionCategories;
@@ -151,6 +151,7 @@
SmallPtrSet<SubCommand *, 4> RegisteredSubCommands;
CommandLineParser() : ActiveSubCommand(nullptr) {
+ RegisteredOptionCategories.insert(&*GeneralCategory);
registerSubCommand(&*TopLevelSubCommand);
registerSubCommand(&*AllSubCommands);
}
@@ -182,15 +183,16 @@
}
void addLiteralOption(Option &Opt, StringRef Name) {
- if (Opt.Subs.empty())
- addLiteralOption(Opt, &*TopLevelSubCommand, Name);
- else {
- for (auto SC : Opt.Subs)
- addLiteralOption(Opt, SC, Name);
- }
+ for(SubCommand *SC: Opt.getSubCommands())
+ addLiteralOption(Opt, SC, Name);
}
- void addOption(Option *O, SubCommand *SC) {
+ void addOption(Option *O, SubCommand *SC, bool ProcessDefaultOptions = false) {
+ if (!ProcessDefaultOptions && O->isDefaultOption()) {
+ DefaultOptions.push_back(std::make_pair(O, SC));
+ return;
+ }
+
bool HadErrors = false;
if (O->hasArgStr()) {
// If it's a DefaultOption, check to make sure it isn't already there.
@@ -232,22 +234,14 @@
for (const auto &Sub : RegisteredSubCommands) {
if (SC == Sub)
continue;
- addOption(O, Sub);
+ addOption(O, Sub, ProcessDefaultOptions);
}
}
}
- void addOption(Option *O, bool ProcessDefaultOption = false) {
- if (!ProcessDefaultOption && O->isDefaultOption()) {
- DefaultOptions.push_back(O);
- return;
- }
-
- if (O->Subs.empty()) {
- addOption(O, &*TopLevelSubCommand);
- } else {
- for (auto SC : O->Subs)
- addOption(O, SC);
+ void addDefaultOptions() {
+ for (std::pair<Option *, SubCommand *> &DO : DefaultOptions) {
+ addOption(DO.first, DO.second, true);
}
}
@@ -285,17 +279,8 @@
}
void removeOption(Option *O) {
- if (O->Subs.empty())
- removeOption(O, &*TopLevelSubCommand);
- else {
- if (O->isInAllSubCommands()) {
- for (auto SC : RegisteredSubCommands)
- removeOption(O, SC);
- } else {
- for (auto SC : O->Subs)
- removeOption(O, SC);
- }
- }
+ for (auto SC : RegisteredSubCommands)
+ removeOption(O, SC);
}
bool hasOptions(const SubCommand &Sub) const {
@@ -324,17 +309,8 @@
}
void updateArgStr(Option *O, StringRef NewName) {
- if (O->Subs.empty())
- updateArgStr(O, NewName, &*TopLevelSubCommand);
- else {
- if (O->isInAllSubCommands()) {
- for (auto SC : RegisteredSubCommands)
- updateArgStr(O, NewName, SC);
- } else {
- for (auto SC : O->Subs)
- updateArgStr(O, NewName, SC);
- }
- }
+ for (auto SC : RegisteredSubCommands)
+ updateArgStr(O, NewName, SC);
}
void printOptionValues();
@@ -389,6 +365,7 @@
MoreHelp.clear();
RegisteredOptionCategories.clear();
+ RegisteredOptionCategories.insert(&*GeneralCategory);
ResetAllOptionOccurrences();
RegisteredSubCommands.clear();
@@ -427,13 +404,38 @@
GlobalParser->MoreHelp.push_back(Help);
}
-void Option::addArgument() {
- GlobalParser->addOption(this);
+void Option::addArgument(SubCommand &SC) {
+ GlobalParser->addOption(this, &SC);
FullyInitialized = true;
}
void Option::removeArgument() { GlobalParser->removeOption(this); }
+SmallPtrSet<OptionCategory *, 1> Option::getCategories() const {
+ SmallPtrSet<OptionCategory *, 1> Cats;
+ for (OptionCategory *C: GlobalParser->RegisteredOptionCategories) {
+ if (C->MemberOptions.find(this) != C->MemberOptions.end())
+ Cats.insert(C);
+ }
+ if (Cats.empty())
+ Cats.insert(&*GeneralCategory);
+ return Cats;
+}
+
+SmallPtrSet<SubCommand *, 1> Option::getSubCommands() const {
+ // This can happen for enums and literal options.
+ if (ArgStr.empty())
+ return SmallPtrSet<SubCommand *, 1>{&*TopLevelSubCommand};
+
+ SmallPtrSet<SubCommand *, 1> Subs;
+ for (SubCommand *SC : GlobalParser->getRegisteredSubcommands()) {
+ auto I = SC->OptionsMap.find(ArgStr);
+ if (I != SC->OptionsMap.end() && I->getValue() == this)
+ Subs.insert(SC);
+ }
+ return Subs;
+}
+
void Option::setArgStr(StringRef S) {
if (FullyInitialized)
GlobalParser->updateArgStr(this, S);
@@ -444,14 +446,7 @@
}
void Option::addCategory(OptionCategory &C) {
- assert(!Categories.empty() && "Categories cannot be empty.");
- // Maintain backward compatibility by replacing the default GeneralCategory
- // if it's still set. Otherwise, just add the new one. The GeneralCategory
- // must be explicitly added if you want multiple categories that include it.
- if (&C != &GeneralCategory && Categories[0] == &GeneralCategory)
- Categories[0] = &C;
- else if (find(Categories, &C) == Categories.end())
- Categories.push_back(&C);
+ C.MemberOptions.insert(this);
}
void Option::reset() {
@@ -462,7 +457,8 @@
}
// Initialise the general option category.
-OptionCategory llvm::cl::GeneralCategory("General options");
+LLVM_REQUIRE_CONSTANT_INITIALIZATION
+ManagedStatic<OptionCategory> llvm::cl::GeneralCategory;
void OptionCategory::registerCategory() {
GlobalParser->registerCategory(this);
@@ -1302,9 +1298,7 @@
auto &SinkOpts = ChosenSubCommand->SinkOpts;
auto &OptionsMap = ChosenSubCommand->OptionsMap;
- for (auto O: DefaultOptions) {
- addOption(O, true);
- }
+ addDefaultOptions();
if (ConsumeAfterOpt) {
assert(PositionalOpts.size() > 0 &&
@@ -2204,7 +2198,7 @@
// options within categories will also be alphabetically sorted.
for (size_t I = 0, E = Opts.size(); I != E; ++I) {
Option *Opt = Opts[I].second;
- for (auto &Cat : Opt->Categories) {
+ for (auto *Cat : Opt->getCategories()) {
assert(CategorizedOptions.count(Cat) > 0 &&
"Option has an unregistered category");
CategorizedOptions[Cat].push_back(Opt);
@@ -2465,7 +2459,7 @@
void cl::HideUnrelatedOptions(cl::OptionCategory &Category, SubCommand &Sub) {
for (auto &I : Sub.OptionsMap) {
- for (auto &Cat : I.second->Categories) {
+ for (OptionCategory *Cat : I.second->getCategories()) {
if (Cat != &Category &&
Cat != &GenericCategory)
I.second->setHiddenFlag(cl::ReallyHidden);
@@ -2476,7 +2470,7 @@
void cl::HideUnrelatedOptions(ArrayRef<const cl::OptionCategory *> Categories,
SubCommand &Sub) {
for (auto &I : Sub.OptionsMap) {
- for (auto &Cat : I.second->Categories) {
+ for (OptionCategory *Cat : I.second->getCategories()) {
if (find(Categories, Cat) == Categories.end() && Cat != &GenericCategory)
I.second->setHiddenFlag(cl::ReallyHidden);
}
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -187,24 +187,27 @@
//
class OptionCategory {
private:
- StringRef const Name;
- StringRef const Description;
+ StringRef Name = "General Category";
+ StringRef Description;
void registerCategory();
public:
- OptionCategory(StringRef const Name,
- StringRef const Description = "")
+ OptionCategory(StringRef Name,
+ StringRef Description = "")
: Name(Name), Description(Description) {
registerCategory();
}
+ OptionCategory() = default;
StringRef getName() const { return Name; }
StringRef getDescription() const { return Description; }
+
+ SmallPtrSet<Option *, 16> MemberOptions;
};
// The general Option Category (used as default category).
-extern OptionCategory GeneralCategory;
+extern ManagedStatic<OptionCategory> GeneralCategory;
//===----------------------------------------------------------------------===//
// SubCommand class
@@ -283,9 +286,12 @@
StringRef ArgStr; // The argument string itself (ex: "help", "o")
StringRef HelpStr; // The descriptive text message for -help
StringRef ValueStr; // String describing what the value of this option is
- SmallVector<OptionCategory *, 1>
- Categories; // The Categories this option belongs to
- SmallPtrSet<SubCommand *, 1> Subs; // The subcommands this option belongs to.
+
+ // Return the set of OptionCategories that this Option belongs to.
+ SmallPtrSet<OptionCategory *, 1> getCategories() const;
+
+ // Return the set of SubCommands that this Option belongs to.
+ SmallPtrSet<SubCommand *, 1> getSubCommands() const;
inline enum NumOccurrencesFlag getNumOccurrencesFlag() const {
return (enum NumOccurrencesFlag)Occurrences;
@@ -317,12 +323,6 @@
return getNumOccurrencesFlag() == cl::ConsumeAfter;
}
- bool isInAllSubCommands() const {
- return any_of(Subs, [](const SubCommand *SC) {
- return SC == &*AllSubCommands;
- });
- }
-
//-------------------------------------------------------------------------===
// Accessor functions set by OptionModifiers
//
@@ -336,16 +336,13 @@
void setMiscFlag(enum MiscFlags M) { Misc |= M; }
void setPosition(unsigned pos) { Position = pos; }
void addCategory(OptionCategory &C);
- void addSubCommand(SubCommand &S) { Subs.insert(&S); }
protected:
explicit Option(enum NumOccurrencesFlag OccurrencesFlag,
enum OptionHidden Hidden)
: NumOccurrences(0), Occurrences(OccurrencesFlag), Value(0),
HiddenFlag(Hidden), Formatting(NormalFormatting), Misc(0),
- FullyInitialized(false), Position(0), AdditionalVals(0) {
- Categories.push_back(&GeneralCategory);
- }
+ FullyInitialized(false), Position(0), AdditionalVals(0) {}
inline void setNumAdditionalVals(unsigned n) { AdditionalVals = n; }
@@ -354,7 +351,14 @@
// addArgument - Register this argument with the commandline system.
//
- void addArgument();
+ virtual void addArgument(SubCommand &SC);
+
+ // addArgument - Only called in done() method to add default
+ // TopLevelSubCommand.
+ void addArgument() {
+ if (!FullyInitialized)
+ addArgument(*TopLevelSubCommand);
+ }
/// Unregisters this option from the CommandLine system.
///
@@ -465,7 +469,7 @@
sub(SubCommand &S) : Sub(S) {}
- template <class Opt> void apply(Opt &O) const { O.addSubCommand(Sub); }
+ template <class Opt> void apply(Opt &O) const { O.addArgument(Sub); }
};
//===----------------------------------------------------------------------===//
@@ -1772,11 +1776,10 @@
error("cl::alias must have argument name specified!");
if (!AliasFor)
error("cl::alias must have an cl::aliasopt(option) specified!");
- if (!Subs.empty())
- error("cl::alias must not have cl::sub(), aliased option's cl::sub() will be used!");
- Subs = AliasFor->Subs;
- Categories = AliasFor->Categories;
- addArgument();
+ for(OptionCategory *Cat: AliasFor->getCategories())
+ addCategory(*Cat);
+ for(SubCommand *SC: AliasFor->getSubCommands())
+ Option::addArgument(*SC);
}
public:
@@ -1790,6 +1793,10 @@
AliasFor = &O;
}
+ // Does nothing when called via apply. Aliases call Option::addArgument
+ // directly in the done() method to actually add the option..
+ void addArgument(SubCommand &SC) override {}
+
template <class... Mods>
explicit alias(const Mods &... Ms)
: Option(Optional, Hidden), AliasFor(nullptr) {
Index: clang/tools/clang-refactor/ClangRefactor.cpp
===================================================================
--- clang/tools/clang-refactor/ClangRefactor.cpp
+++ clang/tools/clang-refactor/ClangRefactor.cpp
@@ -38,11 +38,9 @@
static cl::OptionCategory CommonRefactorOptions("Refactoring options");
static cl::opt<bool> Verbose("v", cl::desc("Use verbose output"),
- cl::cat(cl::GeneralCategory),
cl::sub(*cl::AllSubCommands));
static cl::opt<bool> Inplace("i", cl::desc("Inplace edit <file>s"),
- cl::cat(cl::GeneralCategory),
cl::sub(*cl::AllSubCommands));
} // end namespace opts
@@ -611,7 +609,7 @@
ClangRefactorTool RefactorTool;
CommonOptionsParser Options(
- argc, argv, cl::GeneralCategory, cl::ZeroOrMore,
+ argc, argv, *cl::GeneralCategory, cl::ZeroOrMore,
"Clang-based refactoring tool for C, C++ and Objective-C");
if (auto Err = RefactorTool.Init()) {
Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===================================================================
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -110,7 +110,7 @@
)";
auto Executor = clang::tooling::createExecutorFromCommandLineArgs(
- argc, argv, llvm::cl::GeneralCategory, Overview);
+ argc, argv, *llvm::cl::GeneralCategory, Overview);
if (!Executor) {
llvm::errs() << llvm::toString(Executor.takeError()) << "\n";
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===================================================================
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -77,7 +77,7 @@
// By resetting the parser options, we lost the standard -help flag.
llvm::cl::opt<bool, false, llvm::cl::parser<bool>> Help{
"help", llvm::cl::desc("Display available options"),
- llvm::cl::ValueDisallowed, llvm::cl::cat(llvm::cl::GeneralCategory)};
+ llvm::cl::ValueDisallowed};
virtual void run() = 0;
protected:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
