[PATCH] D114072: [clangd] Record IWYU pragma keep in the IncludeStructure

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry for forgetting about this one. Hopefully I can still help now by disagreeing with Kadir and creating an awkward stalemate instead.

I agree that the ownership situation is ugly, but I think a single object here is still simpler. Partly because the ordering isn't some weird coincidence (see below)



================
Comment at: clang-tools-extra/clangd/Headers.cpp:103
 
+  // Because PP wants to own the PPCallbacks, CommentHandler will be added first
+  // and will also be called before handling the include directive. Example:
----------------
I don't think this has anything to do with ownership or registration order, rather events get recorded in order that constructs *finish* rather than start.

The PP doesn't call InclusionDirective until it sees the end of the line, and it has to skip over the comment to get there.

(I like the example and the sequencing, I'd just replace the first paragraph with "Given:" or so)


================
Comment at: clang-tools-extra/clangd/Headers.h:130
+  // IWYU pragmas but it needs to be explicitly added to as a preprocessor
+  // comment handler. PP wants to own the PPCallbacks, so the typical way to
+  // do both is:
----------------
Alternatively we could just give up on having a pure-ish API and say `void collect(Preprocessor&)`.
Again there's not really any other sensible way to use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114072