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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
