[clang-tools-extra] 8f79203 - [clangd] New ParsingCallback for semantics changes

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-05-26T16:57:30+02:00
New Revision: 8f79203a22d8e04086f4cc9a58bb365148852a09

URL: https://github.com/llvm/llvm-project/commit/8f79203a22d8e04086f4cc9a58bb365148852a09
DIFF: https://github.com/llvm/llvm-project/commit/8f79203a22d8e04086f4cc9a58bb365148852a09.diff

LOG: [clangd] New ParsingCallback for semantics changes

Previously notification of the Server about semantic happened strictly
before notification of the AST thread.
Hence a racy Server could make a request (like semantic tokens) after
the notification, with the assumption that it'll be served fresh
content. But it wasn't true if AST thread wasn't notified about the
change yet.

This change reverses the order of those notifications to prevent racy
interactions.

Differential Revision: https://reviews.llvm.org/D102761

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdServer.cpp
    clang-tools-extra/clangd/TUScheduler.cpp
    clang-tools-extra/clangd/TUScheduler.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 3f8b95b9104ce..0f525f3b9a0a4 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -75,8 +75,6 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
                      const CanonicalIncludes &CanonIncludes) override {
     if (FIndex)
       FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes);
-    if (ServerCallbacks)
-      ServerCallbacks->onSemanticsMaybeChanged(Path);
   }
 
   void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
@@ -105,6 +103,11 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
       ServerCallbacks->onFileUpdated(File, Status);
   }
 
+  void onPreamblePublished(PathRef File) override {
+    if (ServerCallbacks)
+      ServerCallbacks->onSemanticsMaybeChanged(File);
+  }
+
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index 435deb6ec162a..7498598f124fb 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -909,6 +909,7 @@ void PreambleThread::build(Request Req) {
     ASTPeer.updatePreamble(std::move(Req.CI), std::move(Req.Inputs),
                            LatestBuild, std::move(Req.CIDiags),
                            std::move(Req.WantDiags));
+    Callbacks.onPreamblePublished(FileName);
   });
 
   if (!LatestBuild || Inputs.ForceRebuild) {

diff  --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h
index 8df2c019053e0..4731443d3d51b 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -169,6 +169,11 @@ class ParsingCallbacks {
 
   /// Called whenever the TU status is updated.
   virtual void onFileUpdated(PathRef File, const TUStatus &Status) {}
+
+  /// Preamble for the TU have changed. This might imply new semantics (e.g.
+  /// 
diff erent highlightings). Any actions on the file are guranteed to see new
+  /// preamble after the callback.
+  virtual void onPreamblePublished(PathRef File) {}
 };
 
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,