Stopgap fix for finding module for a file mapped in the VFS

If we lookup a path using its 'real' path first, we need to ensure that
when we run header search we still use the VFS-mapped path or we will
not be able to find the corresponding module for the header.

The real problem is that we tie the name of a file to its underlying
FileEntry, which is uniqued by inode, so we only ever get the first name
it is looked up by. This doesn't work with modules, which rely on a
specific file system structure.  I'm hoping to have time to write up a
proposal for fixing this more permanently soon, but as a stopgap this
patch updates the name of the file's directory if it comes from a VFS
mapping.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@209534 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/FileSystemStatCache.h b/include/clang/Basic/FileSystemStatCache.h
index 2e9528db915ca0078cf858365975ee5fc4e4c0f0..af2535661e5ad3b6fb9af6a3abc6d78d534e4fa7 100644 (file)
--- a/include/clang/Basic/FileSystemStatCache.h
+++ b/include/clang/Basic/FileSystemStatCache.h
@@ -38,6 +38,10 @@ struct FileData {
   bool IsDirectory;
   bool IsNamedPipe;
   bool InPCH;
+  bool IsVFSMapped; // FIXME: remove this when files support multiple names
+  FileData()
+      : Size(0), ModTime(0), IsDirectory(false), IsNamedPipe(false),
+        InPCH(false), IsVFSMapped(false) {}
 };
 
 /// \brief Abstract interface for introducing a FileManager cache for 'stat'

diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h
index 978e0408daf15df64e3a2f1d19bf8ba92d731054..0a9949612c03cff1210159a4d8c28bdb4ad217fc 100644 (file)
--- a/include/clang/Basic/VirtualFileSystem.h
+++ b/include/clang/Basic/VirtualFileSystem.h
@@ -39,6 +39,9 @@ class Status {
   llvm::sys::fs::file_type Type;
   llvm::sys::fs::perms Perms;
 
+public:
+  bool IsVFSMapped; // FIXME: remove when files support multiple names
+
 public:
   Status() : Type(llvm::sys::fs::file_type::status_error) {}
   Status(const llvm::sys::fs::file_status &Status);

diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp
index 940fcaf4174d1b5f6274cdd06b4fb7d26b84831b..14731f6b09cd82c56494be7a84b7aebdf4da9df7 100644 (file)
--- a/lib/Basic/FileManager.cpp
+++ b/lib/Basic/FileManager.cpp
@@ -274,6 +274,16 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
 
   NamedFileEnt.setValue(&UFE);
   if (UFE.isValid()) { // Already have an entry with this inode, return it.
+
+    // FIXME: this hack ensures that if we look up a file by a virtual path in
+    // the VFS that the getDir() will have the virtual path, even if we found
+    // the file by a 'real' path first. This is required in order to find a
+    // module's structure when its headers/module map are mapped in the VFS.
+    // We should remove this as soon as we can properly support a file having
+    // multiple names.
+    if (DirInfo != UFE.Dir && Data.IsVFSMapped)
+      UFE.Dir = DirInfo;
+
     // If the stat process opened the file, close it to avoid a FD leak.
     if (F)
       delete F;

diff --git a/lib/Basic/FileSystemStatCache.cpp b/lib/Basic/FileSystemStatCache.cpp
index 44ba48aa2fcca5775b2f4e0efba8147dc878792f..0f16e94a05ec17464dfdbd077d12cd2733a6e752 100644 (file)
--- a/lib/Basic/FileSystemStatCache.cpp
+++ b/lib/Basic/FileSystemStatCache.cpp
@@ -39,6 +39,7 @@ static void copyStatusToFileData(const vfs::Status &Status,
   Data.IsDirectory = Status.isDirectory();
   Data.IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
   Data.InPCH = false;
+  Data.IsVFSMapped = Status.IsVFSMapped;
 }
 
 /// FileSystemStatCache::get - Get the 'stat' information for the specified

diff --git a/lib/Basic/VirtualFileSystem.cpp b/lib/Basic/VirtualFileSystem.cpp
index c89b071a773ebe3183d85dadb5359730cb3de95f..a469c9abc392d7155395d783c91833610b094a41 100644 (file)
--- a/lib/Basic/VirtualFileSystem.cpp
+++ b/lib/Basic/VirtualFileSystem.cpp
@@ -31,13 +31,13 @@ using llvm::sys::fs::UniqueID;
 Status::Status(const file_status &Status)
     : UID(Status.getUniqueID()), MTime(Status.getLastModificationTime()),
       User(Status.getUser()), Group(Status.getGroup()), Size(Status.getSize()),
-      Type(Status.type()), Perms(Status.permissions()) {}
+      Type(Status.type()), Perms(Status.permissions()), IsVFSMapped(false)  {}
 
 Status::Status(StringRef Name, StringRef ExternalName, UniqueID UID,
                sys::TimeValue MTime, uint32_t User, uint32_t Group,
                uint64_t Size, file_type Type, perms Perms)
     : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size),
-      Type(Type), Perms(Perms) {}
+      Type(Type), Perms(Perms), IsVFSMapped(false) {}
 
 bool Status::equivalent(const Status &Other) const {
   return getUniqueID() == Other.getUniqueID();
@@ -801,6 +801,8 @@ ErrorOr<Status> VFSFromYAML::status(const Twine &Path) {
     assert(!S || S->getName() == F->getExternalContentsPath());
     if (S && !F->useExternalName(UseExternalNames))
       S->setName(PathStr);
+    if (S)
+      S->IsVFSMapped = true;
     return S;
   } else { // directory
     DirectoryEntry *DE = cast<DirectoryEntry>(*Result);

diff --git a/test/VFS/Inputs/actual_module.map b/test/VFS/Inputs/actual_module.map
index 282dac37c7b093766af1c8c90da9907df5c6e00c..d2f5b64a38d31b221d7c41d2b2f834763d3fe089 100644 (file)
--- a/test/VFS/Inputs/actual_module.map
+++ b/test/VFS/Inputs/actual_module.map
@@ -2,3 +2,7 @@ module not_real {
   header "not_real.h"
   export *
 }
+module import_some_frame {
+  header "import_some_frame.h"
+  export *
+}

diff --git a/test/VFS/Inputs/import_some_frame.h b/test/VFS/Inputs/import_some_frame.h
new file mode 100644 (file)
index 0000000..c1f68c8
--- /dev/null
+++ b/test/VFS/Inputs/import_some_frame.h
@@ -0,0 +1,2 @@
+#import <SomeFramework/public_header.h>
+#import <SomeFramework/public_header2.h>

diff --git a/test/VFS/Inputs/public_header.h b/test/VFS/Inputs/public_header.h
index 471107762b1538982de33b301b1228a7b0966d4d..09d9969d319d94173f11a68c748ec4df881b4243 100644 (file)
--- a/test/VFS/Inputs/public_header.h
+++ b/test/VFS/Inputs/public_header.h
@@ -1 +1,2 @@
+#import <SomeFramework/public_header2.h>
 void from_framework(void);

diff --git a/test/VFS/Inputs/public_header2.h b/test/VFS/Inputs/public_header2.h
new file mode 100644 (file)
index 0000000..d883613
--- /dev/null
+++ b/test/VFS/Inputs/public_header2.h
@@ -0,0 +1 @@
+// public_header2.h

diff --git a/test/VFS/Inputs/some_frame_module.map b/test/VFS/Inputs/some_frame_module.map
new file mode 100644 (file)
index 0000000..3ce59b2
--- /dev/null
+++ b/test/VFS/Inputs/some_frame_module.map
@@ -0,0 +1,5 @@
+framework module SomeFramework {
+  umbrella header "public_header.h"
+  export *
+  module * { export * }
+}

diff --git a/test/VFS/Inputs/vfsoverlay.yaml b/test/VFS/Inputs/vfsoverlay.yaml
index 5c1380870d57854ee26d007f43ec2cfd6a9e98b1..0aa8cd619a5f62fdad5195aab682e9dd6ed24156 100644 (file)
--- a/test/VFS/Inputs/vfsoverlay.yaml
+++ b/test/VFS/Inputs/vfsoverlay.yaml
@@ -6,14 +6,26 @@
         { 'name': 'not_real.h', 'type': 'file',
           'external-contents': 'INPUT_DIR/actual_header.h'
         },
+        { 'name': 'import_some_frame.h', 'type': 'file',
+          'external-contents': 'INPUT_DIR/import_some_frame.h'
+        },
         { 'name': 'module.map', 'type': 'file',
           'external-contents': 'INPUT_DIR/actual_module.map'
         },
         { 'name': 'include_real.h', 'type': 'file',
           'external-contents': 'INPUT_DIR/include_real.h'
         },
-        { 'name': 'SomeFramework.framework/Headers/public_header.h', 'type': 'file',
-          'external-contents': 'INPUT_DIR/public_header.h'
+        { 'name': 'SomeFramework.framework', 'type': 'directory',
+          'contents': [
+            { 'name': 'Headers', 'type': 'directory',
+              'contents': [
+                { 'name': 'public_header.h', 'type': 'file',
+                  'external-contents': 'INPUT_DIR/public_header.h' },
+                { 'name': 'public_header2.h', 'type': 'file',
+                  'external-contents': 'INPUT_DIR/public_header2.h' }
+              ]
+            }
+          ]
         },
         { 'name': 'Foo.framework/Headers/Foo.h', 'type': 'file',
           'external-contents': 'INPUT_DIR/Foo.h'

diff --git a/test/VFS/real-path-found-first.m b/test/VFS/real-path-found-first.m
new file mode 100644 (file)
index 0000000..f494c6e
--- /dev/null
+++ b/test/VFS/real-path-found-first.m
@@ -0,0 +1,74 @@
+// This test is for cases where we lookup a file by its 'real' path before we
+// use its VFS-mapped path. If we accidentally use the real path in header
+// search, we will not find a module for the headers.  To test that we
+// intentionally rebuild modules, since the precompiled module file refers to
+// the dependency files by real path.
+
+// REQUIRES: shell
+// RUN: rm -rf %t %t-cache %t.pch
+// RUN: mkdir -p %t/SomeFramework.framework/Modules
+// RUN: cp %S/Inputs/some_frame_module.map %t/SomeFramework.framework/Modules/module.modulemap
+// RUN: sed -e "s:INPUT_DIR:%S/Inputs:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsoverlay.yaml > %t.yaml
+
+// Build
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only %s -verify -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module
+
+// Rebuild
+// RUN: echo ' ' >> %t/SomeFramework.framework/Modules/module.modulemap
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only %s -verify -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module
+
+// Load from PCH
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -emit-pch  %s -o %t.pch \
+// RUN:     -Werror=non-modular-include-in-framework-module \
+// RUN:     -fmodules-ignore-macro=WITH_PREFIX
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -include-pch %t.pch -fsyntax-only  %s \
+// RUN:     -Werror=non-modular-include-in-framework-module -DWITH_PREFIX \
+// RUN:     -fmodules-ignore-macro=WITH_PREFIX
+
+// While indexing
+// RUN: c-index-test -index-file %s -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module | FileCheck %s
+// RUN: echo ' ' >> %t/SomeFramework.framework/Modules/module.modulemap
+// RUN: c-index-test -index-file %s -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module | FileCheck %s
+// CHECK: warning: treating
+// CHECK-NOT: error
+
+// With a VFS-mapped module map file
+// RUN: mv %t/SomeFramework.framework/Modules/module.modulemap %t/hide_module.map
+// RUN: echo "{ 'version': 0, 'roots': [ { " > %t2.yaml
+// RUN: echo "'name': '%t/SomeFramework.framework/Modules/module.modulemap'," >> %t2.yaml
+// RUN: echo "'type': 'file', 'external-contents': '%t/hide_module.map' } ] }" >> %t2.yaml
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only %s -verify \
+// RUN:     -Wauto-import -Werror=non-modular-include-in-framework-module
+// RUN: echo ' ' >> %t/hide_module.map
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only %s -verify \
+// RUN:     -Wauto-import -Werror=non-modular-include-in-framework-module
+
+// Within a module build
+// RUN: echo '@import import_some_frame;' | \
+// RUN:   %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:      -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only - \
+// RUN:      -Werror=non-modular-include-in-framework-module -x objective-c -I %t
+// RUN: echo ' ' >> %t/hide_module.map
+// RUN: echo '@import import_some_frame;' | \
+// RUN:   %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:      -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only - \
+// RUN:      -Werror=non-modular-include-in-framework-module -x objective-c -I %t
+
+#ifndef WITH_PREFIX
+#import <SomeFramework/public_header.h> // expected-warning{{treating}}
+#import <SomeFramework/public_header2.h> // expected-warning{{treating}}
+@import SomeFramework.public_header2;
+#endif