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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 4 05:26:50 PST 2019

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.

  rCTE Clang Tools Extra



More information about the cfe-commits mailing list