[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 6 11:57:54 PDT 2020


kadircet marked an inline comment as done.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:224
+              std::vector<Diag> CIDiags, WantDiagnostics WantDiags) {
     // Make possibly expensive copy while not holding the lock.
+    Request Req = {std::move(CI), std::move(PI), std::move(CIDiags), WantDiags,
----------------
sammccall wrote:
> comment is now obsolete, remove it and consider inlining `Req`
i would rather keep it out-of-line since it is used in two places.


================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:325
   mutable std::condition_variable ReqCV;           /* GUARDED_BY(Mutex) */
   std::shared_ptr<const PreambleData> LatestBuild; /* GUARDED_BY(Mutex) */
 
----------------
sammccall wrote:
> this is no longer locked or concurrently accessed right?
right, deleted annottation and added a comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76725/new/

https://reviews.llvm.org/D76725





More information about the cfe-commits mailing list