[clang-tools-extra] 226b045 - [clangd] Look for compilation database in `build` subdirectory of parents.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 24 17:22:31 PDT 2020


Author: Sam McCall
Date: 2020-04-25T02:18:17+02:00
New Revision: 226b045b1fe5c436ebd4d9bc1023e508789822c9

URL: https://github.com/llvm/llvm-project/commit/226b045b1fe5c436ebd4d9bc1023e508789822c9
DIFF: https://github.com/llvm/llvm-project/commit/226b045b1fe5c436ebd4d9bc1023e508789822c9.diff

LOG: [clangd] Look for compilation database in `build` subdirectory of parents.

Summary:
This matches the conventional location of cmake build directories.
It also allows creating a `build` symlink, which seems more flexible than the
compile_commands.json symlinks.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
    clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index aa6727fc20f5..3f8a7d8219f7 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -110,14 +110,24 @@ static bool pathEqual(PathRef A, PathRef B) {
 DirectoryBasedGlobalCompilationDatabase::CachedCDB &
 DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
   // FIXME(ibiryukov): Invalidate cached compilation databases on changes
-  // FIXME(sammccall): this function hot, avoid copying key when hitting cache.
   auto Key = maybeCaseFoldPath(Dir);
   auto R = CompilationDatabases.try_emplace(Key);
   if (R.second) { // Cache miss, try to load CDB.
     CachedCDB &Entry = R.first->second;
     std::string Error;
-    Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
     Entry.Path = std::string(Dir);
+    Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+    // Check for $src/build, the conventional CMake build root.
+    // Probe existence first to avoid each plugin doing IO if it doesn't exist.
+    if (!CompileCommandsDir && !Entry.CDB) {
+      llvm::SmallString<256> BuildDir = Dir;
+      llvm::sys::path::append(BuildDir, "build");
+      if (llvm::sys::fs::is_directory(BuildDir)) {
+        vlog("Found candidate build directory {0}", BuildDir);
+        Entry.CDB =
+            tooling::CompilationDatabase::loadFromDirectory(BuildDir, Error);
+      }
+    }
     if (Entry.CDB)
       log("Loaded compilation database from {0}", Dir);
   }

diff  --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index a6835a00b886..aaae58bacd8e 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -8,6 +8,7 @@
 
 #include "GlobalCompilationDatabase.h"
 
+#include "Matchers.h"
 #include "Path.h"
 #include "TestFS.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -164,6 +165,47 @@ TEST_F(OverlayCDBTest, Adjustments) {
                                            "-DFallback", "-DAdjust_baz.cc"));
 }
 
+// Allows placement of files for tests and cleans them up after.
+class ScratchFS {
+  llvm::SmallString<128> Root;
+
+public:
+  ScratchFS() {
+    EXPECT_FALSE(llvm::sys::fs::createUniqueDirectory("clangd-cdb-test", Root))
+        << "Failed to create unique directory";
+  }
+
+  ~ScratchFS() {
+    EXPECT_FALSE(llvm::sys::fs::remove_directories(Root))
+        << "Failed to cleanup " << Root;
+  }
+
+  llvm::StringRef root() const { return Root; }
+
+  void write(PathRef RelativePath, llvm::StringRef Contents) {
+    std::string AbsPath = path(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());
+  }
+
+  std::string path(PathRef RelativePath) const {
+    llvm::SmallString<128> AbsPath(Root);
+    llvm::sys::path::append(AbsPath, RelativePath);
+    llvm::sys::path::native(AbsPath);
+    return AbsPath.str().str();
+  }
+};
+
 TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
   const char *const CDBOuter =
       R"cdb(
@@ -195,44 +237,9 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
         }
       ]
       )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);
-  llvm::SmallString<128> File;
+  ScratchFS FS;
+  FS.write("compile_commands.json", CDBOuter);
+  FS.write("build/compile_commands.json", CDBInner);
 
   // Note that gen2.cc goes missing with our following model, not sure this
   // happens in practice though.
@@ -244,43 +251,50 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
           DiscoveredFiles = Changes;
         });
 
-    File = FS.Root;
-    llvm::sys::path::append(File, "build", "..", "a.cc");
-    DB.getCompileCommand(File.str());
+    DB.getCompileCommand(FS.path("build/../a.cc"));
     EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(AllOf(
                                      EndsWith("a.cc"), Not(HasSubstr("..")))));
     DiscoveredFiles.clear();
 
-    File = FS.Root;
-    llvm::sys::path::append(File, "build", "gen.cc");
-    DB.getCompileCommand(File.str());
+    DB.getCompileCommand(FS.path("build/gen.cc"));
     EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("gen.cc")));
   }
 
   // With a custom compile commands dir.
   {
-    DirectoryBasedGlobalCompilationDatabase DB(FS.Root.str().str());
+    DirectoryBasedGlobalCompilationDatabase DB(FS.root().str());
     std::vector<std::string> DiscoveredFiles;
     auto Sub =
         DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
           DiscoveredFiles = Changes;
         });
 
-    File = FS.Root;
-    llvm::sys::path::append(File, "a.cc");
-    DB.getCompileCommand(File.str());
+    DB.getCompileCommand(FS.path("a.cc"));
     EXPECT_THAT(DiscoveredFiles,
                 UnorderedElementsAre(EndsWith("a.cc"), EndsWith("gen.cc"),
                                      EndsWith("gen2.cc")));
     DiscoveredFiles.clear();
 
-    File = FS.Root;
-    llvm::sys::path::append(File, "build", "gen.cc");
-    DB.getCompileCommand(File.str());
+    DB.getCompileCommand(FS.path("build/gen.cc"));
     EXPECT_THAT(DiscoveredFiles, IsEmpty());
   }
 }
 
+TEST(GlobalCompilationDatabaseTest, BuildDir) {
+  ScratchFS FS;
+  auto Command = [&](llvm::StringRef Relative) {
+    return DirectoryBasedGlobalCompilationDatabase(llvm::None)
+        .getCompileCommand(FS.path(Relative))
+        .getValueOr(tooling::CompileCommand())
+        .CommandLine;
+  };
+  EXPECT_THAT(Command("x/foo.cc"), IsEmpty());
+  FS.write("x/build/compile_flags.txt", "-DXYZZY");
+  EXPECT_THAT(Command("x/foo.cc"), Contains("-DXYZZY"));
+  EXPECT_THAT(Command("bar.cc"), IsEmpty())
+      << "x/build/compile_flags.txt only applicable to x/";
+}
+
 TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
   OverlayCDB DB(nullptr);
   std::vector<std::string> DiscoveredFiles;


        


More information about the cfe-commits mailing list