[PATCH] D74777: [VFS][WIP] More consistent handling of hybrid paths of Windows+Posix styles

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

In D74777#1882613 <https://reviews.llvm.org/D74777#1882613>, @john.brawn wrote:

> This causes a lot of test failures in llvm/unittests/Support/VirtualFileSystemTest.cpp. A lot of them use //root as a root directory, and e.g. looking at the MappedFiles test what's happening is:
>
> - `//root/foo/bar/a` is added to the DummyFileSystem
> - getFromYAMLString used on a YAML string that maps `//root/file1` to `//root/foo/bar/a`
> - The `//root/foo/bar/a` is canonicalized to `/root/foo/bar/a`
> - `O->status("//root/file1")` matches the `//root/file1` in the YAML, but the subsequent lookup tries to find `/root/foo/bar/a` which doesn't match the `//root/foo/bar/a` that was added to the DummyFileSystem so we get a "file not found" error


I cannot reproduce the problem you're reporting.  For example:

  D:\src\llvm\build\ninja_dbg\unittests\Support\SupportTests.exe --gtest_filter=VFSFromYAMLTest.MappedFiles
  Note: Google Test filter = VFSFromYAMLTest.MappedFiles
  [==========] Running 1 test from 1 test case.
  [----------] Global test environment set-up.
  [----------] 1 test from VFSFromYAMLTest
  [ RUN      ] VFSFromYAMLTest.MappedFiles
  [       OK ] VFSFromYAMLTest.MappedFiles (3 ms)
  [----------] 1 test from VFSFromYAMLTest (10 ms total)
  
  [----------] Global test environment tear-down
  [==========] 1 test from 1 test case ran. (27 ms total)
  [  PASSED  ] 1 test.

In fact, if I make `canonicalize` preserve both slashes at the root whenever they exist, then I get failures even creating the file system from the yaml because `sys::path::is_absolute` no longer thinks `//root/file1` is an absolute path.

I'm showing the result from VFSFromYAMLTest.MappedFiles because that's the specific example you gave.  But all of the clang VFS tests and LLVM Support tests pass for me with this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74777/new/

https://reviews.llvm.org/D74777