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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdServer.cpp:73
+
+  void onPreambleAST(PathRef Path, ASTContext &Ctx,
+                     std::shared_ptr<clang::Preprocessor> PP) override {
----------------
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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50847