[PATCH] D57508: [clangd] Enable include insertion for dynamic index

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clangd/ClangdUnit.cpp:100
+      : File(File), ParsedCallback(ParsedCallback),
+        IWYUHandler(collectIWYUHeaderMaps(&CanonIncludes)) {}
 
----------------
ioeric wrote:
> Does this have to own the `IWYUHandler`? Could we create one when `getCommentHandler` is called?
The class needs to own it, since preprocessor doesn't take the ownership. But we can move the creation to getcommenthandler.


================
Comment at: clangd/ClangdUnit.cpp:125
 
+  CommentHandler *getCommentHandler() override { return IWYUHandler.get(); }
+
----------------
ioeric wrote:
> This looks like a memory leak? `release()`?
Preprocessor doesn't take the ownership, so this seems to be correct ?


================
Comment at: clangd/ClangdUnit.cpp:338
 
+  // Do we really care about IWYU pragmas inside the file rather than the
+  // preamble?
----------------
ioeric wrote:
> I think so? A header file (with IWYU pragma) can also be the main file.
Also call `addSystemHeadersMapping` in case there is no preamble.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508