sammccall added a comment.
Mostly LG, just a couple of possible logic bugs.
Apologies, I was out on vacation and hoped someone else would see this.
================
Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:650
+ bool Fallthrough() const { return ExternalFSValidWD && IsFallthrough; }
+
----------------
this name seems less than ideal:
- it's very similar to `IsFallthrough` but has different semantics
- "fallthrough" itself is not a very clear description of this functionality
IMO
- it's spelled wrong per the style guide
I'd suggest `shouldUseExternalFS()` or so
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1057
+ auto EC = ExternalFS->setCurrentWorkingDirectory(Path);
+ ExternalFSValidWD = static_cast<bool>(EC);
+ }
----------------
this seems backwards - error_code converts to true if it's an *error*
add tests?
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1061
+ // Don't change the working directory if the path doesn't exist.
+ if (!exists(Path))
+ return errc::no_such_file_or_directory;
----------------
this seems like it should go at the top? cd to a nonexistent directory should
avoid changing state and return an error (which means not marking
ExternalFSValidWD as false, I think)
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1065
+ // Non-absolute paths are relative to the current working directory.
+ if (!sys::path::is_absolute(Path)) {
+ SmallString<128> AbsolutePath;
----------------
makeAbsolute already does this check
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65677/new/
https://reviews.llvm.org/D65677
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
