[Lldb-commits] [PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[serge via Phabricator via lldb-commits]

serge-sans-paille added inline comments.


================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
     const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
----------------
kadircet wrote:
> this is actually an incorrect change (even builds shouldn't be succeeding). as the values here can also be `nullptr` (which cannot be stored in a StringLiteral) but moreover we later on assign these to `Prefixes` array, which is of type `char*`, hence the conversion should also be failing.
> 
> but in general i'd actually expect people to be assigning "nullptr"s to these `char*`s, hence if this was a purely mechanical migration without some extra constraints, it might still blow up at runtime even if it succeeds compiles.
Builds (and test suite) all succeed (at least locally :-)). This patch also modifies tablegen to *not* generate `nullptr`, but empty string instead. The constructor of `OptTable::Info` has been modified to turn these "Empty string terminated array" into regular `ArrayRef`, which makes iteration code much more straight forward.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139881/new/

https://reviews.llvm.org/D139881