keith added inline comments.
================
Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1738
+
+TEST_F(VFSFromYAMLTest, RelativePathHitWithoutCWD) {
+ IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
----------------
So this test case is actually failing. The difference between it and the others
is I don't call `FS->setCurrentWorkingDirectory("//root/foo")`. This results in
us (with my most recent change here) performing this logic:
1. Fetch the absolute //root/foo/vfsname
2. This results in `realname` being returned
3. We attempt to canonicalize `realname`, but we have no `pwd`, so this doesn't
result in a valid path
4. everything fails past this
It seems to me, without having a ton of context here, that the value returned
from the VFS lookup should actually be `//root/foo/realname`, since otherwise
we could likely hit one of the same issues as those discussed above where if
you actually had this situation:
- `mkdir /root/foo /root/bar`
- `touch /root/foo/realname /root/bar/realname`
- `cd /root/bar`
- lookup `/root/foo/vfsname`, get back `realname`
- canonicalize `realname` to the wrong path `/root/bar/realname`
You'd end up with the wrong file, but I don't think this is actually new
behavior with my change?
@dexonsmith wdyt?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109128/new/
https://reviews.llvm.org/D109128
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
