amccarth created this revision.
amccarth added reviewers: rnk, vsapsai, arphaman, Bigcheese.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.

Keys in a virtual file system can be in Posix or Windows form or even a 
combination of the two.  Many VFS tests (and a few Clang tests) were XFAILed on 
Windows because of false negatives when comparing paths.

First, we default CaseSenstive to false on Windows.  This allows drive letters 
like "D:" to match "d:".  Windows filesystems are, by default, case 
insensitive, so this makes sense even beyond the drive letter.

Second, we allow slashes to match backslashes when they're used as the root 
component of a path.

Both of these changes are limited to RedirectingFileSystems, so there's little 
chance of affecting other path handling.

These changes allow eleven of the VFS tests to pass on Windows as well as three 
other Clang tests, so they have re-enabled (partially addressing PR43272).


https://reviews.llvm.org/D69958

Files:
  clang/test/Index/index-module-with-vfs.m
  clang/test/Modules/double-quotes.m
  clang/test/Modules/framework-public-includes-private.m
  clang/test/VFS/external-names.c
  clang/test/VFS/framework-import.m
  clang/test/VFS/implicit-include.c
  clang/test/VFS/include-mixed-real-and-virtual.c
  clang/test/VFS/include-real-from-virtual.c
  clang/test/VFS/include-virtual-from-real.c
  clang/test/VFS/include.c
  clang/test/VFS/incomplete-umbrella.m
  clang/test/VFS/module-import.m
  clang/test/VFS/real-path-found-first.m
  clang/test/VFS/relative-path.c
  clang/test/VFS/umbrella-framework-import-skipnonexist.m
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1671,9 +1671,7 @@
 
   // Forward the search to the next component in case this is an empty one.
   if (!FromName.empty()) {
-    if (CaseSensitive ? !Start->equals(FromName)
-                      : !Start->equals_lower(FromName))
-      // failure to match
+    if (!pathComponentMatches(*Start, FromName))
       return make_error_code(llvm::errc::no_such_file_or_directory);
 
     ++Start;
@@ -1695,6 +1693,7 @@
     if (Result || Result.getError() != llvm::errc::no_such_file_or_directory)
       return Result;
   }
+
   return make_error_code(llvm::errc::no_such_file_or_directory);
 }
 
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -651,6 +651,15 @@
     return ExternalFSValidWD && IsFallthrough;
   }
 
+  // In a RedirectingFileSystem, keys can be specified in Posix or Windows
+  // style (or even a mixture of both), so this comparison helper allows
+  // slashes to match backslashes (and vice versa).
+  bool pathComponentMatches(llvm::StringRef lhs, llvm::StringRef rhs) const {
+    if ((CaseSensitive ? lhs.equals(rhs) : lhs.equals_lower(rhs)))
+      return true;
+    return (lhs == "/" && rhs == "\\") || (lhs == "\\" && rhs == "/");
+  }
+
   /// The root(s) of the virtual file system.
   std::vector<std::unique_ptr<Entry>> Roots;
 
@@ -674,7 +683,12 @@
   /// Whether to perform case-sensitive comparisons.
   ///
   /// Currently, case-insensitive matching only works correctly with ASCII.
-  bool CaseSensitive = true;
+  bool CaseSensitive =
+#ifdef _WIN32
+      false;
+#else
+      true;
+#endif
 
   /// IsRelativeOverlay marks whether a ExternalContentsPrefixDir path must
   /// be prefixed in every 'external-contents' when reading from YAML files.
Index: clang/test/VFS/umbrella-framework-import-skipnonexist.m
===================================================================
--- clang/test/VFS/umbrella-framework-import-skipnonexist.m
+++ clang/test/VFS/umbrella-framework-import-skipnonexist.m
@@ -1,8 +1,5 @@
 // REQUIRES: crash-recovery
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
 // RUN: cp -R %S/Inputs/Bar.framework %t/outdir/
Index: clang/test/VFS/relative-path.c
===================================================================
--- clang/test/VFS/relative-path.c
+++ clang/test/VFS/relative-path.c
@@ -3,9 +3,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -I . -ivfsoverlay %t.yaml -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #include "not_real.h"
 
 void foo() {
Index: clang/test/VFS/real-path-found-first.m
===================================================================
--- clang/test/VFS/real-path-found-first.m
+++ clang/test/VFS/real-path-found-first.m
@@ -4,9 +4,6 @@
 // intentionally rebuild modules, since the precompiled module file refers to
 // the dependency files by real path.
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 // RUN: rm -rf %t %t-cache %t.pch
 // RUN: mkdir -p %t/SomeFramework.framework/Modules
 // RUN: cat %S/Inputs/some_frame_module.map > %t/SomeFramework.framework/Modules/module.modulemap
Index: clang/test/VFS/module-import.m
===================================================================
--- clang/test/VFS/module-import.m
+++ clang/test/VFS/module-import.m
@@ -2,9 +2,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 @import not_real;
 
 void foo() {
Index: clang/test/VFS/incomplete-umbrella.m
===================================================================
--- clang/test/VFS/incomplete-umbrella.m
+++ clang/test/VFS/incomplete-umbrella.m
@@ -5,9 +5,6 @@
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
 // RUN:     -ivfsoverlay %t.yaml -F %t -fsyntax-only %s 2>&1 | FileCheck %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 @import Incomplete;
 // CHECK: umbrella header for module 'Incomplete' {{.*}}IncompleteVFS.h
 // CHECK: umbrella header for module 'Incomplete' {{.*}}IncompleteReal.h
Index: clang/test/VFS/include.c
===================================================================
--- clang/test/VFS/include.c
+++ clang/test/VFS/include.c
@@ -1,9 +1,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -I %t -ivfsoverlay %t.yaml -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #include "not_real.h"
 
 void foo() {
Index: clang/test/VFS/include-virtual-from-real.c
===================================================================
--- clang/test/VFS/include-virtual-from-real.c
+++ clang/test/VFS/include-virtual-from-real.c
@@ -4,9 +4,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #include "include_not_real.h"
 
 void foo() {
Index: clang/test/VFS/include-real-from-virtual.c
===================================================================
--- clang/test/VFS/include-real-from-virtual.c
+++ clang/test/VFS/include-real-from-virtual.c
@@ -4,9 +4,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #include "include_real.h"
 
 void foo() {
Index: clang/test/VFS/include-mixed-real-and-virtual.c
===================================================================
--- clang/test/VFS/include-mixed-real-and-virtual.c
+++ clang/test/VFS/include-mixed-real-and-virtual.c
@@ -4,9 +4,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #include "not_real.h"
 #include "real.h"
 
Index: clang/test/VFS/implicit-include.c
===================================================================
--- clang/test/VFS/implicit-include.c
+++ clang/test/VFS/implicit-include.c
@@ -1,9 +1,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %t -include "not_real.h" -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 void foo() {
   bar();
 }
Index: clang/test/VFS/framework-import.m
===================================================================
--- clang/test/VFS/framework-import.m
+++ clang/test/VFS/framework-import.m
@@ -1,9 +1,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -F %t -ivfsoverlay %t.yaml -fsyntax-only %s
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #import <SomeFramework/public_header.h>
 
 void foo() {
Index: clang/test/VFS/external-names.c
===================================================================
--- clang/test/VFS/external-names.c
+++ clang/test/VFS/external-names.c
@@ -1,9 +1,6 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" -e "s@EXTERNAL_NAMES@true@" %S/Inputs/use-external-names.yaml > %t.external.yaml
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" -e "s@EXTERNAL_NAMES@false@" %S/Inputs/use-external-names.yaml > %t.yaml
 
-// FIXME: PR43272
-// XFAIL: system-windows
-
 #include "external-names.h"
 #ifdef REINCLUDE
 #include "external-names.h"
Index: clang/test/Modules/framework-public-includes-private.m
===================================================================
--- clang/test/Modules/framework-public-includes-private.m
+++ clang/test/Modules/framework-public-includes-private.m
@@ -1,6 +1,3 @@
-// FIXME: PR43272
-// XFAIL: system-windows
-
 // RUN: rm -rf %t
 // RUN: mkdir %t
 
Index: clang/test/Modules/double-quotes.m
===================================================================
--- clang/test/Modules/double-quotes.m
+++ clang/test/Modules/double-quotes.m
@@ -1,6 +1,3 @@
-// FIXME: PR43272
-// XFAIL: system-windows
-
 // RUN: rm -rf %t
 // RUN: mkdir %t
 
Index: clang/test/Index/index-module-with-vfs.m
===================================================================
--- clang/test/Index/index-module-with-vfs.m
+++ clang/test/Index/index-module-with-vfs.m
@@ -1,6 +1,3 @@
-// FIXME: PR43272
-// XFAIL: system-windows
-
 @import ModuleNeedsVFS;
 
 void foo() {
@@ -13,7 +10,7 @@
 // RUN: c-index-test -index-file %s -fmodules-cache-path=%t.cache -fmodules -F %t -I %t \
 // RUN:              -ivfsoverlay %t.yaml -Xclang -fdisable-module-hash | FileCheck %s
 
-// CHECK: [importedASTFile]: {{.*}}ModuleNeedsVFS.pcm | loc: 4:1 | name: "ModuleNeedsVFS" | isImplicit: 0
+// CHECK: [importedASTFile]: {{.*}}ModuleNeedsVFS.pcm | loc: 1:1 | name: "ModuleNeedsVFS" | isImplicit: 0
 // CHECK: [indexEntityReference]: kind: function | name: module_needs_vfs
 // CHECK: [indexEntityReference]: kind: function | name: base_module_needs_vfs
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to