[clang-tools-extra] r347297 - [clangd] Allow observation of changes to global CDBs.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 20 02:56:04 PST 2018


Author: sammccall
Date: Tue Nov 20 02:56:03 2018
New Revision: 347297

URL: http://llvm.org/viewvc/llvm-project?rev=347297&view=rev
Log:
[clangd] Allow observation of changes to global CDBs.

Summary:
Currently, changes *within* CDBs are not tracked (CDB has no facility to do so).
However, discovery of new CDBs are tracked (all files are marked as modified).
Also, files whose compilation commands are explicitly set are marked modified.

The intent is to use this for auto-index. Newly discovered files will be indexed
with low priority.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Added:
    clang-tools-extra/trunk/unittests/clangd/FunctionTests.cpp
Modified:
    clang-tools-extra/trunk/clangd/Function.h
    clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
    clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
    clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
    clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp

Modified: clang-tools-extra/trunk/clangd/Function.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Function.h?rev=347297&r1=347296&r2=347297&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Function.h (original)
+++ clang-tools-extra/trunk/clangd/Function.h Tue Nov 20 02:56:03 2018
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/Support/Error.h"
+#include <mutex>
 #include <tuple>
 #include <utility>
 
@@ -83,6 +84,82 @@ ForwardBinder<Func, Args...> Bind(Func F
       std::make_tuple(std::forward<Func>(F), std::forward<Args>(As)...));
 }
 
+/// An Event<T> allows events of type T to be broadcast to listeners.
+template <typename T> class Event {
+public:
+  // A Listener is the callback through which events are delivered.
+  using Listener = std::function<void(const T &)>;
+
+  // A subscription defines the scope of when a listener should receive events.
+  // After destroying the subscription, no more events are received.
+  class LLVM_NODISCARD Subscription {
+    Event *Parent;
+    unsigned ListenerID;
+
+    Subscription(Event *Parent, unsigned ListenerID)
+        : Parent(Parent), ListenerID(ListenerID) {}
+    friend Event;
+
+  public:
+    Subscription() : Parent(nullptr) {}
+    Subscription(Subscription &&Other) : Parent(nullptr) {
+      *this = std::move(Other);
+    }
+    Subscription &operator=(Subscription &&Other) {
+      // If *this is active, unsubscribe.
+      if (Parent) {
+        std::lock_guard<std::recursive_mutex>(Parent->ListenersMu);
+        llvm::erase_if(Parent->Listeners,
+                       [&](const std::pair<Listener, unsigned> &P) {
+                         return P.second == ListenerID;
+                       });
+      }
+      // Take over the other subscription, and mark it inactive.
+      std::tie(Parent, ListenerID) = std::tie(Other.Parent, Other.ListenerID);
+      Other.Parent = nullptr;
+      return *this;
+    }
+    // Destroying a subscription may block if an event is being broadcast.
+    ~Subscription() {
+      if (Parent)
+        *this = Subscription(); // Unsubscribe.
+    }
+  };
+
+  // Adds a listener that will observe all future events until the returned
+  // subscription is destroyed.
+  // May block if an event is currently being broadcast.
+  Subscription observe(Listener L) {
+    std::lock_guard<std::recursive_mutex> Lock(ListenersMu);
+    Listeners.push_back({std::move(L), ++ListenerCount});
+    return Subscription(this, ListenerCount);
+    ;
+  }
+
+  // Synchronously sends an event to all registered listeners.
+  // Must not be called from a listener to this event.
+  void broadcast(const T &V) {
+    // FIXME: it would be nice to dynamically check non-reentrancy here.
+    std::lock_guard<std::recursive_mutex> Lock(ListenersMu);
+    for (const auto &L : Listeners)
+      L.first(V);
+  }
+
+  ~Event() {
+    std::lock_guard<std::recursive_mutex> Lock(ListenersMu);
+    assert(Listeners.empty());
+  }
+
+private:
+  static_assert(std::is_same<typename std::decay<T>::type, T>::value,
+                "use a plain type: event values are always passed by const&");
+
+  std::recursive_mutex ListenersMu;
+  bool IsBroadcasting = false;
+  std::vector<std::pair<Listener, unsigned>> Listeners;
+  unsigned ListenerCount = 0;
+};
+
 } // namespace clangd
 } // namespace clang
 

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=347297&r1=347296&r2=347297&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Tue Nov 20 02:56:03 2018
@@ -49,17 +49,17 @@ DirectoryBasedGlobalCompilationDatabase:
   return None;
 }
 
-tooling::CompilationDatabase *
+std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
 DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
   // FIXME(ibiryukov): Invalidate cached compilation databases on changes
   auto CachedIt = CompilationDatabases.find(Dir);
   if (CachedIt != CompilationDatabases.end())
-    return CachedIt->second.get();
+    return {CachedIt->second.get(), true};
   std::string Error = "";
   auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
   auto Result = CDB.get();
   CompilationDatabases.insert(std::make_pair(Dir, std::move(CDB)));
-  return Result;
+  return {Result, false};
 }
 
 tooling::CompilationDatabase *
@@ -69,14 +69,29 @@ DirectoryBasedGlobalCompilationDatabase:
           path::is_absolute(File, path::Style::windows)) &&
          "path must be absolute");
 
+  tooling::CompilationDatabase *CDB = nullptr;
+  bool Cached = false;
   std::lock_guard<std::mutex> Lock(Mutex);
-  if (CompileCommandsDir)
-    return getCDBInDirLocked(*CompileCommandsDir);
-  for (auto Path = path::parent_path(File); !Path.empty();
-       Path = path::parent_path(Path))
-    if (auto CDB = getCDBInDirLocked(Path))
-      return CDB;
-  return nullptr;
+  if (CompileCommandsDir) {
+    std::tie(CDB, Cached) = getCDBInDirLocked(*CompileCommandsDir);
+  } else {
+    for (auto Path = path::parent_path(File); !CDB && !Path.empty();
+         Path = path::parent_path(Path)) {
+      std::tie(CDB, Cached) = getCDBInDirLocked(Path);
+    }
+  }
+  if (CDB && !Cached)
+    OnCommandChanged.broadcast(CDB->getAllFiles());
+  return CDB;
+}
+
+OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
+                       std::vector<std::string> FallbackFlags)
+    : Base(Base), FallbackFlags(std::move(FallbackFlags)) {
+  if (Base)
+    BaseChanged = Base->watch([this](const std::vector<std::string> Changes) {
+      OnCommandChanged.broadcast(Changes);
+    });
 }
 
 Optional<tooling::CompileCommand>
@@ -101,11 +116,14 @@ tooling::CompileCommand OverlayCDB::getF
 
 void OverlayCDB::setCompileCommand(
     PathRef File, llvm::Optional<tooling::CompileCommand> Cmd) {
-  std::unique_lock<std::mutex> Lock(Mutex);
-  if (Cmd)
-    Commands[File] = std::move(*Cmd);
-  else
-    Commands.erase(File);
+  {
+    std::unique_lock<std::mutex> Lock(Mutex);
+    if (Cmd)
+      Commands[File] = std::move(*Cmd);
+    else
+      Commands.erase(File);
+  }
+  OnCommandChanged.broadcast({File});
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h?rev=347297&r1=347296&r2=347297&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h Tue Nov 20 02:56:03 2018
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H
 
+#include "Function.h"
 #include "Path.h"
 #include "llvm/ADT/StringMap.h"
 #include <memory>
@@ -41,8 +42,15 @@ public:
   /// Clangd should treat the results as unreliable.
   virtual tooling::CompileCommand getFallbackCommand(PathRef File) const;
 
-  /// FIXME(ibiryukov): add facilities to track changes to compilation flags of
-  /// existing targets.
+  using CommandChanged = Event<std::vector<std::string>>;
+  /// The callback is notified when files may have new compile commands.
+  /// The argument is a list of full file paths.
+  CommandChanged::Subscription watch(CommandChanged::Listener L) const {
+    return OnCommandChanged.observe(std::move(L));
+  }
+
+protected:
+  mutable CommandChanged OnCommandChanged;
 };
 
 /// Gets compile args from tooling::CompilationDatabases built for parent
@@ -61,7 +69,8 @@ public:
 
 private:
   tooling::CompilationDatabase *getCDBForFile(PathRef File) const;
-  tooling::CompilationDatabase *getCDBInDirLocked(PathRef File) const;
+  std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
+  getCDBInDirLocked(PathRef File) const;
 
   mutable std::mutex Mutex;
   /// Caches compilation databases loaded from directories(keys are
@@ -81,8 +90,7 @@ public:
   // Base may be null, in which case no entries are inherited.
   // FallbackFlags are added to the fallback compile command.
   OverlayCDB(const GlobalCompilationDatabase *Base,
-             std::vector<std::string> FallbackFlags = {})
-      : Base(Base), FallbackFlags(std::move(FallbackFlags)) {}
+             std::vector<std::string> FallbackFlags = {});
 
   llvm::Optional<tooling::CompileCommand>
   getCompileCommand(PathRef File) const override;
@@ -98,6 +106,7 @@ private:
   llvm::StringMap<tooling::CompileCommand> Commands; /* GUARDED_BY(Mut) */
   const GlobalCompilationDatabase *Base;
   std::vector<std::string> FallbackFlags;
+  CommandChanged::Subscription BaseChanged;
 };
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=347297&r1=347296&r2=347297&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Tue Nov 20 02:56:03 2018
@@ -23,6 +23,7 @@ add_extra_unittest(ClangdTests
   FileIndexTests.cpp
   FindSymbolsTests.cpp
   FSTests.cpp
+  FunctionTests.cpp
   FuzzyMatchTests.cpp
   GlobalCompilationDatabaseTests.cpp
   HeadersTests.cpp

Added: clang-tools-extra/trunk/unittests/clangd/FunctionTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FunctionTests.cpp?rev=347297&view=auto
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/FunctionTests.cpp (added)
+++ clang-tools-extra/trunk/unittests/clangd/FunctionTests.cpp Tue Nov 20 02:56:03 2018
@@ -0,0 +1,53 @@
+//===-- FunctionsTests.cpp ------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Function.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(EventTest, Subscriptions) {
+  Event<int> E;
+  int N = 0;
+  {
+    Event<int>::Subscription SubA;
+    // No subscriptions are active.
+    E.broadcast(42);
+    EXPECT_EQ(0, N);
+
+    Event<int>::Subscription SubB = E.observe([&](int) { ++N; });
+    // Now one is active.
+    E.broadcast(42);
+    EXPECT_EQ(1, N);
+
+    SubA = E.observe([&](int) { ++N; });
+    // Both are active.
+    EXPECT_EQ(1, N);
+    E.broadcast(42);
+    EXPECT_EQ(3, N);
+
+    SubA = std::move(SubB);
+    // One is active.
+    EXPECT_EQ(3, N);
+    E.broadcast(42);
+    EXPECT_EQ(4, N);
+  }
+  // None are active.
+  EXPECT_EQ(4, N);
+  E.broadcast(42);
+  EXPECT_EQ(4, N);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

Modified: clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp?rev=347297&r1=347296&r2=347297&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp Tue Nov 20 02:56:03 2018
@@ -88,6 +88,23 @@ TEST_F(OverlayCDBTest, NoBase) {
               ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
 }
 
+TEST_F(OverlayCDBTest, Watch) {
+  OverlayCDB Inner(nullptr);
+  OverlayCDB Outer(&Inner);
+
+  std::vector<std::vector<std::string>> Changes;
+  auto Sub = Outer.watch([&](const std::vector<std::string> &ChangedFiles) {
+    Changes.push_back(ChangedFiles);
+  });
+
+  Inner.setCompileCommand("A.cpp", tooling::CompileCommand());
+  Outer.setCompileCommand("B.cpp", tooling::CompileCommand());
+  Inner.setCompileCommand("A.cpp", llvm::None);
+  Outer.setCompileCommand("C.cpp", llvm::None);
+  EXPECT_THAT(Changes, ElementsAre(ElementsAre("A.cpp"), ElementsAre("B.cpp"),
+                                   ElementsAre("A.cpp"), ElementsAre("C.cpp")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang




More information about the cfe-commits mailing list