serge-sans-paille updated this revision to Diff 490804.
serge-sans-paille added a comment.
Herald added subscribers: cfe-commits, kadircet, arphaman.
Herald added a project: clang-tools-extra.
Fix build + make StringLiteral initialization constexpr.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142181/new/
https://reviews.llvm.org/D142181
Files:
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang/include/clang/Tooling/Refactoring/RefactoringOption.h
clang/lib/Tooling/Refactoring/RefactoringActions.cpp
llvm/include/llvm/Option/OptTable.h
llvm/include/llvm/Option/Option.h
llvm/include/llvm/Support/CommandLine.h
llvm/lib/CodeGen/TargetPassConfig.cpp
llvm/lib/Support/CommandLine.cpp
llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===================================================================
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -98,7 +98,7 @@
static const char ArgString[] = "new-test-option";
static const char ValueString[] = "Integer";
- StringMap<cl::Option *> &Map =
+ DenseMap<StringRef, cl::Option *> &Map =
cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
ASSERT_EQ(Map.count("test-option"), 1u) << "Could not find option in map.";
@@ -419,7 +419,7 @@
ASSERT_EQ(cl::NotHidden, TestOption2.getOptionHiddenFlag())
<< "Hid extra option that should be visable.";
- StringMap<cl::Option *> &Map =
+ DenseMap<StringRef, cl::Option *> &Map =
cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
ASSERT_TRUE(Map.count("help") == (size_t)0 ||
cl::NotHidden == Map["help"]->getOptionHiddenFlag())
@@ -445,7 +445,7 @@
ASSERT_EQ(cl::NotHidden, TestOption3.getOptionHiddenFlag())
<< "Hid extra option that should be visable.";
- StringMap<cl::Option *> &Map =
+ DenseMap<StringRef, cl::Option *> &Map =
cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
ASSERT_TRUE(Map.count("help") == (size_t)0 ||
cl::NotHidden == Map["help"]->getOptionHiddenFlag())
@@ -1319,7 +1319,7 @@
}
enum class OptionValue { Val };
- const StringRef Opt = "some-option";
+ const StringLiteral Opt = "some-option";
const StringRef HelpText = "some help";
private:
@@ -1421,7 +1421,7 @@
enum class OptionValue { Val };
template <typename... Ts>
- size_t runTest(StringRef ArgName, Ts... OptionAttributes) {
+ size_t runTest(StringLiteral ArgName, Ts... OptionAttributes) {
StackOption<OptionValue> TestOption(ArgName, cl::desc("some help"),
OptionAttributes...);
return getOptionWidth(TestOption);
@@ -1435,7 +1435,7 @@
};
TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) {
- StringRef ArgName("a-long-argument-name");
+ StringLiteral ArgName("a-long-argument-name");
size_t ExpectedStrSize = (" --" + ArgName + "=<value> - ").str().size();
EXPECT_EQ(
runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))),
@@ -1443,7 +1443,7 @@
}
TEST_F(GetOptionWidthTest, GetOptionWidthFirstOptionNameLonger) {
- StringRef OptName("a-long-option-name");
+ StringLiteral OptName("a-long-option-name");
size_t ExpectedStrSize = (" =" + OptName + " - ").str().size();
EXPECT_EQ(
runTest("a", cl::values(clEnumValN(OptionValue::Val, OptName, "help"),
@@ -1452,7 +1452,7 @@
}
TEST_F(GetOptionWidthTest, GetOptionWidthSecondOptionNameLonger) {
- StringRef OptName("a-long-option-name");
+ StringLiteral OptName("a-long-option-name");
size_t ExpectedStrSize = (" =" + OptName + " - ").str().size();
EXPECT_EQ(
runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
@@ -1472,7 +1472,7 @@
TEST_F(GetOptionWidthTest,
GetOptionWidthValueOptionalEmptyOptionWithNoDescription) {
- StringRef ArgName("a");
+ StringLiteral ArgName("a");
// The length of a=<value> (including indentation) is actually the same as the
// =<empty> string, so it is impossible to distinguish via testing the case
// where the empty string is ignored from where it is not ignored.
Index: llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
===================================================================
--- llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
+++ llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
@@ -50,7 +50,7 @@
}
TEST(CommandLineInitTest, GetPresetOptions) {
- StringMap<cl::Option *> &Map =
+ DenseMap<StringRef, cl::Option *> &Map =
cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
for (auto *Str :
Index: llvm/lib/Support/CommandLine.cpp
===================================================================
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -275,7 +275,7 @@
auto End = Sub.OptionsMap.end();
for (auto Name : OptionNames) {
auto I = Sub.OptionsMap.find(Name);
- if (I != End && I->getValue() == O)
+ if (I != End && I->second == O)
Sub.OptionsMap.erase(I);
}
@@ -381,7 +381,7 @@
O->hasArgStr())
addOption(O, sub);
else
- addLiteralOption(*O, sub, E.first());
+ addLiteralOption(*O, sub, E.first);
}
}
}
@@ -572,9 +572,10 @@
/// the specified option on the command line. If there is a value specified
/// (after an equal sign) return that as well. This assumes that leading dashes
/// have already been stripped.
-static Option *LookupNearestOption(StringRef Arg,
- const StringMap<Option *> &OptionsMap,
- std::string &NearestString) {
+static Option *
+LookupNearestOption(StringRef Arg,
+ const DenseMap<StringRef, Option *> &OptionsMap,
+ std::string &NearestString) {
// Reject all dashes.
if (Arg.empty())
return nullptr;
@@ -587,8 +588,8 @@
// Find the closest match.
Option *Best = nullptr;
unsigned BestDistance = 0;
- for (StringMap<Option *>::const_iterator it = OptionsMap.begin(),
- ie = OptionsMap.end();
+ for (DenseMap<StringRef, Option *>::const_iterator it = OptionsMap.begin(),
+ ie = OptionsMap.end();
it != ie; ++it) {
Option *O = it->second;
// Do not suggest really hidden options (not shown in any help).
@@ -722,9 +723,9 @@
//
static Option *getOptionPred(StringRef Name, size_t &Length,
bool (*Pred)(const Option *),
- const StringMap<Option *> &OptionsMap) {
- StringMap<Option *>::const_iterator OMI = OptionsMap.find(Name);
- if (OMI != OptionsMap.end() && !Pred(OMI->getValue()))
+ const DenseMap<StringRef, Option *> &OptionsMap) {
+ DenseMap<StringRef, Option *>::const_iterator OMI = OptionsMap.find(Name);
+ if (OMI != OptionsMap.end() && !Pred(OMI->second))
OMI = OptionsMap.end();
// Loop while we haven't found an option and Name still has at least two
@@ -733,7 +734,7 @@
while (OMI == OptionsMap.end() && Name.size() > 1) {
Name = Name.substr(0, Name.size() - 1); // Chop off the last character.
OMI = OptionsMap.find(Name);
- if (OMI != OptionsMap.end() && !Pred(OMI->getValue()))
+ if (OMI != OptionsMap.end() && !Pred(OMI->second))
OMI = OptionsMap.end();
}
@@ -751,7 +752,7 @@
static Option *
HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value,
bool &ErrorParsing,
- const StringMap<Option *> &OptionsMap) {
+ const DenseMap<StringRef, Option *> &OptionsMap) {
if (Arg.size() == 1)
return nullptr;
@@ -2273,12 +2274,13 @@
}
// Copy Options into a vector so we can sort them as we like.
-static void sortOpts(StringMap<Option *> &OptMap,
+static void sortOpts(DenseMap<StringRef, Option *> &OptMap,
SmallVectorImpl<std::pair<const char *, Option *>> &Opts,
bool ShowHidden) {
SmallPtrSet<Option *, 32> OptionSet; // Duplicate option detection.
- for (StringMap<Option *>::iterator I = OptMap.begin(), E = OptMap.end();
+ for (DenseMap<StringRef, Option *>::iterator I = OptMap.begin(),
+ E = OptMap.end();
I != E; ++I) {
// Ignore really-hidden options.
if (I->second->getOptionHiddenFlag() == ReallyHidden)
@@ -2293,7 +2295,7 @@
continue;
Opts.push_back(
- std::pair<const char *, Option *>(I->getKey().data(), I->second));
+ std::pair<const char *, Option *>(I->first.data(), I->second));
}
// Sort the options list alphabetically.
@@ -2756,7 +2758,7 @@
CommonOptions->ExtraVersionPrinters.push_back(func);
}
-StringMap<Option *> &cl::getRegisteredOptions(SubCommand &Sub) {
+DenseMap<StringRef, Option *> &cl::getRegisteredOptions(SubCommand &Sub) {
initCommonOptions();
auto &Subs = GlobalParser->RegisteredSubCommands;
(void)Subs;
Index: llvm/lib/CodeGen/TargetPassConfig.cpp
===================================================================
--- llvm/lib/CodeGen/TargetPassConfig.cpp
+++ llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -210,28 +210,28 @@
/// Option names for limiting the codegen pipeline.
/// Those are used in error reporting and we didn't want
/// to duplicate their names all over the place.
-static const char StartAfterOptName[] = "start-after";
-static const char StartBeforeOptName[] = "start-before";
-static const char StopAfterOptName[] = "stop-after";
-static const char StopBeforeOptName[] = "stop-before";
+static constexpr StringLiteral StartAfterOptName = "start-after";
+static constexpr StringLiteral StartBeforeOptName = "start-before";
+static constexpr StringLiteral StopAfterOptName = "stop-after";
+static constexpr StringLiteral StopBeforeOptName = "stop-before";
static cl::opt<std::string>
- StartAfterOpt(StringRef(StartAfterOptName),
+ StartAfterOpt(StartAfterOptName,
cl::desc("Resume compilation after a specific pass"),
cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
static cl::opt<std::string>
- StartBeforeOpt(StringRef(StartBeforeOptName),
+ StartBeforeOpt(StartBeforeOptName,
cl::desc("Resume compilation before a specific pass"),
cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
static cl::opt<std::string>
- StopAfterOpt(StringRef(StopAfterOptName),
+ StopAfterOpt(StopAfterOptName,
cl::desc("Stop compilation after a specific pass"),
cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
static cl::opt<std::string>
- StopBeforeOpt(StringRef(StopBeforeOptName),
+ StopBeforeOpt(StopBeforeOptName,
cl::desc("Stop compilation before a specific pass"),
cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
@@ -406,8 +406,7 @@
const PassRegistry &PR = *PassRegistry::getPassRegistry();
const PassInfo *PI = PR.getPassInfo(PassName);
if (!PI)
- report_fatal_error(Twine('\"') + Twine(PassName) +
- Twine("\" pass is not registered."));
+ report_fatal_error("\"" + PassName + "\" pass is not registered.");
return PI;
}
@@ -450,11 +449,11 @@
StopBefore = getPassIDFromName(StopBeforeName);
StopAfter = getPassIDFromName(StopAfterName);
if (StartBefore && StartAfter)
- report_fatal_error(Twine(StartBeforeOptName) + Twine(" and ") +
- Twine(StartAfterOptName) + Twine(" specified!"));
+ report_fatal_error(StartBeforeOptName + " and " + StartAfterOptName +
+ " specified!");
if (StopBefore && StopAfter)
- report_fatal_error(Twine(StopBeforeOptName) + Twine(" and ") +
- Twine(StopAfterOptName) + Twine(" specified!"));
+ report_fatal_error(StopBeforeOptName + " and " + StopAfterOptName +
+ " specified!");
Started = (StartAfter == nullptr) && (StartBefore == nullptr);
}
@@ -526,11 +525,11 @@
std::tie(StopAfter, std::ignore) =
LLVMTM.getPassNameFromLegacyName(StopAfter);
if (!StartBefore.empty() && !StartAfter.empty())
- report_fatal_error(Twine(StartBeforeOptName) + Twine(" and ") +
- Twine(StartAfterOptName) + Twine(" specified!"));
+ report_fatal_error(StartBeforeOptName + " and " + StartAfterOptName +
+ " specified!");
if (!StopBefore.empty() && !StopAfter.empty())
- report_fatal_error(Twine(StopBeforeOptName) + Twine(" and ") +
- Twine(StopAfterOptName) + Twine(" specified!"));
+ report_fatal_error(StopBeforeOptName + " and " + StopAfterOptName +
+ " specified!");
PIC.registerShouldRunOptionalPassCallback(
[=, EnableCurrent = StartBefore.empty() && StartAfter.empty(),
@@ -673,8 +672,8 @@
std::string Res;
static cl::opt<std::string> *PassNames[] = {&StartAfterOpt, &StartBeforeOpt,
&StopAfterOpt, &StopBeforeOpt};
- static const char *OptNames[] = {StartAfterOptName, StartBeforeOptName,
- StopAfterOptName, StopBeforeOptName};
+ static constexpr StringLiteral OptNames[] = {StartAfterOptName, StartBeforeOptName,
+ StopAfterOptName, StopBeforeOptName};
bool IsFirst = true;
for (int Idx = 0; Idx < 4; ++Idx)
if (!PassNames[Idx]->empty()) {
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -232,7 +232,7 @@
SmallVector<Option *, 4> PositionalOpts;
SmallVector<Option *, 4> SinkOpts;
- StringMap<Option *> OptionsMap;
+ DenseMap<StringRef, Option *> OptionsMap;
Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
};
@@ -1242,15 +1242,18 @@
//===----------------------------------------------------------------------===//
// This class is used because we must use partial specialization to handle
-// literal string arguments specially (const char* does not correctly respond to
-// the apply method). Because the syntax to use this is a pain, we have the
-// 'apply' method below to handle the nastiness...
+// literal string arguments specially (const char[...] does not correctly
+// respond to the apply method). Because the syntax to use this is a pain, we
+// have the 'apply' method below to handle the nastiness...
+//
+// Note that char * and StringRef are not part of the specialization, which
+// helps preventing reference to stack variables (but doesn't totally prevents
+// it, unfortunately).
//
template <class Mod> struct applicator {
template <class Opt> static void opt(const Mod &M, Opt &O) { M.apply(O); }
};
-// Handle const char* as a special case...
template <unsigned n> struct applicator<char[n]> {
template <class Opt> static void opt(StringRef Str, Opt &O) {
O.setArgStr(Str);
@@ -1261,7 +1264,7 @@
O.setArgStr(Str);
}
};
-template <> struct applicator<StringRef > {
+template <> struct applicator<StringLiteral> {
template <class Opt> static void opt(StringRef Str, Opt &O) {
O.setArgStr(Str);
}
@@ -2017,7 +2020,7 @@
/// Hopefully this API can be deprecated soon. Any situation where options need
/// to be modified by tools or libraries should be handled by sane APIs rather
/// than just handing around a global list.
-StringMap<Option *> &
+DenseMap<StringRef, Option *> &
getRegisteredOptions(SubCommand &Sub = SubCommand::getTopLevel());
/// Use this to get all registered SubCommands from the provided parser.
Index: llvm/include/llvm/Option/Option.h
===================================================================
--- llvm/include/llvm/Option/Option.h
+++ llvm/include/llvm/Option/Option.h
@@ -95,7 +95,7 @@
}
/// Get the name of this option without any prefix.
- StringRef getName() const {
+ StringLiteral getName() const {
assert(Info && "Must have a valid info!");
return Info->Name;
}
Index: llvm/include/llvm/Option/OptTable.h
===================================================================
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -44,7 +44,7 @@
/// A null terminated array of prefix strings to apply to name while
/// matching.
ArrayRef<StringLiteral> Prefixes;
- StringRef Name;
+ StringLiteral Name;
const char *HelpText;
const char *MetaVar;
unsigned ID;
Index: clang/lib/Tooling/Refactoring/RefactoringActions.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/RefactoringActions.cpp
+++ clang/lib/Tooling/Refactoring/RefactoringActions.cpp
@@ -18,7 +18,7 @@
class DeclNameOption final : public OptionalRefactoringOption<std::string> {
public:
- StringRef getName() const override { return "name"; }
+ llvm::StringLiteral getName() const override { return "name"; }
StringRef getDescription() const override {
return "Name of the extracted declaration";
}
@@ -47,7 +47,7 @@
class OldQualifiedNameOption : public RequiredRefactoringOption<std::string> {
public:
- StringRef getName() const override { return "old-qualified-name"; }
+ llvm::StringLiteral getName() const override { return "old-qualified-name"; }
StringRef getDescription() const override {
return "The old qualified name to be renamed";
}
@@ -55,7 +55,7 @@
class NewQualifiedNameOption : public RequiredRefactoringOption<std::string> {
public:
- StringRef getName() const override { return "new-qualified-name"; }
+ llvm::StringLiteral getName() const override { return "new-qualified-name"; }
StringRef getDescription() const override {
return "The new qualified name to change the symbol to";
}
@@ -63,7 +63,7 @@
class NewNameOption : public RequiredRefactoringOption<std::string> {
public:
- StringRef getName() const override { return "new-name"; }
+ llvm::StringLiteral getName() const override { return "new-name"; }
StringRef getDescription() const override {
return "The new name to change the symbol to";
}
Index: clang/include/clang/Tooling/Refactoring/RefactoringOption.h
===================================================================
--- clang/include/clang/Tooling/Refactoring/RefactoringOption.h
+++ clang/include/clang/Tooling/Refactoring/RefactoringOption.h
@@ -30,7 +30,7 @@
/// Returns the name of the refactoring option.
///
/// Each refactoring option must have a unique name.
- virtual StringRef getName() const = 0;
+ virtual llvm::StringLiteral getName() const = 0;
virtual StringRef getDescription() const = 0;
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -91,7 +91,7 @@
opt<T> Option;
public:
- RetiredFlag(llvm::StringRef Name)
+ RetiredFlag(llvm::StringLiteral Name)
: Option(Name, cat(Retired), desc("Obsolete flag, ignored"), Hidden,
llvm::cl::callback([Name](const T &) {
llvm::errs()
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
