[PATCH] D18849: [modules] Avoid module relocation error when building modules from VFS

[Ben Langmuir via cfe-commits]

benlangmuir added a comment.

> From what I understand from http://lists.llvm.org/pipermail/cfe-dev/2014-July/038273.html, changing the behavior of the cached result has side-effects and demands a non trivial change, am I assuming the right thing?


Yes.

Threading through the Filename seems fine to me.  Specific comments below.


================
Comment at: lib/Basic/FileManager.cpp:221-223
@@ -220,2 +220,5 @@
   // See if there is already an entry in the map.
+  // FIXME: Note that when first requested, the returned file name equals to the
+  // requested path. This is not true when returning a cached result. This is
+  // inconsistent and might lead to clients making the wrong assumptions.
   if (NamedFileEnt.second)
----------------
The first time we lookup a file, we will return whatever name comes back from the VFS, which may or may not be the requested path.  If this is a real file system, it will be the requested path.  If it is a redirecting file system and "use-external-names" is true (which it is by default), then the VFS will return the external contents path, which is generally not the same as the requested path.

================
Comment at: lib/Lex/HeaderSearch.cpp:493
@@ -492,3 +492,3 @@
     // Find the framework in which this header occurs.
-    StringRef FrameworkPath = FE->getDir()->getName();
+    StringRef FrameworkPath = llvm::sys::path::parent_path(FrameworkName);
     bool FoundFramework = false;
----------------
What does this change do?  FE->getDir()->getName() will be the virtual path if there is VFS, which is usually the one you want to operate on.


http://reviews.llvm.org/D18849