[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-02-03 Thread Keith Smiley via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG502f14d6f2ee: [VFS] Add a "redirecting-with" field to overlays (authored by bnbarham, committed by keith). Changed prior to commit: https://review

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-02-01 Thread Ben Barham via Phabricator via cfe-commits
bnbarham added a comment. @keith I don't have commit access, would you be able to merge this if you're happy with it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117937/new/ https://reviews.llvm.org/D117937 _

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-02-01 Thread Ben Barham via Phabricator via cfe-commits
bnbarham updated this revision to Diff 404999. bnbarham added a reviewer: nathawes. bnbarham added a comment. Noticed one of the unit tests wasn't actually testing the right thing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117937/new/ https://r

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-31 Thread Ben Barham via Phabricator via cfe-commits
bnbarham marked an inline comment as done. bnbarham added a comment. Failures look unrelated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117937/new/ https://reviews.llvm.org/D117937 ___ cfe-commits mai

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-28 Thread Ben Barham via Phabricator via cfe-commits
bnbarham marked 5 inline comments as done. bnbarham added inline comments. Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:575 +/// instead> +/// 'redirecting-with': keith wrote: > bnbarham wrote: > > keith wrote: > > > I think `red

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-28 Thread Ben Barham via Phabricator via cfe-commits
bnbarham updated this revision to Diff 404194. bnbarham added a comment. Updated to address Keith's comments. Added an error for an invalid redirect kind as well as when both `redirect-with` and `fallthrough` are given. Also added tests for these cases. Repository: rG LLVM Github Monorepo C

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-28 Thread Keith Smiley via Phabricator via cfe-commits
keith added inline comments. Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:575 +/// instead> +/// 'redirecting-with': bnbarham wrote: > keith wrote: > > I think `redirecting-with` is fine, and I can't come up with something > > b

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-28 Thread Ben Barham via Phabricator via cfe-commits
bnbarham added a comment. Thanks for taking a look at that @keith, I appreciate it :)! I'll make those changes today hopefully. Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:575 +/// instead> +/// 'redirecting-with': keith wrot

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-28 Thread Keith Smiley via Phabricator via cfe-commits
keith accepted this revision. keith added a comment. This revision is now accepted and ready to land. I'm definitely not a VFS expert, but I do think this is much nicer than trying to do the other workarounds with multiple VFSs that you described, I left some minor comments, I reviewed carefully

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-24 Thread Ben Barham via Phabricator via cfe-commits
bnbarham added a comment. In D117937#3268114 , @dexonsmith wrote: > The scenario you describe reminds me of the testcase in > https://reviews.llvm.org/D117730 ; can you explain if/how this patch relates > to that one? So a "fallback" is what we actual

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. Haven't had a chance to look at the code (maybe @keith/@vsapsai/others will have time before I do) but at a high-level this seems pretty sound / SGTM. The scenario you describe reminds me of the testcase in https://reviews.llvm.org/D117730 ; can you explain if/how th

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-21 Thread Ben Barham via Phabricator via cfe-commits
bnbarham added a comment. Note: I am in no way attached to the name I came up with and would be very open to something better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117937/new/ https://reviews.llvm.org/D117937

[PATCH] D117937: [VFS] Add a "redirecting-with" field to overlays

2022-01-21 Thread Ben Barham via Phabricator via cfe-commits
bnbarham created this revision. bnbarham added reviewers: akyrtzi, dexonsmith, keith, JDevlieghere. Herald added a subscriber: hiraditya. bnbarham requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Extend "fallthrough" to all