Skip to content

Commit

Permalink
Revert "Store OptTable::Info::Name as a StringRef"
Browse files Browse the repository at this point in the history
Another revert, for another set of issues I don't reproduce locally...

see https://lab.llvm.org/buildbot/#/builders/139/builds/32327

This reverts commit bdfa310.
  • Loading branch information
serge-sans-paille committed Dec 7, 2022
1 parent a2c9f12 commit 40ade84
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 35 deletions.
4 changes: 2 additions & 2 deletions clang/lib/Driver/ToolChains/Gnu.cpp
Expand Up @@ -331,8 +331,8 @@ static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
const Driver &D = TC.getDriver();
const llvm::opt::OptTable &Opts = D.getOpts();
StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
}
return HasStaticPIE;
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
Expand Up @@ -1055,7 +1055,7 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
// Only add the version-min options if we got a version from somewhere
if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
#define OPTION(PREFIX, NAME, VAR, ...) \
llvm::StringRef opt_##VAR = NAME; \
const char *opt_##VAR = NAME; \
(void)opt_##VAR;
#include "clang/Driver/Options.inc"
#undef OPTION
Expand Down
8 changes: 5 additions & 3 deletions llvm/include/llvm/Option/OptTable.h
Expand Up @@ -44,7 +44,7 @@ class OptTable {
/// A null terminated array of prefix strings to apply to name while
/// matching.
const char *const *Prefixes;
StringRef Name;
const char *Name;
const char *HelpText;
const char *MetaVar;
unsigned ID;
Expand Down Expand Up @@ -102,7 +102,9 @@ class OptTable {
const Option getOption(OptSpecifier Opt) const;

/// Lookup the name of the given option.
StringRef getOptionName(OptSpecifier id) const { return getInfo(id).Name; }
const char *getOptionName(OptSpecifier id) const {
return getInfo(id).Name;
}

/// Get the kind of the given option.
unsigned getOptionKind(OptSpecifier id) const {
Expand Down Expand Up @@ -182,7 +184,7 @@ class OptTable {
/// takes
///
/// \return true in success, and false in fail.
bool addValues(StringRef Option, const char *Values);
bool addValues(const char *Option, const char *Values);

/// Parse a single argument; returning the new argument and
/// updating Index.
Expand Down
50 changes: 29 additions & 21 deletions llvm/lib/Option/OptTable.cpp
Expand Up @@ -36,23 +36,31 @@ namespace opt {
// Ordering on Info. The ordering is *almost* case-insensitive lexicographic,
// with an exception. '\0' comes at the end of the alphabet instead of the
// beginning (thus options precede any other options which prefix them).
static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef B) {
size_t MinSize = std::min(A.size(), B.size());
if (int Res = A.substr(0, MinSize).compare_insensitive(B.substr(0, MinSize)))
return Res;
static int StrCmpOptionNameIgnoreCase(const char *A, const char *B) {
const char *X = A, *Y = B;
char a = tolower(*A), b = tolower(*B);
while (a == b) {
if (a == '\0')
return 0;

a = tolower(*++X);
b = tolower(*++Y);
}

if (A.size() == B.size())
return 0;
if (a == '\0') // A is a prefix of B.
return 1;
if (b == '\0') // B is a prefix of A.
return -1;

return (A.size() == MinSize) ? 1 /* A is a prefix of B. */
: -1 /* B is a prefix of A */;
// Otherwise lexicographic.
return (a < b) ? -1 : 1;
}

#ifndef NDEBUG
static int StrCmpOptionName(StringRef A, StringRef B) {
static int StrCmpOptionName(const char *A, const char *B) {
if (int N = StrCmpOptionNameIgnoreCase(A, B))
return N;
return A.compare(B);
return strcmp(A, B);
}

static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) {
Expand All @@ -78,7 +86,7 @@ static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) {
#endif

// Support lower_bound between info and an option name.
static inline bool operator<(const OptTable::Info &I, StringRef Name) {
static inline bool operator<(const OptTable::Info &I, const char *Name) {
return StrCmpOptionNameIgnoreCase(I.Name, Name) < 0;
}

Expand Down Expand Up @@ -178,7 +186,7 @@ static unsigned matchOption(const OptTable::Info *I, StringRef Str,
bool Matched = IgnoreCase ? Rest.startswith_insensitive(I->Name)
: Rest.startswith(I->Name);
if (Matched)
return Prefix.size() + I->Name.size();
return Prefix.size() + StringRef(I->Name).size();
}
}
return 0;
Expand Down Expand Up @@ -313,7 +321,7 @@ unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
return BestDistance;
}

bool OptTable::addValues(StringRef Option, const char *Values) {
bool OptTable::addValues(const char *Option, const char *Values) {
for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
Info &In = OptionInfos[I];
if (optionMatches(In, Option)) {
Expand All @@ -339,8 +347,8 @@ std::unique_ptr<Arg> OptTable::parseOneArgGrouped(InputArgList &Args,

const Info *End = OptionInfos.data() + OptionInfos.size();
StringRef Name = Str.ltrim(PrefixChars);
const Info *Start =
std::lower_bound(OptionInfos.data() + FirstSearchableIndex, End, Name);
const Info *Start = std::lower_bound(
OptionInfos.data() + FirstSearchableIndex, End, Name.data());
const Info *Fallback = nullptr;
unsigned Prev = Index;

Expand Down Expand Up @@ -395,19 +403,19 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
unsigned FlagsToInclude,
unsigned FlagsToExclude) const {
unsigned Prev = Index;
StringRef Str = Args.getArgString(Index);
const char *Str = Args.getArgString(Index);

// Anything that doesn't start with PrefixesUnion is an input, as is '-'
// itself.
if (isInput(PrefixesUnion, Str))
return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, Str.data());
return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, Str);

const Info *Start = OptionInfos.data() + FirstSearchableIndex;
const Info *End = OptionInfos.data() + OptionInfos.size();
StringRef Name = Str.ltrim(PrefixChars);
StringRef Name = StringRef(Str).ltrim(PrefixChars);

// Search for the first next option which could be a prefix.
Start = std::lower_bound(Start, End, Name);
Start = std::lower_bound(Start, End, Name.data());

// Options are stored in sorted order, with '\0' at the end of the
// alphabet. Since the only options which can accept a string must
Expand Down Expand Up @@ -447,9 +455,9 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
// If we failed to find an option and this arg started with /, then it's
// probably an input path.
if (Str[0] == '/')
return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, Str.data());
return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, Str);

return std::make_unique<Arg>(getOption(UnknownOptionID), Str, Index++, Str.data());
return std::make_unique<Arg>(getOption(UnknownOptionID), Str, Index++, Str);
}

InputArgList OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
Expand Down
11 changes: 5 additions & 6 deletions llvm/unittests/Option/OptionMarshallingTest.cpp
Expand Up @@ -6,11 +6,10 @@
//
//===----------------------------------------------------------------------===//

#include "llvm/ADT/StringRef.h"
#include "gtest/gtest.h"

struct OptionWithMarshallingInfo {
llvm::StringRef Name;
const char *Name;
const char *KeyPath;
const char *ImpliedCheck;
const char *ImpliedValue;
Expand All @@ -28,10 +27,10 @@ static const OptionWithMarshallingInfo MarshallingTable[] = {
};

TEST(OptionMarshalling, EmittedOrderSameAsDefinitionOrder) {
ASSERT_STREQ(MarshallingTable[0].Name.data(), "marshalled-flag-d");
ASSERT_STREQ(MarshallingTable[1].Name.data(), "marshalled-flag-c");
ASSERT_STREQ(MarshallingTable[2].Name.data(), "marshalled-flag-b");
ASSERT_STREQ(MarshallingTable[3].Name.data(), "marshalled-flag-a");
ASSERT_STREQ(MarshallingTable[0].Name, "marshalled-flag-d");
ASSERT_STREQ(MarshallingTable[1].Name, "marshalled-flag-c");
ASSERT_STREQ(MarshallingTable[2].Name, "marshalled-flag-b");
ASSERT_STREQ(MarshallingTable[3].Name, "marshalled-flag-a");
}

TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {
Expand Down
4 changes: 2 additions & 2 deletions llvm/utils/TableGen/OptParserEmitter.cpp
Expand Up @@ -54,9 +54,9 @@ static std::string getOptionSpelling(const Record &R) {

static void emitNameUsingSpelling(raw_ostream &OS, const Record &R) {
size_t PrefixLength;
OS << "llvm::StringRef(&";
OS << "&";
write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
OS << "[" << PrefixLength << "], " << R.getValueAsString("Name").size() << ")";
OS << "[" << PrefixLength << "]";
}

class MarshallingInfo {
Expand Down

0 comments on commit 40ade84

Please sign in to comment.