[PATCH] D117730: [DNM][VFS] Do not overwrite the path when nesting RedirectingFileSystems

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

@keith, any thoughts on this? IIRC, you'd found a few other bugs while you were in there which you'd left for later; not sure if this is related to them or not.



================
Comment at: clang/test/VFS/directory.c:2
 // RUN: rm -rf %t
-// RUN: mkdir -p %t/Underlying
-// RUN: mkdir -p %t/Overlay
-// RUN: mkdir -p %t/Middle
-// RUN: echo '// B.h in Underlying' > %t/Underlying/B.h
-// RUN: echo '#ifdef NESTED' >> %t/Underlying/B.h
-// RUN: echo '#include "C.h"' >> %t/Underlying/B.h
-// RUN: echo '#endif' >> %t/Underlying/B.h
-// RUN: echo '// C.h in Underlying' > %t/Underlying/C.h
-// RUN: echo '// C.h in Middle' > %t/Middle/C.h
-// RUN: echo '// C.h in Overlay' > %t/Overlay/C.h
-
-// 1) Underlying -> Overlay (C.h found, B.h falling back to Underlying)
-// RUN: sed -e "s at INPUT_DIR@%{/t:regex_replacement}/Overlay at g" -e "s at OUT_DIR@%{/t:regex_replacement}/Underlying at g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -fsyntax-only -DNESTED -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-// RUN: sed -e "s at INPUT_DIR@Overlay at g" -e "s at OUT_DIR@%{/t:regex_replacement}/Underlying at g" %S/Inputs/vfsoverlay-directory-relative.yaml > %t/vfs-relative.yaml
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs-relative.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-
-// DIRECT: {{^}}// B.h in Underlying
-// DIRECT: {{^}}// C.h in Overlay
-
-// 2) Underlying -> Middle -> Overlay (C.h found, B.h falling back to Underlying)
-// RUN: sed -e "s at INPUT_DIR@%{/t:regex_replacement}/Overlay at g" -e "s at OUT_DIR@%{/t:regex_replacement}/Middle at g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs.yaml
-// RUN: sed -e "s at INPUT_DIR@%{/t:regex_replacement}/Middle at g" -e "s at OUT_DIR@%{/t:regex_replacement}/Underlying at g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs2.yaml
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -DNESTED -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-
-// Same as direct above
-
-// 3) Underlying -> Middle -> Overlay (C.h falling back to Middle, B.h falling back to Underlying)
-// RUN: rm -f %t/Overlay/C.h
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK %s
-
-// FALLBACK: {{^}}// B.h in Underlying
-// FALLBACK: {{^}}// C.h in Middle
-
-// 3) Underlying -> Middle -> Overlay (C.h falling back to Underlying, B.h falling back to Underlying)
+// RUN: split-file %s %t
+
----------------
This makes it hard to read what has changed. Probably better to commit an NFC patch (updating the testcase with no behaviour change) and then rebase this patch on top.


================
Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:2714-2716
+  // Test the interaction of two overlays where one maps back to the other,
+  // ie. `a` -> `b` and then `b` -> `a`. This should always use `a` if it
+  // exists and fallback to `b` otherwise.
----------------
Remapping in two directions seems like an extra layer of complexity; is that the case where this came up, or was it something more straightforward?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117730