ilya-biryukov added inline comments.
================
Comment at: clangd/ClangdUnit.cpp:714
-void CppFile::cancelRebuilds() {
+void CppFile::cancelRebuild() { deferCancelRebuild().get(); }
+
----------------
klimek wrote:
> Somewhat orthogonal question - why CppFile? We do support other languages in
> Clangd, right? (C / Obj-C)
> I'd have expected this to be called ClangdUnit, especially given the file
> name.
Just wanted to give it a different name when the class changed semantics.
Didn't rename the file yet, though, because was afraid that git would not
detect renames and we'll lose history.
ClangdUnit used to provide implementations for all features (codeComplete,
etc.), CppFile manages AST and Preamble., i.e. doing a different thing.
But I agree, CppFile is a probably a bad name, but I would still go with
something not ending with Unit to not bring up parallels with ASTUnit, that
provides a very different interface.
How about ClangdFile, ClangdFileAST?
================
Comment at: clangd/ClangdUnit.cpp:735
+
+ std::shared_ptr<CppFile> That = shared_from_this();
+ return std::async(std::launch::deferred, [That, RequestRebuildCounter]() {
----------------
klimek wrote:
> Now I know why I don't like the shard_ptr, this is all pretty awkward.
Do you mean `shared_from_this()` specifically?
================
Comment at: clangd/ClangdUnit.cpp:738
+ std::unique_lock<std::mutex> Lock(That->Mutex);
+ CppFile *This = &*That;
+ This->RebuildCond.wait(Lock, [This, RequestRebuildCounter]() {
----------------
klimek wrote:
> Why do we need that instead of just calling the bound shared_ptr This?
This lambda is a deferred computation and might get executed on a different
thread, so we want to keep a `shared_ptr` to our class around (`weak_ptr` would
also work) to make sure this class does not get deleted before this lambda is
called.
`This` local var is for capture in a lambda that is passed to `wait` in the
code below (we could've captured `That`, but that would be an extra
`shared_ptr` copy).
https://reviews.llvm.org/D36397
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
