[PATCH] D50847: [clangd] Add callbacks on parsed AST in addition to parsed preambles

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks! Looks good.



================
Comment at: clangd/ClangdServer.cpp:73
+
+  void onPreambleAST(PathRef Path, ASTContext &Ctx,
+                     std::shared_ptr<clang::Preprocessor> PP) override {
----------------
ilya-biryukov wrote:
> hokein wrote:
> > I think `Ctx` should be a `pointer` which gives us a way (passing a nullptr) to clean up the `FileIndex`, the same as `ParsedAST` below.
> > 
> > And I discover that we don't cleanup dynamic index when a file is being closed, is it expected or a bug?
> I suggest we add an extra method to `DynamicIndex` that we call when the file is closed. I don't think it's intentional that we don't clean up the index for the closed files.
> Not sure what's the best way to handle invalid ASTs, but we're never calling the current callback with `nullptr` anyway, so I suggest we keep it a reference to better reflect that callbacks don't actually handle any errors for us.
SG, and it is out of scope of this patch. Let's figure it out in the clean-up-index patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50847