[clang][deps] NFC: Move `ResourceDirectoryCache`

[jansvoboda11]

Summary

The ResourceDirectoryCache is useful not only in the clang-scan-deps CLI tool, but also in the downstream libclang API. This NFC patch moves it to DependencyScanningService.h. to make it reusable.

[dexonsmith]

I don't think this should be reused. If I'm reading the code correctly, this is just a super heavyweight (but cached) call to Driver::GetResourcesPath(). It should probably be deleted instead.

I suggest instead:

1. Use Driver::GetResourcesPath() directly in the libclang application.
2. Figure out if it's safe to do that here as well, and if so, delete this and use it here.

Regarding "is it safe?", the question is whether we expect the clang-scan-deps to be built from the same sources as the clang it's scanning for. I think we do. We're relying on that already for the results of the scan to be correct.

clang/tools/clang-scan-deps/ClangScanDeps.cpp

422–423

This is likely much faster, since it avoids a lock:

assert(!Args.empty());
std::string ResourceDir = Driver::GetResourcePath(Args[0]);

But if the string operations turn out to be expensive we could add caching on top.

Unless, is it supported/expected for clang-scan-deps to be from a different toolchain than clang?

424–427

I assume this is for performance (to avoid doing the filesystem search in each invocation)? If so, please add a comment explaining that.

[jansvoboda11]

Fair points. Abandoning in favor of D108366.

clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h

 //===- DependencyScanningService.h - clang-scan-deps service ===-*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_SERVICE_H
 #define LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_SERVICE_H
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
+
+#include <map>
+#include <mutex>
+#include <string>
 
 namespace clang {
 namespace tooling {
 namespace dependencies {
 
 /// The mode in which the dependency scanner will operate to find the
 /// dependencies.
 enum class ScanningMode {
enum class ScanningOutputFormat {
   /// intermodule dependency information.
   Make,
 
   /// This outputs the full module dependency graph suitable for use for
   /// explicitly building modules.
   Full,
 };
 
+class ResourceDirectoryCache {
+public:
+  /// findResourceDir finds the resource directory relative to the clang
+  /// compiler being used in Args, by running it with "-print-resource-dir"
+  /// option and cache the results for reuse. \returns resource directory path
+  /// associated with the given invocation command or empty string if the
+  /// compiler path is NOT an absolute path.
+  StringRef findResourceDir(const CommandLineArguments &Args, bool ClangCLMode);
+
+private:
+  std::map<std::string, std::string> Cache;
+  std::mutex CacheLock;
+};
+
 /// The dependency scanning service contains the shared state that is used by
 /// the invidual dependency scanning workers.
 class DependencyScanningService {
 public:
   DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
                             bool ReuseFileManager = true,
                             bool SkipExcludedPPRanges = true);
 

clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp

 //===- DependencyScanningService.cpp - clang-scan-deps service ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Program.h"
 
 using namespace clang;
 using namespace tooling;
 using namespace dependencies;
 
+StringRef
+ResourceDirectoryCache::findResourceDir(const CommandLineArguments &Args,
+                                        bool ClangCLMode) {
+  if (Args.size() < 1)
+    return "";
+
+  const std::string &ClangBinaryPath = Args[0];
+  if (!llvm::sys::path::is_absolute(ClangBinaryPath))
+    return "";
+
+  const std::string &ClangBinaryName =
+      std::string(llvm::sys::path::filename(ClangBinaryPath));
+
+  std::unique_lock<std::mutex> LockGuard(CacheLock);
+  const auto &CachedResourceDir = Cache.find(ClangBinaryPath);
+  if (CachedResourceDir != Cache.end())
+    return CachedResourceDir->second;
+
+  std::vector<StringRef> PrintResourceDirArgs{ClangBinaryName};
+  if (ClangCLMode)
+    PrintResourceDirArgs.push_back("/clang:-print-resource-dir");
+  else
+    PrintResourceDirArgs.push_back("-print-resource-dir");
+
+  llvm::SmallString<64> OutputFile, ErrorFile;
+  llvm::sys::fs::createTemporaryFile("print-resource-dir-output",
+                                     "" /*no-suffix*/, OutputFile);
+  llvm::sys::fs::createTemporaryFile("print-resource-dir-error",
+                                     "" /*no-suffix*/, ErrorFile);
+  llvm::FileRemover OutputRemover(OutputFile.c_str());
+  llvm::FileRemover ErrorRemover(ErrorFile.c_str());
+  llvm::Optional<StringRef> Redirects[] = {
+      {""}, // Stdin
+      OutputFile.str(),
+      ErrorFile.str(),
+  };
+  if (const int RC = llvm::sys::ExecuteAndWait(
+      ClangBinaryPath, PrintResourceDirArgs, {}, Redirects)) {
+    auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str());
+    llvm::errs() << ErrorBuf.get()->getBuffer();
+    return "";
+  }
+
+  auto OutputBuf = llvm::MemoryBuffer::getFile(OutputFile.c_str());
+  if (!OutputBuf)
+    return "";
+  StringRef Output = OutputBuf.get()->getBuffer().rtrim('\n');
+
+  Cache[ClangBinaryPath] = Output.str();
+  return Cache[ClangBinaryPath];
+}
+
 DependencyScanningService::DependencyScanningService(
     ScanningMode Mode, ScanningOutputFormat Format, bool ReuseFileManager,
     bool SkipExcludedPPRanges)
     : Mode(Mode), Format(Format), ReuseFileManager(ReuseFileManager),
       SkipExcludedPPRanges(SkipExcludedPPRanges) {
   // Initialize targets for object file support.
   llvm::InitializeAllTargets();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmPrinters();
   llvm::InitializeAllAsmParsers();
 }

clang/tools/clang-scan-deps/ClangScanDeps.cpp

 //===- ClangScanDeps.cpp - Implementation of clang-scan-deps --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/JSON.h"
-#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include <mutex>
-#include <thread>
 
 using namespace clang;
 using namespace tooling::dependencies;
 
 namespace {
 
 class SharedStream {
 public:
   SharedStream(raw_ostream &OS) : OS(OS) {}
   void applyLocked(llvm::function_ref<void(raw_ostream &OS)> Fn) {
     std::unique_lock<std::mutex> LockGuard(Lock);
     Fn(OS);
     OS.flush();
   }
 
 private:
   std::mutex Lock;
   raw_ostream &OS;
 };
 
-class ResourceDirectoryCache {
-public:
-  /// findResourceDir finds the resource directory relative to the clang
-  /// compiler being used in Args, by running it with "-print-resource-dir"
-  /// option and cache the results for reuse. \returns resource directory path
-  /// associated with the given invocation command or empty string if the
-  /// compiler path is NOT an absolute path.
-  StringRef findResourceDir(const tooling::CommandLineArguments &Args,
-                            bool ClangCLMode) {
-    if (Args.size() < 1)
-      return "";
-
-    const std::string &ClangBinaryPath = Args[0];
-    if (!llvm::sys::path::is_absolute(ClangBinaryPath))
-      return "";
-
-    const std::string &ClangBinaryName =
-        std::string(llvm::sys::path::filename(ClangBinaryPath));
-
-    std::unique_lock<std::mutex> LockGuard(CacheLock);
-    const auto &CachedResourceDir = Cache.find(ClangBinaryPath);
-    if (CachedResourceDir != Cache.end())
-      return CachedResourceDir->second;
-
-    std::vector<StringRef> PrintResourceDirArgs{ClangBinaryName};
-    if (ClangCLMode)
-      PrintResourceDirArgs.push_back("/clang:-print-resource-dir");
-    else
-      PrintResourceDirArgs.push_back("-print-resource-dir");
-
-    llvm::SmallString<64> OutputFile, ErrorFile;
-    llvm::sys::fs::createTemporaryFile("print-resource-dir-output",
-                                       "" /*no-suffix*/, OutputFile);
-    llvm::sys::fs::createTemporaryFile("print-resource-dir-error",
-                                       "" /*no-suffix*/, ErrorFile);
-    llvm::FileRemover OutputRemover(OutputFile.c_str());
-    llvm::FileRemover ErrorRemover(ErrorFile.c_str());
-    llvm::Optional<StringRef> Redirects[] = {
-        {""}, // Stdin
-        OutputFile.str(),
-        ErrorFile.str(),
-    };
-    if (const int RC = llvm::sys::ExecuteAndWait(
-            ClangBinaryPath, PrintResourceDirArgs, {}, Redirects)) {
-      auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str());
-      llvm::errs() << ErrorBuf.get()->getBuffer();
-      return "";
-    }
-
-    auto OutputBuf = llvm::MemoryBuffer::getFile(OutputFile.c_str());
-    if (!OutputBuf)
-      return "";
-    StringRef Output = OutputBuf.get()->getBuffer().rtrim('\n');
-
-    Cache[ClangBinaryPath] = Output.str();
-    return Cache[ClangBinaryPath];
-  }
-
-private:
-  std::map<std::string, std::string> Cache;
-  std::mutex CacheLock;
-};
-
 llvm::cl::opt<bool> Help("h", llvm::cl::desc("Alias for -help"),
                          llvm::cl::Hidden);
 
 llvm::cl::OptionCategory DependencyScannerCategory("Tool options");
 
 static llvm::cl::opt<ScanningMode> ScanMode(
     "mode",
     llvm::cl::desc("The preprocessing mode used to compute the dependencies"),
AdjustingCompilations->appendArgumentsAdjuster(
         // The clang-cl driver passes "-o -" to the frontend. Inject the real
         // file here to ensure "-MT" can be deduced if need be.
         if (ClangCLMode && !LastO.empty()) {
           AdjustedArgs.push_back("/clang:-o");
           AdjustedArgs.push_back("/clang:" + LastO);
         }
 
         if (!HasResourceDir) {
           StringRef ResourceDir =
               ResourceDirCache.findResourceDir(Args, ClangCLMode);
           if (!ResourceDir.empty()) {
             AdjustedArgs.push_back("-resource-dir");
             AdjustedArgs.push_back(std::string(ResourceDir));
           }
         }
         AdjustedArgs.insert(AdjustedArgs.end(), FlagsEnd, Args.end());
         return AdjustedArgs;
       });
 
   SharedStream Errs(llvm::errs());
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());