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

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 12 16:35:36 PDT 2016


bruno abandoned this revision.
bruno added a comment.

Hi Ben,


================
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)
----------------
benlangmuir wrote:
> bruno wrote:
> > benlangmuir wrote:
> > > 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.
> > The behaviour I'm seeing for VFS is a bit different: If it is a redirecting file system and "use-external-names" is true, then the FileManager returns the requested file path. Sub-sequential cached queries return the "use-external-names" / real-path instead. This is the rationale why I'm trying to thread the FileName instead of the FileEntry. Am I missing something?
> That's not supposed to happen.  You can see below that we always use the name from the StatCache, which in turn comes from the the VFS status() result.  Have you traced through what's happening here?  We should figure out how we got the requested path as the name.

I double checked here and you're right, it actually always return the real path for the first request *and* the cached one when "use-external-names" == true. I was misguided by the fact that other places are using the virtual FileName instead of the one in FileEntry, this further lead to inconsistent paths. I'm abandoning this patch since it does not make sense for the crash reproducer, using "use-external-names" = false is the right thing there. 


http://reviews.llvm.org/D18849





More information about the cfe-commits mailing list