Skip to content

Commit

Permalink
Revert [CommandLine] Remove OptionCategory and SubCommand caches from…
Browse files Browse the repository at this point in the history 
… the Option class.

This reverts r364134 (git commit a5b83bc)

Caused errors in the asan bot, so the GeneralCategory global needs to
be changed to ManagedStatic.

Differential Revision: https://reviews.llvm.org/D62105

llvm-svn: 364141
  • Loading branch information
@donhinton
donhinton committed Jun 22, 2019
1 parent 1fa07eb commit 64b0924
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 118 deletions.
46 changes: 20 additions & 26 deletions llvm/include/llvm/Support/CommandLine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,6 @@ class OptionCategory {

StringRef getName() const { return Name; }
StringRef getDescription() const { return Description; }

SmallPtrSet<Option *, 16> MemberOptions;
};

// The general Option Category (used as default category).
Expand Down Expand Up @@ -285,12 +283,9 @@ class Option {
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

// 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;
SmallVector<OptionCategory *, 1>
Categories; // The Categories this option belongs to
SmallPtrSet<SubCommand *, 1> Subs; // The subcommands this option belongs to.

inline enum NumOccurrencesFlag getNumOccurrencesFlag() const {
return (enum NumOccurrencesFlag)Occurrences;
Expand Down Expand Up @@ -322,6 +317,12 @@ class Option {
return getNumOccurrencesFlag() == cl::ConsumeAfter;
}

bool isInAllSubCommands() const {
return any_of(Subs, [](const SubCommand *SC) {
return SC == &*AllSubCommands;
});
}

//-------------------------------------------------------------------------===
// Accessor functions set by OptionModifiers
//
Expand All @@ -335,13 +336,16 @@ class Option {
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) {}
FullyInitialized(false), Position(0), AdditionalVals(0) {
Categories.push_back(&GeneralCategory);
}

inline void setNumAdditionalVals(unsigned n) { AdditionalVals = n; }

Expand All @@ -350,14 +354,7 @@ class Option {

// addArgument - Register this argument with the commandline system.
//
virtual void addArgument(SubCommand &SC);

// addArgument - Only called in done() method to add default
// TopLevelSubCommand.
void addArgument() {
if (!FullyInitialized)
addArgument(*TopLevelSubCommand);
}
void addArgument();

/// Unregisters this option from the CommandLine system.
///
Expand Down Expand Up @@ -468,7 +465,7 @@ struct sub {

sub(SubCommand &S) : Sub(S) {}

template <class Opt> void apply(Opt &O) const { O.addArgument(Sub); }
template <class Opt> void apply(Opt &O) const { O.addSubCommand(Sub); }
};

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -1775,10 +1772,11 @@ class alias : public Option {
error("cl::alias must have argument name specified!");
if (!AliasFor)
error("cl::alias must have an cl::aliasopt(option) specified!");
for(OptionCategory *Cat: AliasFor->getCategories())
addCategory(*Cat);
for(SubCommand *SC: AliasFor->getSubCommands())
Option::addArgument(*SC);
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();
}

public:
Expand All @@ -1792,10 +1790,6 @@ class alias : public Option {
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) {
Expand Down
107 changes: 58 additions & 49 deletions llvm/lib/Support/CommandLine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class CommandLineParser {
// 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<std::pair<Option*, SubCommand*>, 4> DefaultOptions;
SmallVector<Option*, 4> DefaultOptions;

// This collects the different option categories that have been registered.
SmallPtrSet<OptionCategory *, 16> RegisteredOptionCategories;
Expand Down Expand Up @@ -182,16 +182,15 @@ class CommandLineParser {
}

void addLiteralOption(Option &Opt, StringRef Name) {
for(SubCommand *SC: Opt.getSubCommands())
addLiteralOption(Opt, SC, Name);
}

void addOption(Option *O, SubCommand *SC, bool ProcessDefaultOptions = false) {
if (!ProcessDefaultOptions && O->isDefaultOption()) {
DefaultOptions.push_back(std::make_pair(O, SC));
return;
if (Opt.Subs.empty())
addLiteralOption(Opt, &*TopLevelSubCommand, Name);
else {
for (auto SC : Opt.Subs)
addLiteralOption(Opt, SC, Name);
}
}

void addOption(Option *O, SubCommand *SC) {
bool HadErrors = false;
if (O->hasArgStr()) {
// If it's a DefaultOption, check to make sure it isn't already there.
Expand Down Expand Up @@ -233,14 +232,22 @@ class CommandLineParser {
for (const auto &Sub : RegisteredSubCommands) {
if (SC == Sub)
continue;
addOption(O, Sub, ProcessDefaultOptions);
addOption(O, Sub);
}
}
}

void addDefaultOptions() {
for (std::pair<Option *, SubCommand *> &DO : DefaultOptions) {
addOption(DO.first, DO.second, true);
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);
}
}

Expand Down Expand Up @@ -278,8 +285,17 @@ class CommandLineParser {
}

void removeOption(Option *O) {
for (auto SC : RegisteredSubCommands)
removeOption(O, SC);
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);
}
}
}

bool hasOptions(const SubCommand &Sub) const {
Expand Down Expand Up @@ -308,8 +324,17 @@ class CommandLineParser {
}

void updateArgStr(Option *O, StringRef NewName) {
for (auto SC : RegisteredSubCommands)
updateArgStr(O, NewName, SC);
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);
}
}
}

void printOptionValues();
Expand Down Expand Up @@ -402,38 +427,13 @@ extrahelp::extrahelp(StringRef Help) : morehelp(Help) {
GlobalParser->MoreHelp.push_back(Help);
}

void Option::addArgument(SubCommand &SC) {
GlobalParser->addOption(this, &SC);
void Option::addArgument() {
GlobalParser->addOption(this);
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);
Expand All @@ -444,7 +444,14 @@ void Option::setArgStr(StringRef S) {
}

void Option::addCategory(OptionCategory &C) {
C.MemberOptions.insert(this);
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);
}

void Option::reset() {
Expand Down Expand Up @@ -1295,7 +1302,9 @@ bool CommandLineParser::ParseCommandLineOptions(int argc,
auto &SinkOpts = ChosenSubCommand->SinkOpts;
auto &OptionsMap = ChosenSubCommand->OptionsMap;

addDefaultOptions();
for (auto O: DefaultOptions) {
addOption(O, true);
}

if (ConsumeAfterOpt) {
assert(PositionalOpts.size() > 0 &&
Expand Down Expand Up @@ -2195,7 +2204,7 @@ class CategorizedHelpPrinter : public HelpPrinter {
// 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->getCategories()) {
for (auto &Cat : Opt->Categories) {
assert(CategorizedOptions.count(Cat) > 0 &&
"Option has an unregistered category");
CategorizedOptions[Cat].push_back(Opt);
Expand Down Expand Up @@ -2456,7 +2465,7 @@ cl::getRegisteredSubcommands() {

void cl::HideUnrelatedOptions(cl::OptionCategory &Category, SubCommand &Sub) {
for (auto &I : Sub.OptionsMap) {
for (OptionCategory *Cat : I.second->getCategories()) {
for (auto &Cat : I.second->Categories) {
if (Cat != &Category &&
Cat != &GenericCategory)
I.second->setHiddenFlag(cl::ReallyHidden);
Expand All @@ -2467,7 +2476,7 @@ void cl::HideUnrelatedOptions(cl::OptionCategory &Category, SubCommand &Sub) {
void cl::HideUnrelatedOptions(ArrayRef<const cl::OptionCategory *> Categories,
SubCommand &Sub) {
for (auto &I : Sub.OptionsMap) {
for (OptionCategory *Cat : I.second->getCategories()) {
for (auto &Cat : I.second->Categories) {
if (find(Categories, Cat) == Categories.end() && Cat != &GenericCategory)
I.second->setHiddenFlag(cl::ReallyHidden);
}
Expand Down
Loading

0 comments on commit 64b0924

Please sign in to comment.