kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:469
// Also grab prefixes for each option, these are not fully exposed.
- const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+ llvm::StringLiteral Prefixes[DriverID::LastOption] = {};
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] =
VALUE;
----------------
this is still wrong semantically (and i believe are failing tests/builds, you
can see it in the premerge bot builds for yourself
https://buildkite.com/llvm-project/premerge-checks/builds/126513#018510e5-592b-453e-a213-a2cffc9c9ac2).
This was an array of pointers to null-terminated string blocks. Now you're
turning it into just an array of strings.
IIUC, your intention is to change a list of strings terminated with a nullptr
into an arrayref. so this should itself be an `ArrayRef<StringLiteral>`s.
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
+ llvm::StringLiteral Prefixes[DriverID::LastOption] = {};
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] =
VALUE;
#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,
\
----------------
why not directly store ArrayRef's here? instead of an empty string terminated
array? (same for rest of the patch)
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:502
for (unsigned A = ID; A != DriverID::OPT_INVALID; A = NextAlias[A]) {
- if (Prefixes[A] == nullptr) // option groups.
+ if (Prefixes[A].empty()) // option groups.
continue;
----------------
previously this was checking for an empty array, now it's checking for an empty
string. so semantics are completely diffrenet.
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:511
// Iterate over each spelling of the alias, e.g. -foo vs --foo.
- for (auto *Prefix = Prefixes[A]; *Prefix != nullptr; ++Prefix) {
- llvm::SmallString<64> Buf(*Prefix);
+ for (StringRef Prefix : ArrayRef<StringRef>(Prefixes,
DriverID::LastOption - 1) ) {
+ llvm::SmallString<64> Buf(Prefix);
----------------
not even sure what was your intention here, but this was previously iterating
over all the possible alternatives of a prefix until it hit the nullptr
terminator. now you're making it iterate over something completely different
(which i don't know how to describe).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139881/new/
https://reviews.llvm.org/D139881
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
