[PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

Nathan Hawes via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 21 23:55:35 PST 2021


nathawes planned changes to this revision.
nathawes added a comment.

Thanks for reviewing @dexonsmith!

> I don't see a test for a case where the mapped directory already exists in the external FS ... Unless I just missed it, can you add one?

I believe the `clang/test/VFS/directory.c` regression test covers that case. In that one the real file system is initially set up as follows:

  %t/Underlying/
      C.h
      B.h
  %t/Middle
      C.h
  %t/Overlay
      C.h

And it then sets up vfs overlay yaml files with fallthrough set true that redirect Underlying -> Overlay, or Underlying -> Middle and Middle -> Overlay, as specified on the numbered lines in the test (e.g. `// 1) Underlying -> Overlay`).

> My intuition is we'd want to overlay the directory contents, not replace them

I think it behaves as you expect (as an overlay, rather than replacement) when fallthrough is set to true. You can see that happening in first case in the same test (`clang/test/VFS/directory.c`). It has the real file system set up as above and creates a vfs yaml file that maps `%t/Underlying` to `%t/Overlay` in the external/real file system with fallthrough set true. It invokes clang passing that overlay file, an include path of `%t/Underlying`, and a source file that includes both C.h and B.h,  and checks that it finds the C.h from Overlay (after redirecting Underlying -> Overlay) and B.h from Underlying (after redirecting Underlying -> Overlay, not finding it, and falling back to the original path in the underlying file system).

I really should have a unit test showing the directory iterator output directly, though. That makes the overlay behavior much more obvious.


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

https://reviews.llvm.org/D94844



More information about the cfe-commits mailing list