ioeric added a comment.
Nice! The code looks much simpler!
Just some drive-by nits. I don't know the threading work well enough to give
useful comments. Will leave the approval to others.
================
Comment at: clangd/ClangdUnit.cpp:399
+ std::unique_ptr<CompilerInvocation> CI;
+ {
+ // FIXME(ibiryukov): store diagnostics from CommandLine when we start
----------------
Do we still need this block?
================
Comment at: clangd/ClangdUnit.cpp:434
+ // Collect diagnostics from both the preamble and the AST.
if (NewPreamble) {
Diagnostics.insert(Diagnostics.begin(), NewPreamble->Diags.begin(),
----------------
nit: no braces
================
Comment at: clangd/ClangdUnit.h:161
- /// Mutex protects all fields, declared below it, FileName and Command are
not
- /// mutated.
- mutable std::mutex Mutex;
- /// A counter to cancel old rebuilds.
- unsigned RebuildCounter;
- /// Used to wait when rebuild is finished before starting another one.
- bool RebuildInProgress;
- /// Condition variable to indicate changes to RebuildInProgress.
- std::condition_variable RebuildCond;
-
- /// Size of the last built AST, in bytes.
- std::size_t ASTMemUsage;
- /// Size of the last build Preamble, in bytes.
- std::size_t PreambleMemUsage;
-
- /// Promise and future for the latests AST. Fulfilled during rebuild.
- /// We use std::shared_ptr here because MVSC fails to compile non-copyable
- /// classes as template arguments of promise/future.
- std::promise<std::shared_ptr<ParsedASTWrapper>> ASTPromise;
- std::shared_future<std::shared_ptr<ParsedASTWrapper>> ASTFuture;
-
- /// Promise and future for the latests Preamble. Fulfilled during rebuild.
- std::promise<std::shared_ptr<const PreambleData>> PreamblePromise;
- std::shared_future<std::shared_ptr<const PreambleData>> PreambleFuture;
- /// Latest preamble that was built. May be stale, but always available
without
- /// waiting for rebuild to finish.
- std::shared_ptr<const PreambleData> LatestAvailablePreamble;
- /// Utility class, required by clang.
+ /// The latest parsed AST.
+ llvm::Optional<ParsedAST> AST;
----------------
`s/latest/last/`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43065
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
