kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
kadircet added a parent revision: D70769: [Support] add vfs support for
ExpandResponseFiles.
kadircet edited parent revisions, added: D70222: [clang][Tooling] Add support
for .rsp files in compile_commands.json; removed: D70769: [Support] add vfs
support for ExpandResponseFiles.
This is a follow-up to D70769 <https://reviews.llvm.org/D70769> and D70222
<https://reviews.llvm.org/D70222>, which allows propagation of
current directory down to ExpandResponseFiles for handling of relative paths.
Previously clients had to mutate FS to achieve that, which is not thread-safe
and can even be thread-hostile in the case of real file system.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70857
Files:
clang/include/clang/Tooling/CompilationDatabase.h
clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
clang/lib/Tooling/JSONCompilationDatabase.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp
Index: llvm/lib/Support/CommandLine.cpp
===================================================================
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -24,6 +24,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Triple.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Config/config.h"
@@ -1045,14 +1046,20 @@
return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' && S[2] == '\xbf');
}
-static llvm::Error ExpandResponseFile(StringRef FName, StringSaver &Saver,
- TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &NewArgv,
- bool MarkEOLs, bool RelativeNames,
- llvm::vfs::FileSystem &FS) {
- llvm::ErrorOr<std::string> CurrDirOrErr = FS.getCurrentWorkingDirectory();
- if (!CurrDirOrErr)
- return llvm::errorCodeToError(CurrDirOrErr.getError());
+static llvm::Error ExpandResponseFile(
+ StringRef FName, StringSaver &Saver, TokenizerCallback Tokenizer,
+ SmallVectorImpl<const char *> &NewArgv, bool MarkEOLs, bool RelativeNames,
+ llvm::vfs::FileSystem &FS, llvm::Optional<llvm::StringRef> CurrentDir) {
+ SmallString<128> CurrDir;
+ if (llvm::sys::path::is_relative(FName)) {
+ if (!CurrentDir)
+ llvm::sys::fs::current_path(CurrDir);
+ else
+ CurrDir = *CurrentDir;
+ llvm::sys::path::append(CurrDir, FName);
+ FName = CurrDir.str();
+ }
+
llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> MemBufOrErr =
FS.getBufferForFile(FName);
if (!MemBufOrErr)
@@ -1078,28 +1085,26 @@
// Tokenize the contents into NewArgv.
Tokenizer(Str, Saver, NewArgv, MarkEOLs);
+ if (!RelativeNames)
+ return Error::success();
+ llvm::StringRef BasePath = llvm::sys::path::parent_path(FName);
// If names of nested response files should be resolved relative to including
// file, replace the included response file names with their full paths
// obtained by required resolution.
- if (RelativeNames)
- for (unsigned I = 0; I < NewArgv.size(); ++I)
- if (NewArgv[I]) {
- StringRef Arg = NewArgv[I];
- if (Arg.front() == '@') {
- StringRef FileName = Arg.drop_front();
- if (llvm::sys::path::is_relative(FileName)) {
- SmallString<128> ResponseFile;
- ResponseFile.append(1, '@');
- if (llvm::sys::path::is_relative(FName)) {
- ResponseFile.append(CurrDirOrErr.get());
- }
- llvm::sys::path::append(
- ResponseFile, llvm::sys::path::parent_path(FName), FileName);
- NewArgv[I] = Saver.save(ResponseFile.c_str()).data();
- }
- }
- }
+ for (auto &Arg : NewArgv) {
+ // Skip non-rsp file arguments.
+ if (!Arg || Arg[0] != '@')
+ continue;
+ StringRef FileName(Arg + 1);
+ // Skip if non-relative.
+ if (!llvm::sys::path::is_relative(FileName))
+ continue;
+
+ SmallString<128> ResponseFile;
+ llvm::sys::path::append(ResponseFile, "@", BasePath, FileName);
+ Arg = Saver.save(ResponseFile.c_str()).data();
+ }
return Error::success();
}
@@ -1107,7 +1112,8 @@
/// StringSaver and tokenization strategy.
bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
SmallVectorImpl<const char *> &Argv, bool MarkEOLs,
- bool RelativeNames, llvm::vfs::FileSystem &FS) {
+ bool RelativeNames, llvm::vfs::FileSystem &FS,
+ llvm::Optional<llvm::StringRef> CurrentDir) {
bool AllExpanded = true;
struct ResponseFileRecord {
const char *File;
@@ -1174,7 +1180,7 @@
SmallVector<const char *, 0> ExpandedArgv;
if (llvm::Error Err =
ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
- RelativeNames, FS)) {
+ RelativeNames, FS, CurrentDir)) {
// We couldn't read this file, so we leave it in the argument stream and
// move on.
// TODO: The error should be propagated up the stack.
@@ -1209,7 +1215,7 @@
if (llvm::Error Err =
ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv,
/*MarkEOLs*/ false, /*RelativeNames*/ true,
- *llvm::vfs::getRealFileSystem())) {
+ *llvm::vfs::getRealFileSystem(), llvm::None)) {
// TODO: The error should be propagated up the stack.
llvm::consumeError(std::move(Err));
return false;
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -20,6 +20,8 @@
#define LLVM_SUPPORT_COMMANDLINE_H
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
@@ -1966,12 +1968,16 @@
/// \param [in] RelativeNames true if names of nested response files must be
/// resolved relative to including file.
/// \param [in] FS File system used for all file access when running the tool.
+/// \param [in] CurrentDir Path used to resolve relative rsp files when \p
+/// RelativeNames is set to true, when set to None, process' cwd is used
+/// instead.
/// \return true if all @files were expanded successfully or there were none.
bool ExpandResponseFiles(
StringSaver &Saver, TokenizerCallback Tokenizer,
SmallVectorImpl<const char *> &Argv, bool MarkEOLs = false,
bool RelativeNames = false,
- llvm::vfs::FileSystem &FS = *llvm::vfs::getRealFileSystem());
+ llvm::vfs::FileSystem &FS = *llvm::vfs::getRealFileSystem(),
+ llvm::Optional<llvm::StringRef> CurrentDir = llvm::None);
/// Mark all options not part of this category as cl::ReallyHidden.
///
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -29,6 +29,7 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/YAMLParser.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
@@ -169,8 +170,7 @@
JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
return Base ? inferTargetAndDriverMode(
inferMissingCompileCommands(expandResponseFiles(
- std::move(Base),
- llvm::vfs::createPhysicalFileSystem().release())))
+ std::move(Base), llvm::vfs::getRealFileSystem())))
: nullptr;
}
};
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/ConvertUTF.h"
@@ -47,12 +48,6 @@
private:
std::vector<CompileCommand> expand(std::vector<CompileCommand> Cmds) const {
for (auto &Cmd : Cmds) {
- // FIXME: we should rather propagate the current directory into
- // ExpandResponseFiles as well in addition to FS.
- if (std::error_code EC = FS->setCurrentWorkingDirectory(Cmd.Directory)) {
- llvm::consumeError(llvm::errorCodeToError(EC));
- continue;
- }
bool SeenRSPFile = false;
llvm::SmallVector<const char *, 20> Argv;
Argv.reserve(Cmd.CommandLine.size());
@@ -64,7 +59,8 @@
continue;
llvm::BumpPtrAllocator Alloc;
llvm::StringSaver Saver(Alloc);
- llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS);
+ llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
+ llvm::StringRef(Cmd.Directory));
Cmd.CommandLine.assign(Argv.begin(), Argv.end());
}
return Cmds;
Index: clang/include/clang/Tooling/CompilationDatabase.h
===================================================================
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -222,7 +222,6 @@
/// Returns a wrapped CompilationDatabase that will expand all rsp(response)
/// files on commandline returned by underlying database.
-/// Note: This may change the working directory of FS.
std::unique_ptr<CompilationDatabase>
expandResponseFiles(std::unique_ptr<CompilationDatabase> Base,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
