[clang-tools-extra] r340401 - [clangd] Add callbacks on parsed AST in addition to parsed preambles

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 22 04:39:16 PDT 2018


Author: ibiryukov
Date: Wed Aug 22 04:39:16 2018
New Revision: 340401

URL: http://llvm.org/viewvc/llvm-project?rev=340401&view=rev
Log:
[clangd] Add callbacks on parsed AST in addition to parsed preambles

Summary:
Will be used for updating the dynamic index on updates to the open files.
Currently we collect only information coming from the preamble
AST. This has a bunch of limitations:
  - Dynamic index misses important information from the body of the
    file, e.g. locations of definitions.
  - XRefs cannot be collected at all, since we can only obtain full
    information for the current file (preamble is parsed with skipped
    function bodies, therefore not reliable).

This patch only adds the new callback, actually updates to the index
will be done in a follow-up patch.

Reviewers: hokein

Reviewed By: hokein

Subscribers: kadircet, javed.absar, ioeric, MaskRay, jkorous, arphaman, cfe-commits

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

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

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=340401&r1=340400&r2=340401&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Aug 22 04:39:16 2018
@@ -66,6 +66,24 @@ public:
   Optional<Expected<tooling::AtomicChanges>> Result;
 };
 
+class UpdateFileIndex : public ParsingCallbacks {
+public:
+  UpdateFileIndex(FileIndex *FileIdx) : FileIdx(FileIdx) {}
+
+  void onPreambleAST(PathRef Path, ASTContext &Ctx,
+                     std::shared_ptr<clang::Preprocessor> PP) override {
+    if (FileIdx)
+      FileIdx->update(Path, &Ctx, std::move(PP));
+  }
+
+  void onMainAST(PathRef Path, ParsedAST &AST) override {
+    // FIXME: merge results from the main file into the index too.
+  }
+
+private:
+  FileIndex *FileIdx;
+};
+
 } // namespace
 
 ClangdServer::Options ClangdServer::optsForTest() {
@@ -85,20 +103,16 @@ ClangdServer::ClangdServer(GlobalCompila
                                    : getStandardResourceDir()),
       FileIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex(Opts.URISchemes)
                                            : nullptr),
+      FileIdxUpdater(llvm::make_unique<UpdateFileIndex>(FileIdx.get())),
       PCHs(std::make_shared<PCHContainerOperations>()),
       // Pass a callback into `WorkScheduler` to extract symbols from a newly
       // parsed file and rebuild the file index synchronously each time an AST
       // is parsed.
       // FIXME(ioeric): this can be slow and we may be able to index on less
       // critical paths.
-      WorkScheduler(
-          Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-          FileIdx
-              ? [this](PathRef Path, ASTContext &AST,
-                       std::shared_ptr<Preprocessor>
-                           PP) { FileIdx->update(Path, &AST, std::move(PP)); }
-              : PreambleParsedCallback(),
-          Opts.UpdateDebounce, Opts.RetentionPolicy) {
+      WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+                    *FileIdxUpdater, Opts.UpdateDebounce,
+                    Opts.RetentionPolicy) {
   if (FileIdx && Opts.StaticIndex) {
     MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
     Index = MergedIndex.get();

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=340401&r1=340400&r2=340401&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Aug 22 04:39:16 2018
@@ -223,6 +223,8 @@ private:
   SymbolIndex *Index;
   // If present, an up-to-date of symbols in open files. Read via Index.
   std::unique_ptr<FileIndex> FileIdx;
+  /// Callbacks responsible for updating FileIdx.
+  std::unique_ptr<ParsingCallbacks> FileIdxUpdater;
   // If present, a merged view of FileIdx and an external index. Read via Index.
   std::unique_ptr<SymbolIndex> MergedIndex;
   // If set, this represents the workspace path.

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=340401&r1=340400&r2=340401&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Wed Aug 22 04:39:16 2018
@@ -158,8 +158,7 @@ class ASTWorker {
             Semaphore &Barrier, bool RunSync,
             steady_clock::duration UpdateDebounce,
             std::shared_ptr<PCHContainerOperations> PCHs,
-            bool StorePreamblesInMemory,
-            PreambleParsedCallback PreambleCallback);
+            bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
 
 public:
   /// Create a new ASTWorker and return a handle to it.
@@ -173,7 +172,7 @@ public:
                                 steady_clock::duration UpdateDebounce,
                                 std::shared_ptr<PCHContainerOperations> PCHs,
                                 bool StorePreamblesInMemory,
-                                PreambleParsedCallback PreambleCallback);
+                                ParsingCallbacks &Callbacks);
   ~ASTWorker();
 
   void update(ParseInputs Inputs, WantDiagnostics,
@@ -228,8 +227,8 @@ private:
   const Path FileName;
   /// Whether to keep the built preambles in memory or on disk.
   const bool StorePreambleInMemory;
-  /// Callback, passed to the preamble builder.
-  const PreambleParsedCallback PreambleCallback;
+  /// Callback, invoked when preamble or main file AST is built.
+  ParsingCallbacks &Callbacks;
   /// Helper class required to build the ASTs.
   const std::shared_ptr<PCHContainerOperations> PCHs;
 
@@ -299,10 +298,10 @@ ASTWorkerHandle ASTWorker::create(PathRe
                                   steady_clock::duration UpdateDebounce,
                                   std::shared_ptr<PCHContainerOperations> PCHs,
                                   bool StorePreamblesInMemory,
-                                  PreambleParsedCallback PreambleCallback) {
+                                  ParsingCallbacks &Callbacks) {
   std::shared_ptr<ASTWorker> Worker(new ASTWorker(
       FileName, IdleASTs, Barrier, /*RunSync=*/!Tasks, UpdateDebounce,
-      std::move(PCHs), StorePreamblesInMemory, std::move(PreambleCallback)));
+      std::move(PCHs), StorePreamblesInMemory, Callbacks));
   if (Tasks)
     Tasks->runAsync("worker:" + llvm::sys::path::filename(FileName),
                     [Worker]() { Worker->run(); });
@@ -314,12 +313,11 @@ ASTWorker::ASTWorker(PathRef FileName, T
                      Semaphore &Barrier, bool RunSync,
                      steady_clock::duration UpdateDebounce,
                      std::shared_ptr<PCHContainerOperations> PCHs,
-                     bool StorePreamblesInMemory,
-                     PreambleParsedCallback PreambleCallback)
+                     bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
     : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
       FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory),
-      PreambleCallback(std::move(PreambleCallback)), PCHs(std::move(PCHs)),
-      Barrier(Barrier), Done(false) {}
+      Callbacks(Callbacks), PCHs(std::move(PCHs)), Barrier(Barrier),
+      Done(false) {}
 
 ASTWorker::~ASTWorker() {
   // Make sure we remove the cached AST, if any.
@@ -365,7 +363,11 @@ void ASTWorker::update(
         getPossiblyStalePreamble();
     std::shared_ptr<const PreambleData> NewPreamble =
         buildPreamble(FileName, *Invocation, OldPreamble, OldCommand, Inputs,
-                      PCHs, StorePreambleInMemory, PreambleCallback);
+                      PCHs, StorePreambleInMemory,
+                      [this](PathRef Path, ASTContext &Ctx,
+                             std::shared_ptr<clang::Preprocessor> PP) {
+                        Callbacks.onPreambleAST(FileName, Ctx, std::move(PP));
+                      });
 
     bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
     {
@@ -415,6 +417,7 @@ void ASTWorker::update(
     // Note *AST can be still be null if buildAST fails.
     if (*AST) {
       OnUpdated((*AST)->getDiagnostics());
+      Callbacks.onMainAST(FileName, **AST);
       DiagsWereReported = true;
     }
     // Stash the AST in the cache for further use.
@@ -618,6 +621,11 @@ unsigned getDefaultAsyncThreadsCount() {
   return HardwareConcurrency;
 }
 
+ParsingCallbacks &noopParsingCallbacks() {
+  static ParsingCallbacks *Instance = new ParsingCallbacks;
+  return *Instance;
+}
+
 struct TUScheduler::FileData {
   /// Latest inputs, passed to TUScheduler::update().
   std::string Contents;
@@ -627,12 +635,12 @@ struct TUScheduler::FileData {
 
 TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
                          bool StorePreamblesInMemory,
-                         PreambleParsedCallback PreambleCallback,
+                         ParsingCallbacks &Callbacks,
                          std::chrono::steady_clock::duration UpdateDebounce,
                          ASTRetentionPolicy RetentionPolicy)
     : StorePreamblesInMemory(StorePreamblesInMemory),
-      PCHOps(std::make_shared<PCHContainerOperations>()),
-      PreambleCallback(std::move(PreambleCallback)), Barrier(AsyncThreadsCount),
+      PCHOps(std::make_shared<PCHContainerOperations>()), Callbacks(Callbacks),
+      Barrier(AsyncThreadsCount),
       IdleASTs(llvm::make_unique<ASTCache>(RetentionPolicy.MaxRetainedASTs)),
       UpdateDebounce(UpdateDebounce) {
   if (0 < AsyncThreadsCount) {
@@ -670,8 +678,7 @@ void TUScheduler::update(
     // Create a new worker to process the AST-related tasks.
     ASTWorkerHandle Worker = ASTWorker::create(
         File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
-        Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory,
-        PreambleCallback);
+        Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, Callbacks);
     FD = std::unique_ptr<FileData>(new FileData{
         Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
   } else {

Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=340401&r1=340400&r2=340401&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Wed Aug 22 04:39:16 2018
@@ -51,6 +51,30 @@ struct ASTRetentionPolicy {
   unsigned MaxRetainedASTs = 3;
 };
 
+class ParsingCallbacks {
+public:
+  virtual ~ParsingCallbacks() = default;
+
+  /// Called on the AST that was built for emitting the preamble. The built AST
+  /// contains only AST nodes from the #include directives at the start of the
+  /// file. AST node in the current file should be observed on onMainAST call.
+  virtual void onPreambleAST(PathRef Path, ASTContext &Ctx,
+                             std::shared_ptr<clang::Preprocessor> PP) {}
+  /// Called on the AST built for the file itself. Note that preamble AST nodes
+  /// are not deserialized and should be processed in the onPreambleAST call
+  /// instead.
+  /// The \p AST always contains all AST nodes for the main file itself, and
+  /// only a portion of the AST nodes deserialized from the preamble. Note that
+  /// some nodes from the preamble may have been deserialized and may also be
+  /// accessed from the main file AST, e.g. redecls of functions from preamble,
+  /// etc. Clients are expected to process only the AST nodes from the main file
+  /// in this callback (obtained via ParsedAST::getLocalTopLevelDecls) to obtain
+  /// optimal performance.
+  virtual void onMainAST(PathRef Path, ParsedAST &AST) {}
+};
+
+ParsingCallbacks &noopParsingCallbacks();
+
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -61,7 +85,7 @@ struct ASTRetentionPolicy {
 class TUScheduler {
 public:
   TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
-              PreambleParsedCallback PreambleCallback,
+              ParsingCallbacks &ASTCallbacks,
               std::chrono::steady_clock::duration UpdateDebounce,
               ASTRetentionPolicy RetentionPolicy);
   ~TUScheduler();
@@ -132,7 +156,7 @@ public:
 private:
   const bool StorePreamblesInMemory;
   const std::shared_ptr<PCHContainerOperations> PCHOps;
-  const PreambleParsedCallback PreambleCallback;
+  ParsingCallbacks &Callbacks;
   Semaphore Barrier;
   llvm::StringMap<std::unique_ptr<FileData>> Files;
   std::unique_ptr<ASTCache> IdleASTs;

Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=340401&r1=340400&r2=340401&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Wed Aug 22 04:39:16 2018
@@ -17,10 +17,11 @@
 
 namespace clang {
 namespace clangd {
+namespace {
 
 using ::testing::_;
-using ::testing::Each;
 using ::testing::AnyOf;
+using ::testing::Each;
 using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
@@ -44,8 +45,7 @@ protected:
 
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
-                /*StorePreamblesInMemory=*/true,
-                /*PreambleParsedCallback=*/nullptr,
+                /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
 
@@ -101,8 +101,7 @@ TEST_F(TUSchedulerTests, WantDiagnostics
     Notification Ready;
     TUScheduler S(
         getDefaultAsyncThreadsCount(),
-        /*StorePreamblesInMemory=*/true,
-        /*PreambleParsedCallback=*/nullptr,
+        /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
     auto Path = testPath("foo.cpp");
@@ -130,8 +129,7 @@ TEST_F(TUSchedulerTests, Debounce) {
   std::atomic<int> CallbackCount(0);
   {
     TUScheduler S(getDefaultAsyncThreadsCount(),
-                  /*StorePreamblesInMemory=*/true,
-                  /*PreambleParsedCallback=*/nullptr,
+                  /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
                   /*UpdateDebounce=*/std::chrono::seconds(1),
                   ASTRetentionPolicy());
     // FIXME: we could probably use timeouts lower than 1 second here.
@@ -162,8 +160,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
   // Run TUScheduler and collect some stats.
   {
     TUScheduler S(getDefaultAsyncThreadsCount(),
-                  /*StorePreamblesInMemory=*/true,
-                  /*PreambleParsedCallback=*/nullptr,
+                  /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
                   /*UpdateDebounce=*/std::chrono::milliseconds(50),
                   ASTRetentionPolicy());
 
@@ -261,7 +258,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
   Policy.MaxRetainedASTs = 2;
   TUScheduler S(
       /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
-      PreambleParsedCallback(),
+      noopParsingCallbacks(),
       /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
@@ -311,7 +308,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
 TEST_F(TUSchedulerTests, EmptyPreamble) {
   TUScheduler S(
       /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-      PreambleParsedCallback(),
+      noopParsingCallbacks(),
       /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
       ASTRetentionPolicy());
 
@@ -358,7 +355,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPrea
   // the same time. All reads should get the same non-null preamble.
   TUScheduler S(
       /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-      PreambleParsedCallback(),
+      noopParsingCallbacks(),
       /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
       ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
@@ -391,7 +388,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPrea
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
   TUScheduler S(
       /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-      /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+      /*StorePreambleInMemory=*/true, noopParsingCallbacks(),
       /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
       ASTRetentionPolicy());
 
@@ -444,7 +441,7 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChan
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   TUScheduler S(
       /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-      /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+      /*StorePreambleInMemory=*/true, noopParsingCallbacks(),
       /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
       ASTRetentionPolicy());
 
@@ -475,5 +472,6 @@ TEST_F(TUSchedulerTests, NoChangeDiags)
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
 }
 
+} // namespace
 } // namespace clangd
 } // namespace clang




More information about the cfe-commits mailing list