kousikk created this revision.
kousikk added reviewers: arphaman, Bigcheese, jkorous, dexonsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
kousikk edited the summary of this revision.
kousikk edited the summary of this revision.
If -resource-dir is not specified as part of the compilation command, then by
default
clang-scan-deps picks up a directory relative to its own path as
resource-directory.
This is probably not the right behavior - since resource directory should be
picked relative
to the path of the clang-compiler in the compilation command.
This patch adds support for it along with a cache to store the resource-dir
paths based on
compiler paths.
Notes:
1. "-resource-dir" is a behavior that's specific to clang, gcc does not have
that flag. That's why if I'm not able to find a resource-dir, I quietly ignore
it.
2. Should I also use the mtime of the compiler in the cache? I think its not
strictly necessary since we assume the filesystem is immutable.
3. From my testing, this does not regress performance.
4. Will try to get this tested on Windows.
But basically the problem that this patch is trying to solve is, clients might
not always want to specify
"-resource-dir" in their compile commands, so scan-deps must auto-infer it
correctly.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D69122
Files:
clang/include/clang/Tooling/ArgumentsAdjusters.h
clang/lib/Tooling/ArgumentsAdjusters.cpp
clang/lib/Tooling/CommonOptionsParser.cpp
clang/lib/Tooling/Tooling.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -12,6 +12,7 @@
#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
#include "clang/Tooling/JSONCompilationDatabase.h"
#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/FileUtilities.h"
#include "llvm/Support/Options.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/Signals.h"
@@ -38,6 +39,64 @@
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.
+ std::string FindResourceDir(const tooling::CommandLineArguments &Args, StringRef Directory) {
+ if (Args.size() < 1)
+ return "";
+
+ const std::string &ClangBinaryPath = MakeAbsolutePath(Directory, Args[0]);
+
+ std::unique_lock<std::mutex> LockGuard(CacheLock);
+ const auto& CachedResourceDir = Cache.find(ClangBinaryPath);
+ if (CachedResourceDir != Cache.end())
+ return CachedResourceDir->second;
+
+ std::vector<StringRef> PrintResourceDirArgs{"clang", "-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
+ StringRef(OutputFile),
+ StringRef(ErrorFile),
+ };
+ 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:
+ /// \returns the absolute path formed by joining the current directory with the given path.
+ std::string MakeAbsolutePath(StringRef CurrentDir, StringRef Path) {
+ if (Path.empty())
+ return "";
+ llvm::SmallString<256> InitialDirectory(CurrentDir);
+ llvm::SmallString<256> AbsolutePath(Path);
+ llvm::sys::fs::make_absolute(InitialDirectory, AbsolutePath);
+ return AbsolutePath.str();
+ }
+
+ std::map<std::string, std::string> Cache;
+ std::mutex CacheLock;
+};
+
/// The high-level implementation of the dependency discovery tool that runs on
/// an individual worker thread.
class DependencyScanningTool {
@@ -218,12 +277,14 @@
auto AdjustingCompilations =
std::make_unique<tooling::ArgumentsAdjustingCompilations>(
std::move(Compilations));
+ ResourceDirectoryCache ResourceDirCache;
AdjustingCompilations->appendArgumentsAdjuster(
- [](const tooling::CommandLineArguments &Args, StringRef FileName) {
+ [&ResourceDirCache](const tooling::CommandLineArguments &Args, StringRef FileName, StringRef Directory) {
std::string LastO = "";
bool HasMT = false;
bool HasMQ = false;
bool HasMD = false;
+ bool HasResourceDir = false;
// We need to find the last -o value.
if (!Args.empty()) {
std::size_t Idx = Args.size() - 1;
@@ -237,6 +298,8 @@
HasMQ = true;
if (Args[Idx] == "-MD")
HasMD = true;
+ if (Args[Idx] == "-resource-dir")
+ HasResourceDir = true;
}
--Idx;
}
@@ -264,6 +327,14 @@
AdjustedArgs.push_back("-Xclang");
AdjustedArgs.push_back("-sys-header-deps");
AdjustedArgs.push_back("-Wno-error");
+
+ if (!HasResourceDir) {
+ const std::string &ResourceDir = ResourceDirCache.FindResourceDir(Args, Directory);
+ if (!ResourceDir.empty()) {
+ AdjustedArgs.push_back("-resource-dir");
+ AdjustedArgs.push_back(ResourceDir);
+ }
+ }
return AdjustedArgs;
});
AdjustingCompilations->appendArgumentsAdjuster(
Index: clang/lib/Tooling/Tooling.cpp
===================================================================
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -189,8 +189,11 @@
llvm::IntrusiveRefCntPtr<FileManager> Files(
new FileManager(FileSystemOptions(), VFS));
ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster();
+ const llvm::ErrorOr<std::string> &WD = VFS->getCurrentWorkingDirectory();
+ if (!WD)
+ return false;
ToolInvocation Invocation(
- getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
+ getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef, WD.get()), FileNameRef),
std::move(ToolAction), Files.get(), std::move(PCHContainerOps));
return Invocation.run();
}
@@ -527,7 +530,7 @@
std::vector<std::string> CommandLine = CompileCommand.CommandLine;
if (ArgsAdjuster)
- CommandLine = ArgsAdjuster(CommandLine, CompileCommand.Filename);
+ CommandLine = ArgsAdjuster(CommandLine, CompileCommand.Filename, CompileCommand.Directory);
assert(!CommandLine.empty());
// Add the resource dir based on the binary of this tool. argv[0] in the
@@ -630,8 +633,11 @@
llvm::IntrusiveRefCntPtr<FileManager> Files(
new FileManager(FileSystemOptions(), OverlayFileSystem));
+ const auto &CWD = OverlayFileSystem->getCurrentWorkingDirectory();
+ if (!CWD)
+ return nullptr;
ToolInvocation Invocation(
- getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileName), FileName),
+ getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileName, CWD.get()), FileName),
&Action, Files.get(), std::move(PCHContainerOps));
InMemoryFileSystem->addFile(FileName, 0,
Index: clang/lib/Tooling/CommonOptionsParser.cpp
===================================================================
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -76,7 +76,7 @@
std::vector<CompileCommand> Commands) const {
for (CompileCommand &Command : Commands)
for (const auto &Adjuster : Adjusters)
- Command.CommandLine = Adjuster(Command.CommandLine, Command.Filename);
+ Command.CommandLine = Adjuster(Command.CommandLine, Command.Filename, Command.Directory);
return Commands;
}
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===================================================================
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -21,7 +21,7 @@
/// Add -fsyntax-only option to the command line arguments.
ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
- return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+ return [](const CommandLineArguments &Args, StringRef /*unused*/, StringRef /*unused*/) {
CommandLineArguments AdjustedArgs;
bool HasSyntaxOnly = false;
for (size_t i = 0, e = Args.size(); i < e; ++i) {
@@ -40,7 +40,7 @@
}
ArgumentsAdjuster getClangStripOutputAdjuster() {
- return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+ return [](const CommandLineArguments &Args, StringRef /*unused*/, StringRef /*unused*/) {
CommandLineArguments AdjustedArgs;
for (size_t i = 0, e = Args.size(); i < e; ++i) {
StringRef Arg = Args[i];
@@ -58,7 +58,7 @@
}
ArgumentsAdjuster getClangStripSerializeDiagnosticAdjuster() {
- return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+ return [](const CommandLineArguments &Args, StringRef /*unused*/, StringRef /*unused*/) {
CommandLineArguments AdjustedArgs;
for (size_t i = 0, e = Args.size(); i < e; ++i) {
StringRef Arg = Args[i];
@@ -74,7 +74,7 @@
}
ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
- return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+ return [](const CommandLineArguments &Args, StringRef /*unused*/, StringRef /*unused*/) {
CommandLineArguments AdjustedArgs;
for (size_t i = 0, e = Args.size(); i < e; ++i) {
StringRef Arg = Args[i];
@@ -95,7 +95,7 @@
ArgumentsAdjuster getInsertArgumentAdjuster(const CommandLineArguments &Extra,
ArgumentInsertPosition Pos) {
- return [Extra, Pos](const CommandLineArguments &Args, StringRef /*unused*/) {
+ return [Extra, Pos](const CommandLineArguments &Args, StringRef /*unused*/, StringRef /*unused*/) {
CommandLineArguments Return(Args);
CommandLineArguments::iterator I;
@@ -122,13 +122,13 @@
return Second;
if (!Second)
return First;
- return [First, Second](const CommandLineArguments &Args, StringRef File) {
- return Second(First(Args, File), File);
+ return [First, Second](const CommandLineArguments &Args, StringRef File, StringRef Directory) {
+ return Second(First(Args, File, Directory), File, Directory);
};
}
ArgumentsAdjuster getStripPluginsAdjuster() {
- return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+ return [](const CommandLineArguments &Args, StringRef /*unused*/, StringRef /*unused*/) {
CommandLineArguments AdjustedArgs;
for (size_t I = 0, E = Args.size(); I != E; I++) {
// According to https://clang.llvm.org/docs/ClangPlugins.html
Index: clang/include/clang/Tooling/ArgumentsAdjusters.h
===================================================================
--- clang/include/clang/Tooling/ArgumentsAdjusters.h
+++ clang/include/clang/Tooling/ArgumentsAdjusters.h
@@ -33,7 +33,7 @@
/// Command line argument adjuster is responsible for command line arguments
/// modification before the arguments are used to run a frontend action.
using ArgumentsAdjuster = std::function<CommandLineArguments(
- const CommandLineArguments &, StringRef Filename)>;
+ const CommandLineArguments &, StringRef Filename, StringRef Directory)>;
/// Gets an argument adjuster that converts input command line arguments
/// to the "syntax check only" variant.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
