[PATCH] D130935: [clang] Only modify FileEntryRef names that are externally remapped

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir updated this revision to Diff 449124.
benlangmuir added a comment.

Flip the order of comparisons to avoid unnecessary string comparisons.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130935/new/

https://reviews.llvm.org/D130935

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


Index: clang/unittests/Basic/FileManagerTest.cpp
===================================================================
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -44,16 +44,16 @@
       }
     }
 
-    if (!StatPath)
-      StatPath = Path;
-
     auto fileType = IsFile ?
       llvm::sys::fs::file_type::regular_file :
       llvm::sys::fs::file_type::directory_file;
-    llvm::vfs::Status Status(StatPath, llvm::sys::fs::UniqueID(1, INode),
+    llvm::vfs::Status Status(StatPath ? StatPath : Path,
+                             llvm::sys::fs::UniqueID(1, INode),
                              /*MTime*/{}, /*User*/0, /*Group*/0,
                              /*Size*/0, fileType,
                              llvm::sys::fs::perms::all_all);
+    if (StatPath)
+      Status.ExposesExternalVFSPath = true;
     StatCalls[Path] = Status;
   }
 
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -274,8 +274,8 @@
   if (!UFE)
     UFE = new (FilesAlloc.Allocate()) FileEntry();
 
-  if (Status.getName() == Filename) {
-    // The name matches. Set the FileEntry.
+  if (!Status.ExposesExternalVFSPath || Status.getName() == Filename) {
+    // Use the requested name. Set the FileEntry.
     NamedFileEnt->second = FileEntryRef::MapValue(*UFE, DirInfo);
   } else {
     // Name mismatch. We need a redirect. First grab the actual entry we want
@@ -292,19 +292,7 @@
     // filesystems behave and confuses parts of clang expect to see the
     // name-as-accessed on the \a FileEntryRef.
     //
-    // Further, it isn't *just* external names, but will also give back absolute
-    // paths when a relative path was requested - the check is comparing the
-    // name from the status, which is passed an absolute path resolved from the
-    // current working directory. `clang-apply-replacements` appears to depend
-    // on this behaviour, though it's adjusting the working directory, which is
-    // definitely not supported. Once that's fixed this hack should be able to
-    // be narrowed to only when there's an externally mapped name given back.
-    //
     // A potential plan to remove this is as follows -
-    //   - Add API to determine if the name has been rewritten by the VFS.
-    //   - Fix `clang-apply-replacements` to pass down the absolute path rather
-    //     than changing the CWD. Narrow this hack down to just externally
-    //     mapped paths.
     //   - Expose the requested filename. One possibility would be to allow
     //     redirection-FileEntryRefs to be returned, rather than returning
     //     the pointed-at-FileEntryRef, and customizing `getName()` to look


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130935.449124.patch
Type: text/x-patch
Size: 2796 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220801/a062cf3d/attachment.bin>