[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.


================
Comment at: clang-tools-extra/clangd/CollectMacros.cpp:42
+    if (isInsideMainFile(Loc, SM)) {
+      Position Start = sourceLocToPosition(SM, Loc);
+      Position End = {Start.line + 1, 0};
----------------
kadircet wrote:
> are we fine with these annotations spanning `#pragma [[mark XX]]` rather than the whole line or just `XX` ?
I think it's okay for now, I added a FIXME which I'll address in a follow up


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:655
         llvm::iterator_range<const SourceLocation *> Locations) {
-      for (const auto &P : llvm::zip(Protocols, Locations)) {
         Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
----------------
kadircet wrote:
> this change does not look relevant, can you please revert?
Reverted, will send out another change to fix the warning


================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:441
+  if (Preamble)
+    Marks = Preamble->Marks;
+  Clang->getPreprocessor().addPPCallbacks(
----------------
kadircet wrote:
> We are not patching marks for stale preambles, which I believe is fine but worth a FIXME. If you are interested please take a look at https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L427, it should be fairly easy to handle movements of `#pragma mark`s in preamble section.
Added a FIXME. Does something similar need to be done for MainFileMacros?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904