[PATCH] D43065: [clangd] Remove threading-related code from ClangdUnit.h

[Eric Liu via Phabricator via cfe-commits]

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