[clangd] GlobalCompilationDatabase interface changes

Summary:
- GlobalCompilationDatabase now returns a single command (that's all we use)
- fallback flags are now part of the GlobalCompilationDatabase.
  There's a default implementation that they can optionally customize.
- this allows us to avoid invoking the fallback logic on two separate codepaths
- race on extra flags fixed by locking the mutex
- made GCD const-correct (DBGCD does have mutating methods)

Reviewers: hokein

Subscribers: klimek, cfe-commits, ilya-biryukov

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

llvm-svn: 319647

4 files changed, 73 insertions, 64 deletions

diff --git a/clang-tools-extra/clangd/ClangdUnitStore.cpp b/clang-tools-extra/clangd/ClangdUnitStore.cpp
index 8814d7afedb..a2a8f36ee31 100644
--- a/clang-tools-extra/clangd/ClangdUnitStore.cpp
+++ b/clang-tools-extra/clangd/ClangdUnitStore.cpp
@@ -58,16 +58,14 @@ CppFileCollection::recreateFileIfCompileCommandChanged(
 tooling::CompileCommand
 CppFileCollection::getCompileCommand(GlobalCompilationDatabase &CDB,
                                      PathRef File, PathRef ResourceDir) {
-  std::vector<tooling::CompileCommand> Commands = CDB.getCompileCommands(File);
-  if (Commands.empty())
-    // Add a fake command line if we know nothing.
-    Commands.push_back(getDefaultCompileCommand(File));
+  llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
+  if (!C) // FIXME: Suppress diagnostics? Let the user know?
+    C = CDB.getFallbackCommand(File);
 
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
-  Commands.front().CommandLine.push_back("-resource-dir=" +
-                                         std::string(ResourceDir));
-  return std::move(Commands.front());
+  C->CommandLine.push_back("-resource-dir=" + ResourceDir.str());
+  return std::move(*C);
 }
 
 bool CppFileCollection::compileCommandsAreEqual(
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index c6dc9afbba7..8767b2d9fd7 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -16,24 +16,11 @@
 namespace clang {
 namespace clangd {
 
-static void addExtraFlags(tooling::CompileCommand &Command,
-                          const std::vector<std::string> &ExtraFlags) {
-  if (ExtraFlags.empty())
-    return;
-  assert(Command.CommandLine.size() >= 2 &&
-         "Expected a command line containing at least 2 arguments, the "
-         "compiler binary and the output file");
-  // The last argument of CommandLine is the name of the input file.
-  // Add ExtraFlags before it.
-  auto It = Command.CommandLine.end();
-  --It;
-  Command.CommandLine.insert(It, ExtraFlags.begin(), ExtraFlags.end());
-}
-
-tooling::CompileCommand getDefaultCompileCommand(PathRef File) {
-  std::vector<std::string> CommandLine{"clang", File.str()};
+tooling::CompileCommand
+GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   return tooling::CompileCommand(llvm::sys::path::parent_path(File),
-                                 llvm::sys::path::filename(File), CommandLine,
+                                 llvm::sys::path::filename(File),
+                                 {"clang", File.str()},
                                  /*Output=*/"");
 }
 
@@ -42,33 +29,50 @@ DirectoryBasedGlobalCompilationDatabase::
         clangd::Logger &Logger, llvm::Optional<Path> CompileCommandsDir)
     : Logger(Logger), CompileCommandsDir(std::move(CompileCommandsDir)) {}
 
-std::vector<tooling::CompileCommand>
-DirectoryBasedGlobalCompilationDatabase::getCompileCommands(PathRef File) {
-  std::vector<tooling::CompileCommand> Commands;
-
-  auto CDB = getCompilationDatabase(File);
-  if (CDB)
-    Commands = CDB->getCompileCommands(File);
-  if (Commands.empty())
-    Commands.push_back(getDefaultCompileCommand(File));
-
-  auto It = ExtraFlagsForFile.find(File);
-  if (It != ExtraFlagsForFile.end()) {
-    // Append the user-specified flags to the compile commands.
-    for (tooling::CompileCommand &Command : Commands)
-      addExtraFlags(Command, It->second);
+llvm::Optional<tooling::CompileCommand>
+DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
+  if (auto CDB = getCompilationDatabase(File)) {
+    auto Candidates = CDB->getCompileCommands(File);
+    if (!Candidates.empty()) {
+      addExtraFlags(File, Candidates.front());
+      return std::move(Candidates.front());
+    }
   }
+  return llvm::None;
+}
 
-  return Commands;
+tooling::CompileCommand
+DirectoryBasedGlobalCompilationDatabase::getFallbackCommand(
+    PathRef File) const {
+  auto C = GlobalCompilationDatabase::getFallbackCommand(File);
+  addExtraFlags(File, C);
+  return C;
 }
 
 void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile(
     PathRef File, std::vector<std::string> ExtraFlags) {
+  std::lock_guard<std::mutex> Lock(Mutex);
   ExtraFlagsForFile[File] = std::move(ExtraFlags);
 }
 
+void DirectoryBasedGlobalCompilationDatabase::addExtraFlags(
+    PathRef File, tooling::CompileCommand &C) const {
+  std::lock_guard<std::mutex> Lock(Mutex);
+
+  auto It = ExtraFlagsForFile.find(File);
+  if (It == ExtraFlagsForFile.end())
+    return;
+
+  auto &Args = C.CommandLine;
+  assert(Args.size() >= 2 && "Expected at least [compiler, source file]");
+  // The last argument of CommandLine is the name of the input file.
+  // Add ExtraFlags before it.
+  Args.insert(Args.end() - 1, It->second.begin(), It->second.end());
+}
+
 tooling::CompilationDatabase *
-DirectoryBasedGlobalCompilationDatabase::tryLoadDatabaseFromPath(PathRef File) {
+DirectoryBasedGlobalCompilationDatabase::tryLoadDatabaseFromPath(
+    PathRef File) const {
 
   namespace path = llvm::sys::path;
   auto CachedIt = CompilationDatabases.find(File);
@@ -91,7 +95,8 @@ DirectoryBasedGlobalCompilationDatabase::tryLoadDatabaseFromPath(PathRef File) {
 }
 
 tooling::CompilationDatabase *
-DirectoryBasedGlobalCompilationDatabase::getCompilationDatabase(PathRef File) {
+DirectoryBasedGlobalCompilationDatabase::getCompilationDatabase(
+    PathRef File) const {
   std::lock_guard<std::mutex> Lock(Mutex);
 
   namespace path = llvm::sys::path;
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
index c43ca0dead1..81cdbcc008a 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -27,16 +27,19 @@ namespace clangd {
 
 class Logger;
 
-/// Returns a default compile command to use for \p File.
-tooling::CompileCommand getDefaultCompileCommand(PathRef File);
-
 /// Provides compilation arguments used for parsing C and C++ files.
 class GlobalCompilationDatabase {
 public:
   virtual ~GlobalCompilationDatabase() = default;
 
-  virtual std::vector<tooling::CompileCommand>
-  getCompileCommands(PathRef File) = 0;
+  /// If there are any known-good commands for building this file, returns one.
+  virtual llvm::Optional<tooling::CompileCommand>
+  getCompileCommand(PathRef File) const = 0;
+
+  /// Makes a guess at how to build a file.
+  /// The default implementation just runs clang on the file.
+  /// 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.
@@ -50,19 +53,26 @@ public:
   DirectoryBasedGlobalCompilationDatabase(
       clangd::Logger &Logger, llvm::Optional<Path> CompileCommandsDir);
 
-  std::vector<tooling::CompileCommand>
-  getCompileCommands(PathRef File) override;
+  /// Scans File's parents looking for compilation databases.
+  /// Any extra flags will be added.
+  llvm::Optional<tooling::CompileCommand>
+  getCompileCommand(PathRef File) const override;
+
+  /// Uses the default fallback command, adding any extra flags.
+  tooling::CompileCommand getFallbackCommand(PathRef File) const override;
 
+  /// Sets the extra flags that should be added to a file.
   void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
 
 private:
-  tooling::CompilationDatabase *getCompilationDatabase(PathRef File);
-  tooling::CompilationDatabase *tryLoadDatabaseFromPath(PathRef File);
+  tooling::CompilationDatabase *getCompilationDatabase(PathRef File) const;
+  tooling::CompilationDatabase *tryLoadDatabaseFromPath(PathRef File) const;
+  void addExtraFlags(PathRef File, tooling::CompileCommand &C) const;
 
-  std::mutex Mutex;
+  mutable std::mutex Mutex;
   /// Caches compilation databases loaded from directories(keys are
   /// directories).
-  llvm::StringMap<std::unique_ptr<clang::tooling::CompilationDatabase>>
+  mutable llvm::StringMap<std::unique_ptr<clang::tooling::CompilationDatabase>>
       CompilationDatabases;
 
   /// Stores extra flags per file.
diff --git a/clang-tools-extra/unittests/clangd/ClangdTests.cpp b/clang-tools-extra/unittests/clangd/ClangdTests.cpp
index 73462e8da5c..c8d4397219f 100644
--- a/clang-tools-extra/unittests/clangd/ClangdTests.cpp
+++ b/clang-tools-extra/unittests/clangd/ClangdTests.cpp
@@ -212,21 +212,17 @@ public:
       ExtraClangFlags.push_back("-ffreestanding");
   }
 
-  std::vector<tooling::CompileCommand>
-  getCompileCommands(PathRef File) override {
+  llvm::Optional<tooling::CompileCommand>
+  getCompileCommand(PathRef File) const override {
     if (ExtraClangFlags.empty())
-      return {};
-
-    std::vector<std::string> CommandLine;
-    CommandLine.reserve(3 + ExtraClangFlags.size());
-    CommandLine.insert(CommandLine.end(), {"clang", "-fsyntax-only"});
-    CommandLine.insert(CommandLine.end(), ExtraClangFlags.begin(),
-                       ExtraClangFlags.end());
-    CommandLine.push_back(File.str());
+      return llvm::None;
 
+    auto CommandLine = ExtraClangFlags;
+    CommandLine.insert(CommandLine.begin(), "clang");
+    CommandLine.insert(CommandLine.end(), File.str());
     return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
                                     llvm::sys::path::filename(File),
-                                    CommandLine, "")};
+                                    std::move(CommandLine), "")};
   }
 
   std::vector<std::string> ExtraClangFlags;