[clangd] Reland rL365634

This was reverted in rL365678, the failure was due to YAML parsing of
compile_commands.json.

Converting backslashes to forward slashes to fix the issue in unittest.

llvm-svn: 365748

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

@@ -8,12 +8,18 @@
 
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
+#include "Path.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include <string>
+#include <tuple>
+#include <vector>
 
 namespace clang {
 namespace clangd {
@@ -43,6 +49,16 @@ std::string getStandardResourceDir() {
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
 }
 
+// Runs the given action on all parent directories of filename, starting from
+// deepest directory and going up to root. Stops whenever action succeeds.
+void actOnAllParentDirectories(PathRef FileName,
+                               llvm::function_ref<bool(PathRef)> Action) {
+  for (auto Path = llvm::sys::path::parent_path(FileName);
+       !Path.empty() && !Action(Path);
+       Path = llvm::sys::path::parent_path(Path))
+    ;
+}
+
 } // namespace
 
 static std::string getFallbackClangPath() {
@@ -81,60 +97,138 @@ DirectoryBasedGlobalCompilationDatabase::
     ~DirectoryBasedGlobalCompilationDatabase() = default;
 
 llvm::Optional<tooling::CompileCommand>
-DirectoryBasedGlobalCompilationDatabase::getCompileCommand(
+DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
-    PathRef File, ProjectInfo *Project) const {
+  CDBLookupRequest Req;
-  if (auto CDB = getCDBForFile(File, Project)) {
+  Req.FileName = File;
-    auto Candidates = CDB->getCompileCommands(File);
+  Req.ShouldBroadcast = true;
-    if (!Candidates.empty()) {
+
-      return std::move(Candidates.front());
+  auto Res = lookupCDB(Req);
-    }
+  if (!Res) {
-  } else {
     log("Failed to find compilation database for {0}", File);
+    return llvm::None;
   }
+
+  auto Candidates = Res->CDB->getCompileCommands(File);
+  if (!Candidates.empty())
+    return std::move(Candidates.front());
+
   return None;
 }
 
-std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
+std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ 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(), true};
+    return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
   std::string Error = "";
-  auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+
-  auto Result = CDB.get();
+  CachedCDB Entry;
-  CompilationDatabases.insert(std::make_pair(Dir, std::move(CDB)));
+  Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+  auto Result = Entry.CDB.get();
+  CompilationDatabases[Dir] = std::move(Entry);
+
   return {Result, false};
 }
 
-tooling::CompilationDatabase *
+llvm::Optional<DirectoryBasedGlobalCompilationDatabase::CDBLookupResult>
-DirectoryBasedGlobalCompilationDatabase::getCDBForFile(
+DirectoryBasedGlobalCompilationDatabase::lookupCDB(
-    PathRef File, ProjectInfo *Project) const {
+    CDBLookupRequest Request) const {
-  namespace path = llvm::sys::path;
+  assert(llvm::sys::path::is_absolute(Request.FileName) &&
-  assert((path::is_absolute(File, path::Style::posix) ||
-          path::is_absolute(File, path::Style::windows)) &&
          "path must be absolute");
 
-  tooling::CompilationDatabase *CDB = nullptr;
+  CDBLookupResult Result;
-  bool Cached = false;
+  bool SentBroadcast = false;
+
+  {
     std::lock_guard<std::mutex> Lock(Mutex);
     if (CompileCommandsDir) {
-    std::tie(CDB, Cached) = getCDBInDirLocked(*CompileCommandsDir);
+      std::tie(Result.CDB, SentBroadcast) =
-    if (Project && CDB)
+          getCDBInDirLocked(*CompileCommandsDir);
-      Project->SourceRoot = *CompileCommandsDir;
+      Result.PI.SourceRoot = *CompileCommandsDir;
     } else {
-    for (auto Path = path::parent_path(File); !CDB && !Path.empty();
+      actOnAllParentDirectories(
-         Path = path::parent_path(Path)) {
+          Request.FileName, [this, &SentBroadcast, &Result](PathRef Path) {
-      std::tie(CDB, Cached) = getCDBInDirLocked(Path);
+            std::tie(Result.CDB, SentBroadcast) = getCDBInDirLocked(Path);
-      if (Project && CDB)
+            Result.PI.SourceRoot = Path;
-        Project->SourceRoot = Path;
+            return Result.CDB != nullptr;
+          });
+    }
+
+    if (!Result.CDB)
+      return llvm::None;
+
+    // Mark CDB as broadcasted to make sure discovery is performed once.
+    if (Request.ShouldBroadcast && !SentBroadcast)
+      CompilationDatabases[Result.PI.SourceRoot].SentBroadcast = true;
+  }
+
+  // FIXME: Maybe make the following part async, since this can block retrieval
+  // of compile commands.
+  if (Request.ShouldBroadcast && !SentBroadcast)
+    broadcastCDB(Result);
+  return Result;
+}
+
+void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
+    CDBLookupResult Result) const {
+  assert(Result.CDB && "Trying to broadcast an invalid CDB!");
+
+  std::vector<std::string> AllFiles = Result.CDB->getAllFiles();
+  // We assume CDB in CompileCommandsDir owns all of its entries, since we don't
+  // perform any search in parent paths whenever it is set.
+  if (CompileCommandsDir) {
+    assert(*CompileCommandsDir == Result.PI.SourceRoot &&
+           "Trying to broadcast a CDB outside of CompileCommandsDir!");
+    OnCommandChanged.broadcast(std::move(AllFiles));
+    return;
+  }
+
+  llvm::StringMap<bool> DirectoryHasCDB;
+  {
+    std::lock_guard<std::mutex> Lock(Mutex);
+    // Uniquify all parent directories of all files.
+    for (llvm::StringRef File : AllFiles) {
+      actOnAllParentDirectories(File, [&](PathRef Path) {
+        auto It = DirectoryHasCDB.try_emplace(Path);
+        // Already seen this path, and all of its parents.
+        if (!It.second)
+          return true;
+
+        auto Res = getCDBInDirLocked(Path);
+        It.first->second = Res.first != nullptr;
+        return Path == Result.PI.SourceRoot;
+      });
     }
   }
-  // FIXME: getAllFiles() may return relative paths, we need absolute paths.
+
-  // Hopefully the fix is to change JSONCompilationDatabase and the interface.
+  std::vector<std::string> GovernedFiles;
-  if (CDB && !Cached)
+  for (llvm::StringRef File : AllFiles) {
-    OnCommandChanged.broadcast(CDB->getAllFiles());
+    // A file is governed by this CDB if lookup for the file would find it.
-  return CDB;
+    // Independent of whether it has an entry for that file or not.
+    actOnAllParentDirectories(File, [&](PathRef Path) {
+      if (DirectoryHasCDB.lookup(Path)) {
+        if (Path == Result.PI.SourceRoot)
+          GovernedFiles.push_back(File);
+        // Stop as soon as we hit a CDB.
+        return true;
+      }
+      return false;
+    });
+  }
+
+  OnCommandChanged.broadcast(std::move(GovernedFiles));
+}
+
+llvm::Optional<ProjectInfo>
+DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const {
+  CDBLookupRequest Req;
+  Req.FileName = File;
+  Req.ShouldBroadcast = false;
+  auto Res = lookupCDB(Req);
+  if (!Res)
+    return llvm::None;
+  return Res->PI;
 }
 
 OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
@@ -150,19 +244,16 @@ OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
 }
 
 llvm::Optional<tooling::CompileCommand>
-OverlayCDB::getCompileCommand(PathRef File, ProjectInfo *Project) const {
+OverlayCDB::getCompileCommand(PathRef File) const {
   llvm::Optional<tooling::CompileCommand> Cmd;
   {
     std::lock_guard<std::mutex> Lock(Mutex);
     auto It = Commands.find(File);
-    if (It != Commands.end()) {
+    if (It != Commands.end())
-      if (Project)
-        Project->SourceRoot = "";
       Cmd = It->second;
   }
-  }
   if (!Cmd && Base)
-    Cmd = Base->getCompileCommand(File, Project);
+    Cmd = Base->getCompileCommand(File);
   if (!Cmd)
     return llvm::None;
   adjustArguments(*Cmd, ResourceDir);
@@ -191,5 +282,17 @@ void OverlayCDB::setCompileCommand(
   OnCommandChanged.broadcast({File});
 }
 
+llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const {
+  {
+    std::lock_guard<std::mutex> Lock(Mutex);
+    auto It = Commands.find(File);
+    if (It != Commands.end())
+      return ProjectInfo{};
+  }
+  if (Base)
+    return Base->getProjectInfo(File);
+
+  return llvm::None;
+}
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/GlobalCompilationDatabase.h

@@ -36,9 +36,13 @@ public:
   virtual ~GlobalCompilationDatabase() = default;
 
   /// If there are any known-good commands for building this file, returns one.
-  /// If the ProjectInfo pointer is set, it will also be populated.
   virtual llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const = 0;
+  getCompileCommand(PathRef File) const = 0;
+
+  /// Finds the closest project to \p File.
+  virtual llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const {
+    return llvm::None;
+  }
 
   /// Makes a guess at how to build a file.
   /// The default implementation just runs clang on the file.
@@ -67,20 +71,40 @@ public:
 
   /// Scans File's parents looking for compilation databases.
   /// Any extra flags will be added.
+  /// Might trigger OnCommandChanged, if CDB wasn't broadcasted yet.
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
+  getCompileCommand(PathRef File) const override;
+
+  /// Returns the path to first directory containing a compilation database in
+  /// \p File's parents.
+  llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
 
 private:
-  tooling::CompilationDatabase *getCDBForFile(PathRef File,
+  std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ bool>
-                                              ProjectInfo *) const;
-  std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
   getCDBInDirLocked(PathRef File) const;
 
+  struct CDBLookupRequest {
+    PathRef FileName;
+    // Whether this lookup should trigger discovery of the CDB found.
+    bool ShouldBroadcast = false;
+  };
+  struct CDBLookupResult {
+    tooling::CompilationDatabase *CDB = nullptr;
+    ProjectInfo PI;
+  };
+  llvm::Optional<CDBLookupResult> lookupCDB(CDBLookupRequest Request) const;
+
+  // Performs broadcast on governed files.
+  void broadcastCDB(CDBLookupResult Res) const;
+
   mutable std::mutex Mutex;
   /// Caches compilation databases loaded from directories(keys are
   /// directories).
-  mutable llvm::StringMap<std::unique_ptr<clang::tooling::CompilationDatabase>>
+  struct CachedCDB {
-      CompilationDatabases;
+    std::unique_ptr<clang::tooling::CompilationDatabase> CDB = nullptr;
+    bool SentBroadcast = false;
+  };
+  mutable llvm::StringMap<CachedCDB> CompilationDatabases;
 
   /// Used for command argument pointing to folder where compile_commands.json
   /// is located.
@@ -105,8 +129,9 @@ public:
              llvm::Optional<std::string> ResourceDir = llvm::None);
 
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
+  getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.
   void

clang-tools-extra/clangd/QueryDriverDatabase.cpp

@@ -215,8 +215,8 @@ public:
   }
 
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File, ProjectInfo *PI = nullptr) const override {
+  getCompileCommand(PathRef File) const override {
-    auto Cmd = Base->getCompileCommand(File, PI);
+    auto Cmd = Base->getCompileCommand(File);
     if (!Cmd || Cmd->CommandLine.empty())
       return Cmd;
 
@@ -240,6 +240,10 @@ public:
     return addSystemIncludes(*Cmd, SystemIncludes);
   }
 
+  llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override {
+    return Base->getProjectInfo(File);
+  }
+
 private:
   mutable std::mutex Mu;
   // Caches includes extracted from a driver.

clang-tools-extra/clangd/index/Background.cpp

@@ -619,11 +619,15 @@ BackgroundIndex::loadShards(std::vector<std::string> ChangedFiles) {
   llvm::StringSet<> LoadedShards;
   Rebuilder.startLoading();
   for (const auto &File : ChangedFiles) {
-    ProjectInfo PI;
+    auto Cmd = CDB.getCompileCommand(File);
-    auto Cmd = CDB.getCompileCommand(File, &PI);
     if (!Cmd)
       continue;
-    BackgroundIndexStorage *IndexStorage = IndexStorageFactory(PI.SourceRoot);
+
+    std::string ProjectRoot;
+    if (auto PI = CDB.getProjectInfo(File))
+      ProjectRoot = std::move(PI->SourceRoot);
+
+    BackgroundIndexStorage *IndexStorage = IndexStorageFactory(ProjectRoot);
     auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards);
     for (const auto &Dependency : Dependencies) {
       if (!Dependency.NeedsReIndexing || FilesToIndex.count(Dependency.Path))

clang-tools-extra/clangd/unittests/ClangdTests.cpp

@@ -1113,7 +1113,7 @@ TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
         : CanReturnCommand(CanReturnCommand) {}
 
     llvm::Optional<tooling::CompileCommand>
-    getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override {
+    getCompileCommand(PathRef File) const override {
       // FIXME: make this timeout and fail instead of waiting forever in case
       // something goes wrong.
       CanReturnCommand.wait();

clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

@@ -8,10 +8,21 @@
 
 #include "GlobalCompilationDatabase.h"
 
+#include "Path.h"
 #include "TestFS.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <fstream>
+#include <string>
 
 namespace clang {
 namespace clangd {
@@ -20,8 +31,10 @@ using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::EndsWith;
+using ::testing::IsEmpty;
 using ::testing::Not;
 using ::testing::StartsWith;
+using ::testing::UnorderedElementsAre;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
@@ -50,13 +63,9 @@ class OverlayCDBTest : public ::testing::Test {
   class BaseCDB : public GlobalCompilationDatabase {
   public:
     llvm::Optional<tooling::CompileCommand>
-    getCompileCommand(llvm::StringRef File,
+    getCompileCommand(llvm::StringRef File) const override {
-                      ProjectInfo *Project) const override {
+      if (File == testPath("foo.cc"))
-      if (File == testPath("foo.cc")) {
-        if (Project)
-          Project->SourceRoot = testRoot();
         return cmd(File, "-DA=1");
-      }
       return None;
     }
 
@@ -64,6 +73,10 @@ class OverlayCDBTest : public ::testing::Test {
     getFallbackCommand(llvm::StringRef File) const override {
       return cmd(File, "-DA=2");
     }
+
+    llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override {
+      return ProjectInfo{testRoot()};
+    }
   };
 
 protected:
@@ -153,6 +166,110 @@ TEST_F(OverlayCDBTest, Adjustments) {
                     Not(Contains("random-plugin"))));
 }
 
+TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
+  const char *const CDBOuter =
+      R"cdb(
+      [
+        {
+          "file": "a.cc",
+          "command": "",
+          "directory": "{0}",
+        },
+        {
+          "file": "build/gen.cc",
+          "command": "",
+          "directory": "{0}",
+        },
+        {
+          "file": "build/gen2.cc",
+          "command": "",
+          "directory": "{0}",
+        }
+      ]
+      )cdb";
+  const char *const CDBInner =
+      R"cdb(
+      [
+        {
+          "file": "gen.cc",
+          "command": "",
+          "directory": "{0}/build",
+        }
+      ]
+      )cdb";
+  class CleaningFS {
+  public:
+    llvm::SmallString<128> Root;
+
+    CleaningFS() {
+      EXPECT_FALSE(
+          llvm::sys::fs::createUniqueDirectory("clangd-cdb-test", Root))
+          << "Failed to create unique directory";
+    }
+
+    ~CleaningFS() {
+      EXPECT_FALSE(llvm::sys::fs::remove_directories(Root))
+          << "Failed to cleanup " << Root;
+    }
+
+    void registerFile(PathRef RelativePath, llvm::StringRef Contents) {
+      llvm::SmallString<128> AbsPath(Root);
+      llvm::sys::path::append(AbsPath, RelativePath);
+
+      EXPECT_FALSE(llvm::sys::fs::create_directories(
+          llvm::sys::path::parent_path(AbsPath)))
+          << "Failed to create directories for: " << AbsPath;
+
+      std::error_code EC;
+      llvm::raw_fd_ostream OS(AbsPath, EC);
+      EXPECT_FALSE(EC) << "Failed to open " << AbsPath << " for writing";
+      OS << llvm::formatv(Contents.data(),
+                          llvm::sys::path::convert_to_slash(Root));
+      OS.close();
+
+      EXPECT_FALSE(OS.has_error());
+    }
+  };
+
+  CleaningFS FS;
+  FS.registerFile("compile_commands.json", CDBOuter);
+  FS.registerFile("build/compile_commands.json", CDBInner);
+
+  // Note that gen2.cc goes missing with our following model, not sure this
+  // happens in practice though.
+  {
+    DirectoryBasedGlobalCompilationDatabase DB(llvm::None);
+    std::vector<std::string> DiscoveredFiles;
+    auto Sub =
+        DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
+          DiscoveredFiles = Changes;
+        });
+    DB.getCompileCommand((FS.Root + "/a.cc").str());
+    EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("a.cc")));
+
+    DB.getCompileCommand((FS.Root + "/build/gen.cc").str());
+    EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("gen.cc")));
+  }
+
+  // With a custom compile commands dir.
+  {
+    DirectoryBasedGlobalCompilationDatabase DB(FS.Root.str().str());
+    std::vector<std::string> DiscoveredFiles;
+    auto Sub =
+        DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
+          DiscoveredFiles = Changes;
+        });
+    DB.getCompileCommand((FS.Root + "/a.cc").str());
+    EXPECT_THAT(DiscoveredFiles,
+                UnorderedElementsAre(EndsWith("a.cc"), EndsWith("gen.cc"),
+                                     EndsWith("gen2.cc")));
+
+    DiscoveredFiles.clear();
+    DB.getCompileCommand((FS.Root + "/build/gen.cc").str());
+    EXPECT_THAT(DiscoveredFiles, IsEmpty());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/TestFS.cpp

@@ -6,7 +6,11 @@
 //
 //===----------------------------------------------------------------------===//
 #include "TestFS.h"
+#include "GlobalCompilationDatabase.h"
+#include "Path.h"
 #include "URI.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
@@ -36,9 +40,13 @@ MockCompilationDatabase::MockCompilationDatabase(llvm::StringRef Directory,
   // -ffreestanding avoids implicit stdc-predef.h.
 }
 
+llvm::Optional<ProjectInfo>
+MockCompilationDatabase::getProjectInfo(PathRef File) const {
+  return ProjectInfo{Directory};
+};
+
 llvm::Optional<tooling::CompileCommand>
-MockCompilationDatabase::getCompileCommand(PathRef File,
+MockCompilationDatabase::getCompileCommand(PathRef File) const {
-                                           ProjectInfo *Project) const {
   if (ExtraClangFlags.empty())
     return None;
 
@@ -57,8 +65,6 @@ MockCompilationDatabase::getCompileCommand(PathRef File,
     CommandLine.push_back(RelativeFilePath.str());
   }
 
-  if (Project)
-    Project->SourceRoot = Directory;
   return {tooling::CompileCommand(Directory != llvm::StringRef()
                                       ? Directory
                                       : llvm::sys::path::parent_path(File),

clang-tools-extra/clangd/unittests/TestFS.h

@@ -12,6 +12,8 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTFS_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTFS_H
 #include "ClangdServer.h"
+#include "GlobalCompilationDatabase.h"
+#include "Path.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VirtualFileSystem.h"
@@ -48,7 +50,9 @@ public:
                           StringRef RelPathPrefix = StringRef());
 
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
+  getCompileCommand(PathRef File) const override;
+
+  llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
 
   std::vector<std::string> ExtraClangFlags;