[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:
> 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.
LG, I'll come up with a follow-up cleanup patch when done with these patches


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50847