sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: llvm/include/llvm/Support/CommandLine.h:1972
+/// \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.
----------------
This seems confusing.
Suppose we have `@x/a`, which includes `@y/b`
Relative=true, CurrentDir=none: we read ./x/y/b
Relative=true, CurrentDir=foo: we read foo/x/y/b
Relative=false, CurrentDir=none: we read ./y/b
Relative=false, CurrentDir=foo: we read... ./y/b?
Seems like it would be clearer to just say relative paths are relative to this
directory, so the last one would be foo/y/b.
Actually looking at the code, I think that's what you implemented?
================
Comment at: llvm/lib/Support/CommandLine.cpp:1105
+ SmallString<128> ResponseFile;
+ llvm::sys::path::append(ResponseFile, "@", BasePath, FileName);
+ Arg = Saver.save(ResponseFile.c_str()).data();
----------------
This is @/BasePath I think (args are segments).
(I think this means you were running the wrong tests)
================
Comment at: llvm/lib/Support/CommandLine.cpp:1183
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
----------------
this is subtle, consider a comment: CurrentDir is only relevant for "top-level"
expansions || !RelativeNames, but nested ones always have absolute paths if
RelativeNames so CurrentDir is ignored.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70857/new/
https://reviews.llvm.org/D70857
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
