[clang] 501f92d - [llvm] Construct option's prefixed name at compile-time

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 9 09:52:40 PDT 2023


Author: Jan Svoboda
Date: 2023-08-09T09:52:34-07:00
New Revision: 501f92d343828c0066d3286c3ae6606021b8b271

URL: https://github.com/llvm/llvm-project/commit/501f92d343828c0066d3286c3ae6606021b8b271
DIFF: https://github.com/llvm/llvm-project/commit/501f92d343828c0066d3286c3ae6606021b8b271.diff

LOG: [llvm] Construct option's prefixed name at compile-time

Some Clang command-line handling code could benefit from the option's prefixed name being a `StringLiteral`. This patch changes the `llvm::opt` TableGen backend to generate and emit that into the .inc file.

Depends on D157028.

Reviewed By: MaskRay

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/CompileCommands.cpp
    clang/lib/Driver/Driver.cpp
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/lib/Tooling/Tooling.cpp
    llvm/include/llvm/Option/OptTable.h
    llvm/include/llvm/Option/Option.h
    llvm/lib/Option/OptTable.cpp
    llvm/unittests/Option/OptionMarshallingTest.cpp
    llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index 957a6f873e1022..00f4c4ca9fef0a 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -493,8 +493,8 @@ llvm::ArrayRef<ArgStripper::Rule> ArgStripper::rulesFor(llvm::StringRef Arg) {
   static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
   static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
       NAME##_init, std::size(NAME##_init) - 1);
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-               HELP, METAVAR, VALUES)                                          \
+#define OPTION(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS,       \
+               FLAGS, PARAM, HELP, METAVAR, VALUES)                            \
   Prefixes[DriverID::OPT_##ID] = PREFIX;
 #include "clang/Driver/Options.inc"
 #undef OPTION
@@ -505,8 +505,8 @@ llvm::ArrayRef<ArgStripper::Rule> ArgStripper::rulesFor(llvm::StringRef Arg) {
       DriverID AliasID;
       const void *AliasArgs;
     } AliasTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-               HELP, METAVAR, VALUES)                                          \
+#define OPTION(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS,       \
+               FLAGS, PARAM, HELP, METAVAR, VALUES)                            \
   {DriverID::OPT_##ID, DriverID::OPT_##ALIAS, ALIASARGS},
 #include "clang/Driver/Options.inc"
 #undef OPTION

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index b34003041373f5..60f5eed8e2d729 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -238,7 +238,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
 }
 
 void Driver::setDriverMode(StringRef Value) {
-  static const std::string OptName =
+  static StringRef OptName =
       getOpts().getOption(options::OPT_driver_mode).getPrefixedName();
   if (auto M = llvm::StringSwitch<std::optional<DriverMode>>(Value)
                    .Case("gcc", GCCMode)
@@ -6554,7 +6554,7 @@ bool clang::driver::willEmitRemarks(const ArgList &Args) {
 
 llvm::StringRef clang::driver::getDriverMode(StringRef ProgName,
                                              ArrayRef<const char *> Args) {
-  static const std::string OptName =
+  static StringRef OptName =
       getDriverOptTable().getOption(options::OPT_driver_mode).getPrefixedName();
   llvm::StringRef Opt;
   for (StringRef Arg : Args) {

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index be53ce3e472659..c4082db0090733 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -422,8 +422,8 @@ static T extractMaskValue(T KeyPath) {
 }
 
 #define PARSE_OPTION_WITH_MARSHALLING(                                         \
-    ARGS, DIAGS, PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,  \
-    PARAM, HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT,     \
+    ARGS, DIAGS, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS,     \
+    FLAGS, PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT,        \
     KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER,          \
     DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)                              \
   if ((FLAGS)&options::CC1Option) {                                            \
@@ -439,10 +439,10 @@ static T extractMaskValue(T KeyPath) {
 // Capture the extracted value as a lambda argument to avoid potential issues
 // with lifetime extension of the reference.
 #define GENERATE_OPTION_WITH_MARSHALLING(                                      \
-    CONSUMER, PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,     \
-    PARAM, HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT,     \
-    KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER,          \
-    DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)                              \
+    CONSUMER, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \
+    PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,      \
+    DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
+    MERGER, EXTRACTOR, TABLE_INDEX)                                            \
   if ((FLAGS)&options::CC1Option) {                                            \
     [&](const auto &Extracted) {                                               \
       if (ALWAYS_EMIT ||                                                       \

diff  --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp
index dc82a1f3772dd2..e292fa724d2bb7 100644
--- a/clang/lib/Tooling/Tooling.cpp
+++ b/clang/lib/Tooling/Tooling.cpp
@@ -276,14 +276,14 @@ void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine,
     return;
   const auto &Table = driver::getDriverOptTable();
   // --target=X
-  const std::string TargetOPT =
+  StringRef TargetOPT =
       Table.getOption(driver::options::OPT_target).getPrefixedName();
   // -target X
-  const std::string TargetOPTLegacy =
+  StringRef TargetOPTLegacy =
       Table.getOption(driver::options::OPT_target_legacy_spelling)
           .getPrefixedName();
   // --driver-mode=X
-  const std::string DriverModeOPT =
+  StringRef DriverModeOPT =
       Table.getOption(driver::options::OPT_driver_mode).getPrefixedName();
   auto TargetMode =
       driver::ToolChain::getTargetAndModeFromProgramName(InvokedAs);
@@ -303,7 +303,7 @@ void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine,
   }
   if (ShouldAddTarget) {
     CommandLine.insert(++CommandLine.begin(),
-                       TargetOPT + TargetMode.TargetPrefix);
+                       (TargetOPT + TargetMode.TargetPrefix).str());
   }
 }
 

diff  --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h
index ef7d05d6a1ee99..092f41e0ff5a55 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -44,7 +44,7 @@ class OptTable {
     /// A null terminated array of prefix strings to apply to name while
     /// matching.
     ArrayRef<StringLiteral> Prefixes;
-    StringRef Name;
+    StringLiteral PrefixedName;
     const char *HelpText;
     const char *MetaVar;
     unsigned ID;
@@ -55,6 +55,11 @@ class OptTable {
     unsigned short AliasID;
     const char *AliasArgs;
     const char *Values;
+
+    StringRef getName() const {
+      unsigned PrefixLength = Prefixes.empty() ? 0 : Prefixes[0].size();
+      return PrefixedName.drop_front(PrefixLength);
+    }
   };
 
 private:
@@ -111,7 +116,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; }
+  StringRef getOptionName(OptSpecifier id) const {
+    return getInfo(id).getName();
+  }
 
   /// Get the kind of the given option.
   unsigned getOptionKind(OptSpecifier id) const {
@@ -298,31 +305,31 @@ class PrecomputedOptTable : public OptTable {
 
 } // end namespace llvm
 
-#define LLVM_MAKE_OPT_ID_WITH_ID_PREFIX(ID_PREFIX, PREFIX, NAME, ID, KIND,     \
-                                        GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-                                        HELPTEXT, METAVAR, VALUES)             \
+#define LLVM_MAKE_OPT_ID_WITH_ID_PREFIX(ID_PREFIX, PREFIX, PREFIXED_NAME, ID,  \
+                                        KIND, GROUP, ALIAS, ALIASARGS, FLAGS,  \
+                                        PARAM, HELPTEXT, METAVAR, VALUES)      \
   ID_PREFIX##ID
 
-#define LLVM_MAKE_OPT_ID(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS,      \
-                         FLAGS, PARAM, HELPTEXT, METAVAR, VALUES)              \
-  LLVM_MAKE_OPT_ID_WITH_ID_PREFIX(OPT_, PREFIX, NAME, ID, KIND, GROUP, ALIAS,  \
-                                  ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR,  \
-                                  VALUE)
+#define LLVM_MAKE_OPT_ID(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS,        \
+                         ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR, VALUES)   \
+  LLVM_MAKE_OPT_ID_WITH_ID_PREFIX(OPT_, PREFIX, PREFIXED_NAME, ID, KIND,       \
+                                  GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,       \
+                                  HELPTEXT, METAVAR, VALUE)
 
 #define LLVM_CONSTRUCT_OPT_INFO_WITH_ID_PREFIX(                                \
-    ID_PREFIX, PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-    HELPTEXT, METAVAR, VALUES)                                                 \
+    ID_PREFIX, PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS,       \
+    FLAGS, PARAM, HELPTEXT, METAVAR, VALUES)                                   \
   llvm::opt::OptTable::Info {                                                  \
-    PREFIX, NAME, HELPTEXT, METAVAR, ID_PREFIX##ID,                            \
+    PREFIX, PREFIXED_NAME, HELPTEXT, METAVAR, ID_PREFIX##ID,                   \
         llvm::opt::Option::KIND##Class, PARAM, FLAGS, ID_PREFIX##GROUP,        \
         ID_PREFIX##ALIAS, ALIASARGS, VALUES                                    \
   }
 
-#define LLVM_CONSTRUCT_OPT_INFO(PREFIX, NAME, ID, KIND, GROUP, ALIAS,          \
+#define LLVM_CONSTRUCT_OPT_INFO(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, \
                                 ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR,    \
                                 VALUES)                                        \
-  LLVM_CONSTRUCT_OPT_INFO_WITH_ID_PREFIX(OPT_, PREFIX, NAME, ID, KIND, GROUP,  \
-                                         ALIAS, ALIASARGS, FLAGS, PARAM,       \
-                                         HELPTEXT, METAVAR, VALUES)
+  LLVM_CONSTRUCT_OPT_INFO_WITH_ID_PREFIX(OPT_, PREFIX, PREFIXED_NAME, ID,      \
+                                         KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \
+                                         PARAM, HELPTEXT, METAVAR, VALUES)
 
 #endif // LLVM_OPTION_OPTTABLE_H

diff  --git a/llvm/include/llvm/Option/Option.h b/llvm/include/llvm/Option/Option.h
index 9b35e81cd99144..2460bbedd21f58 100644
--- a/llvm/include/llvm/Option/Option.h
+++ b/llvm/include/llvm/Option/Option.h
@@ -97,7 +97,7 @@ class Option {
   /// Get the name of this option without any prefix.
   StringRef getName() const {
     assert(Info && "Must have a valid info!");
-    return Info->Name;
+    return Info->getName();
   }
 
   const Option getGroup() const {
@@ -130,10 +130,9 @@ class Option {
   }
 
   /// Get the name of this option with the default prefix.
-  std::string getPrefixedName() const {
-    std::string Ret(getPrefix());
-    Ret += getName();
-    return Ret;
+  StringLiteral getPrefixedName() const {
+    assert(Info && "Must have a valid info!");
+    return Info->PrefixedName;
   }
 
   /// Get the help text for this option.

diff  --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 3f53ac119c69e0..3d7bef8af0dbc7 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -59,7 +59,7 @@ static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) {
   if (&A == &B)
     return false;
 
-  if (int N = StrCmpOptionName(A.Name, B.Name))
+  if (int N = StrCmpOptionName(A.getName(), B.getName()))
     return N < 0;
 
   for (size_t I = 0, K = std::min(A.Prefixes.size(), B.Prefixes.size()); I != K;
@@ -77,7 +77,7 @@ static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) {
 
 // Support lower_bound between info and an option name.
 static inline bool operator<(const OptTable::Info &I, StringRef Name) {
-  return StrCmpOptionNameIgnoreCase(I.Name, Name) < 0;
+  return StrCmpOptionNameIgnoreCase(I.getName(), Name) < 0;
 }
 
 } // end namespace opt
@@ -163,10 +163,10 @@ static unsigned matchOption(const OptTable::Info *I, StringRef Str,
   for (auto Prefix : I->Prefixes) {
     if (Str.startswith(Prefix)) {
       StringRef Rest = Str.substr(Prefix.size());
-      bool Matched = IgnoreCase ? Rest.starts_with_insensitive(I->Name)
-                                : Rest.startswith(I->Name);
+      bool Matched = IgnoreCase ? Rest.starts_with_insensitive(I->getName())
+                                : Rest.startswith(I->getName());
       if (Matched)
-        return Prefix.size() + StringRef(I->Name).size();
+        return Prefix.size() + StringRef(I->getName()).size();
     }
   }
   return 0;
@@ -175,8 +175,8 @@ static unsigned matchOption(const OptTable::Info *I, StringRef Str,
 // Returns true if one of the Prefixes + In.Names matches Option
 static bool optionMatches(const OptTable::Info &In, StringRef Option) {
   for (auto Prefix : In.Prefixes)
-    if (Option.endswith(In.Name))
-      if (Option.slice(0, Option.size() - In.Name.size()) == Prefix)
+    if (Option.endswith(In.getName()))
+      if (Option.slice(0, Option.size() - In.getName().size()) == Prefix)
         return true;
   return false;
 }
@@ -215,7 +215,7 @@ OptTable::findByPrefix(StringRef Cur, unsigned int DisableFlags) const {
       continue;
 
     for (auto Prefix : In.Prefixes) {
-      std::string S = (Prefix + In.Name + "\t").str();
+      std::string S = (Prefix + In.getName() + "\t").str();
       if (In.HelpText)
         S += In.HelpText;
       if (StringRef(S).startswith(Cur) && S != std::string(Cur) + "\t")
@@ -240,7 +240,7 @@ unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
 
   for (const Info &CandidateInfo :
        ArrayRef<Info>(OptionInfos).drop_front(FirstSearchableIndex)) {
-    StringRef CandidateName = CandidateInfo.Name;
+    StringRef CandidateName = CandidateInfo.getName();
 
     // We can eliminate some option prefix/name pairs as candidates right away:
     // * Ignore option candidates with empty names, such as "--", or names
@@ -529,7 +529,7 @@ InputArgList OptTable::parseArgs(int Argc, char *const *Argv,
 
 static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) {
   const Option O = Opts.getOption(Id);
-  std::string Name = O.getPrefixedName();
+  std::string Name = O.getPrefixedName().str();
 
   // Add metavar, if used.
   switch (O.getKind()) {

diff  --git a/llvm/unittests/Option/OptionMarshallingTest.cpp b/llvm/unittests/Option/OptionMarshallingTest.cpp
index c4d48acc7a3f9f..d9320950845392 100644
--- a/llvm/unittests/Option/OptionMarshallingTest.cpp
+++ b/llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 struct OptionWithMarshallingInfo {
-  llvm::StringRef Name;
+  llvm::StringLiteral PrefixedName;
   const char *KeyPath;
   const char *ImpliedCheck;
   const char *ImpliedValue;
@@ -18,20 +18,20 @@ struct OptionWithMarshallingInfo {
 
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(                                               \
-    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
-    HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+    PREFIX_TYPE, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,      \
+    PARAM, HELPTEXT, METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,      \
     DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
     MERGER, EXTRACTOR, TABLE_INDEX)                                            \
-  {NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
+  {PREFIXED_NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
 #include "Opts.inc"
 #undef OPTION_WITH_MARSHALLING
 };
 
 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_EQ(MarshallingTable[0].PrefixedName, "-marshalled-flag-d");
+  ASSERT_EQ(MarshallingTable[1].PrefixedName, "-marshalled-flag-c");
+  ASSERT_EQ(MarshallingTable[2].PrefixedName, "-marshalled-flag-b");
+  ASSERT_EQ(MarshallingTable[3].PrefixedName, "-marshalled-flag-a");
 }
 
 TEST(OptionMarshalling, EmittedSpecifiedKeyPath) {

diff  --git a/llvm/utils/TableGen/OptParserEmitter.cpp b/llvm/utils/TableGen/OptParserEmitter.cpp
index a04680b5d91e10..ff9736781ce20a 100644
--- a/llvm/utils/TableGen/OptParserEmitter.cpp
+++ b/llvm/utils/TableGen/OptParserEmitter.cpp
@@ -34,30 +34,14 @@ static raw_ostream &write_cstring(raw_ostream &OS, llvm::StringRef Str) {
   return OS;
 }
 
-static std::string getOptionSpelling(const Record &R, size_t &PrefixLength) {
+static std::string getOptionPrefixedName(const Record &R) {
   std::vector<StringRef> Prefixes = R.getValueAsListOfStrings("Prefixes");
   StringRef Name = R.getValueAsString("Name");
 
-  if (Prefixes.empty()) {
-    PrefixLength = 0;
+  if (Prefixes.empty())
     return Name.str();
-  }
-
-  PrefixLength = Prefixes[0].size();
-  return (Twine(Prefixes[0]) + Twine(Name)).str();
-}
 
-static std::string getOptionSpelling(const Record &R) {
-  size_t PrefixLength;
-  return getOptionSpelling(R, PrefixLength);
-}
-
-static void emitNameUsingSpelling(raw_ostream &OS, const Record &R) {
-  size_t PrefixLength;
-  OS << "llvm::StringLiteral(";
-  write_cstring(
-      OS, StringRef(getOptionSpelling(R, PrefixLength)).substr(PrefixLength));
-  OS << ")";
+  return (Prefixes[0] + Twine(Name)).str();
 }
 
 class MarshallingInfo {
@@ -105,8 +89,6 @@ struct SimpleEnumValueTable {
   }
 
   void emit(raw_ostream &OS) const {
-    write_cstring(OS, StringRef(getOptionSpelling(R)));
-    OS << ", ";
     OS << ShouldParse;
     OS << ", ";
     OS << ShouldAlwaysEmit;
@@ -346,8 +328,8 @@ static void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
     std::vector<StringRef> RPrefixes = R.getValueAsListOfStrings("Prefixes");
     OS << Prefixes[PrefixKeyT(RPrefixes.begin(), RPrefixes.end())] << ", ";
 
-    // The option string.
-    emitNameUsingSpelling(OS, R);
+    // The option prefixed name.
+    write_cstring(OS, getOptionPrefixedName(R));
 
     // The option identifier name.
     OS << ", " << getOptionName(R);


        


More information about the cfe-commits mailing list