[llvm] 1275ab1 - Improve VFS compatibility on Windows

Adrian McCarthy via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 14 08:49:02 PST 2019


Author: Adrian McCarthy
Date: 2019-11-14T08:48:47-08:00
New Revision: 1275ab1620b665eb06231ce3c4e5068c97d9b618

URL: https://github.com/llvm/llvm-project/commit/1275ab1620b665eb06231ce3c4e5068c97d9b618
DIFF: https://github.com/llvm/llvm-project/commit/1275ab1620b665eb06231ce3c4e5068c97d9b618.diff

LOG: Improve VFS compatibility on Windows

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.

This solves the majority of PR43272.  Additional VFS test failures will
be fixed in separate patches.

Differential Revision: https://reviews.llvm.org/D69958

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/test/Index/index-module-with-vfs.m b/clang/test/Index/index-module-with-vfs.m
index a1c74cfd8de0..46fa68dfa130 100644
--- a/clang/test/Index/index-module-with-vfs.m
+++ b/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 @@ void foo() {
 // 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
 

diff  --git a/clang/test/Modules/double-quotes.m b/clang/test/Modules/double-quotes.m
index bed6a48726b5..4ce712ccc6c5 100644
--- a/clang/test/Modules/double-quotes.m
+++ b/clang/test/Modules/double-quotes.m
@@ -1,6 +1,3 @@
-// FIXME: PR43272
-// XFAIL: system-windows
-
 // RUN: rm -rf %t
 // RUN: mkdir %t
 

diff  --git a/clang/test/Modules/framework-public-includes-private.m b/clang/test/Modules/framework-public-includes-private.m
index be9223016baf..0f1e3a242a15 100644
--- a/clang/test/Modules/framework-public-includes-private.m
+++ b/clang/test/Modules/framework-public-includes-private.m
@@ -1,6 +1,3 @@
-// FIXME: PR43272
-// XFAIL: system-windows
-
 // RUN: rm -rf %t
 // RUN: mkdir %t
 

diff  --git a/clang/test/VFS/external-names.c b/clang/test/VFS/external-names.c
index 569c226c8684..1e12c930c35e 100644
--- a/clang/test/VFS/external-names.c
+++ b/clang/test/VFS/external-names.c
@@ -1,9 +1,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at g" -e "s at EXTERNAL_NAMES@true@" %S/Inputs/use-external-names.yaml > %t.external.yaml
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at g" -e "s at 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"

diff  --git a/clang/test/VFS/framework-import.m b/clang/test/VFS/framework-import.m
index 231a3884bb9e..858f1f57fbd1 100644
--- a/clang/test/VFS/framework-import.m
+++ b/clang/test/VFS/framework-import.m
@@ -1,9 +1,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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() {

diff  --git a/clang/test/VFS/implicit-include.c b/clang/test/VFS/implicit-include.c
index 39187417925e..654e0a87de0e 100644
--- a/clang/test/VFS/implicit-include.c
+++ b/clang/test/VFS/implicit-include.c
@@ -1,9 +1,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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();
 }

diff  --git a/clang/test/VFS/include-mixed-real-and-virtual.c b/clang/test/VFS/include-mixed-real-and-virtual.c
index 90b29640c685..e4297c5737d9 100644
--- a/clang/test/VFS/include-mixed-real-and-virtual.c
+++ b/clang/test/VFS/include-mixed-real-and-virtual.c
@@ -4,9 +4,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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"
 

diff  --git a/clang/test/VFS/include-real-from-virtual.c b/clang/test/VFS/include-real-from-virtual.c
index dad71602ee00..3a41c4ea2c76 100644
--- a/clang/test/VFS/include-real-from-virtual.c
+++ b/clang/test/VFS/include-real-from-virtual.c
@@ -4,9 +4,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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() {

diff  --git a/clang/test/VFS/include-virtual-from-real.c b/clang/test/VFS/include-virtual-from-real.c
index 60142a52f9a9..0b0d4cd0025a 100644
--- a/clang/test/VFS/include-virtual-from-real.c
+++ b/clang/test/VFS/include-virtual-from-real.c
@@ -4,9 +4,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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() {

diff  --git a/clang/test/VFS/include.c b/clang/test/VFS/include.c
index 5ece6cc9634b..16a1bca71a72 100644
--- a/clang/test/VFS/include.c
+++ b/clang/test/VFS/include.c
@@ -1,9 +1,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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() {

diff  --git a/clang/test/VFS/incomplete-umbrella.m b/clang/test/VFS/incomplete-umbrella.m
index b8c22bda784c..5b2a1e0b4e1b 100644
--- a/clang/test/VFS/incomplete-umbrella.m
+++ b/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

diff  --git a/clang/test/VFS/module-import.m b/clang/test/VFS/module-import.m
index 7557cb9555f3..336a72d31cfa 100644
--- a/clang/test/VFS/module-import.m
+++ b/clang/test/VFS/module-import.m
@@ -2,9 +2,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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() {

diff  --git a/clang/test/VFS/real-path-found-first.m b/clang/test/VFS/real-path-found-first.m
index 493222a76a3d..8d7d21bf7832 100644
--- a/clang/test/VFS/real-path-found-first.m
+++ b/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

diff  --git a/clang/test/VFS/relative-path.c b/clang/test/VFS/relative-path.c
index 39360d5f781b..fc4ae151d87f 100644
--- a/clang/test/VFS/relative-path.c
+++ b/clang/test/VFS/relative-path.c
@@ -3,9 +3,6 @@
 // RUN: sed -e "s at INPUT_DIR@%/S/Inputs at g" -e "s at OUT_DIR@%/t at 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() {

diff  --git a/clang/test/VFS/umbrella-framework-import-skipnonexist.m b/clang/test/VFS/umbrella-framework-import-skipnonexist.m
index 5ef48ed44a0e..6f536b40a911 100644
--- a/clang/test/VFS/umbrella-framework-import-skipnonexist.m
+++ b/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/

diff  --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index cdbfdbf0c4db..3b4c4aff9bdc 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -532,7 +532,7 @@ class RedirectingFileSystemParser;
 /// \endverbatim
 ///
 /// All configuration options are optional.
-///   'case-sensitive': <boolean, default=true>
+///   'case-sensitive': <boolean, default=(true for Posix, false for Windows)>
 ///   'use-external-names': <boolean, default=true>
 ///   'overlay-relative': <boolean, default=false>
 ///   'fallthrough': <boolean, default=true>
@@ -651,6 +651,17 @@ class RedirectingFileSystem : public vfs::FileSystem {
     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 (representing a root) to match backslashes (and vice versa).  Note
+  // that, other than the root, patch components should not contain slashes or
+  // backslashes.
+  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 +685,12 @@ class RedirectingFileSystem : public vfs::FileSystem {
   /// 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.

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 40b748eab18e..2d5c04baa57e 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -1671,9 +1671,7 @@ RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
 
   // 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 @@ RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
     if (Result || Result.getError() != llvm::errc::no_such_file_or_directory)
       return Result;
   }
+
   return make_error_code(llvm::errc::no_such_file_or_directory);
 }
 


        


More information about the llvm-commits mailing list