[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

[Simon Marchi via Phabricator via cfe-commits]

simark created this revision.
Herald added a subscriber: cfe-commits.

I noticed that when getVirtualFile is called for a file, subsequent
calls to getFile will return a FileEntry without the RealPathName
computed:

  const FileEntry *VFE = Mgr->getVirtualFile("/foo/../bar", ...);
  const FileEntry *FE = Mgr->getFile("/foo/../bar");
  
  FE->tryGetRealPathName() returns an empty string.

This is because getVirtualFile creates a FileEntry without computing the
RealPathName field and stores it in SeenFileEntries.  getFile then
simply retrieves this object and returns it, when asked for the same
file.

I think it's reasonable to try to compute RealPathName in getVirtualFile
too.

There might be a similar issue with the File field.  If you call
getVirtualFile, then call getFile with open=true, you may get a
FileEntry with the File field NULL, which is not what you requested.  I
have not addressed this issue in this patch though.


Repository:
  rC Clang

https://reviews.llvm.org/D49197

Files:
  lib/Basic/FileManager.cpp
  unittests/Basic/FileManagerTest.cpp


Index: unittests/Basic/FileManagerTest.cpp
===================================================================
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -322,4 +322,40 @@
   EXPECT_EQ(Path, ExpectedResult);
 }
 
+/// When calling getVirtualFile then getFile for the same file, the real path
+/// should be computed (in other words, getVirtualFile should compute the real
+/// path).
+TEST_F(FileManagerTest, getVirtualFileThenGetFileRealPathName) {
+  SmallString<64> RealPath;
+  SmallString<64> NonRealPath;
+  SmallString<64> WorkingDirectory;
+
+#ifdef _WIN32
+  RealPath = "C:\\a\\b";
+  NonRealPath = "C:\\a\\..\\a\\b";
+  WorkingDirectory = "C:\\";
+#else
+  RealPath = "/a/b";
+  NonRealPath = "/a/../a/b";
+  WorkingDirectory = "/";
+#endif
+
+  auto FS =
+      IntrusiveRefCntPtr<vfs::InMemoryFileSystem>(new vfs::InMemoryFileSystem);
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(WorkingDirectory));
+
+  FS->addFile(RealPath, 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  const FileEntry *VFE = Manager.getVirtualFile(NonRealPath, 0, 0);
+  ASSERT_TRUE(VFE != nullptr);
+  EXPECT_EQ(VFE->tryGetRealPathName(), RealPath);
+
+  const FileEntry *FE = Manager.getFile(NonRealPath);
+  ASSERT_TRUE(FE != nullptr);
+  EXPECT_EQ(FE->tryGetRealPathName(), RealPath);
+}
+
 } // anonymous namespace
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -390,6 +390,11 @@
   UFE->UID     = NextFileUID++;
   UFE->IsValid = true;
   UFE->File.reset();
+
+  SmallString<128> RealPathName;
+  if (!FS->getRealPath(InterndFileName, RealPathName))
+    UFE->RealPathName = RealPathName.str();
+
   return UFE;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49197.155026.patch
Type: text/x-patch
Size: 1829 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180711/c75e593d/attachment-0001.bin>