dexonsmith added a comment.
I'm wondering about this:
> Currently, calling setCurrentWorkingDirectory on RedirectingFileSystem
> will attempt to change the working directory for the external FS and if
> that fails, it will set ExternalFSValidWD to false which prevents
> fallthrough.
I'm worried having inconsistent working directories could be worse than
blocking fallthrough. Consider:
# physical
/a/b/c/d.c
/c/README
# virtual
/x/README
What if someone does `setWorkingDirectory()` for `/a/b/c/d.c` followed by `/x`,
and then looks up the relative path `d.c`? Or changes directory to `../c`?
I wonder if there's a way to split up `shouldUseExternalFS()` (or refactor
other code) to allow the external FS to be partially used without adding this
kind of inconsistency.
- Absolute paths should still work just fine.
- Relative paths can be made absolute by the virtual FS first (if the WD isn't
valid), then passed through.
================
Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:764
+ std::error_code setVirtualWorkingDirectory(const Twine &Path);
+
----------------
I think this could use a comment to explain its behaviour since it's not
obvious.
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1079
// Always change the external FS but ignore its result.
+ if (ExternalFS && SetExternalCWD) {
----------------
This comment looks out-of-date.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94811/new/
https://reviews.llvm.org/D94811
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
