bnbarham added inline comments.
================
Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:1010
+ std::error_code makeAbsolute(StringRef WorkingDir,
+ SmallVectorImpl<char> &Path) const;
----------------
Should be private IMO
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1369
// append Path ourselves.
+ if (!sys::path::is_absolute(WorkingDir, sys::path::Style::posix) &&
+ !sys::path::is_absolute(WorkingDir,
----------------
Did you find this was needed? It's already checked by the normal `makeAbsolute`
(which checks this already) and the only other caller is when we're using the
overlay directory path, which should always be absolute.
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1913
+ RedirectingFileSystem::RootRelativeKind::OverlayDir) {
+ SmallString<256> FullPath = FS->getOverlayFileDir();
+ assert(!FullPath.empty() && "Overlay file directory must exist");
----------------
This is unused now. Maybe we should just merge the `else if` and `else`
branches and just grab either `getOverlayFileDir` or
`getCurrentWorkingDirectory` depending on `RootRelative`. They should otherwise
be identical.
================
Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1875
+
+ IntrusiveRefCntPtr<vfs::FileSystem> FSOverlayRelative =
+ getFromYAMLString("{\n"
----------------
Just override the previous `FS`/`S` IMO - that way we avoid the accidental
`FS->status` a few lines down 😅
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137473/new/
https://reviews.llvm.org/D137473
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
