aboutsummaryrefslogtreecommitdiff
path: root/clang/lib/Basic/FileManager.cpp
diff options
context:
space:
mode:
authorBen Barham <ben_barham@apple.com>2022-04-04 16:38:46 -0700
committerBen Barham <ben_barham@apple.com>2022-04-05 14:24:40 -0700
commitf65b0b5dcfeb04e9e6794b32a075432ce3de1ccd (patch)
treee71988b58638ca3123b773026406edda326e3351 /clang/lib/Basic/FileManager.cpp
parentc2f6460145175d265cd1a7ad7906b778bb11fa3d (diff)
downloadllvm-f65b0b5dcfeb04e9e6794b32a075432ce3de1ccd.zip
llvm-f65b0b5dcfeb04e9e6794b32a075432ce3de1ccd.tar.gz
llvm-f65b0b5dcfeb04e9e6794b32a075432ce3de1ccd.tar.bz2
Revert "[VFS] RedirectingFileSystem only replace path if not already mapped"
This reverts commit 3fda0edc51fd68192a30e302d45db081bb02d7f9, which breaks crash reproducers in very specific circumstances. Specifically, since crash reproducers have `UseExternalNames` set to false, the `File->getFileEntry().getDir()->getName()` call in `DoFrameworkLookup` would use the *cached* directory name instead of the directory of the looked-up file. The plan is to re-commit this patch but to *add* `ExposesExternalVFSPath` rather than replace `IsVFSMapped`. Differential Revision: https://reviews.llvm.org/D123103
Diffstat (limited to 'clang/lib/Basic/FileManager.cpp')
-rw-r--r--clang/lib/Basic/FileManager.cpp32
1 files changed, 12 insertions, 20 deletions
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 7c2cc58..b955e02 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -274,15 +274,12 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
if (!UFE)
UFE = new (FilesAlloc.Allocate()) FileEntry();
- // FIXME: This should just check `!Status.ExposesExternalVFSPath`, but the
- // else branch also ends up fixing up relative paths to be the actually
- // looked up absolute path. This isn't necessarily desired, but does seem to
- // be relied on in some clients.
if (Status.getName() == Filename) {
// The name matches. Set the FileEntry.
NamedFileEnt->second = FileEntryRef::MapValue(*UFE, DirInfo);
} else {
- // We need a redirect. First grab the actual entry we want to return.
+ // Name mismatch. We need a redirect. First grab the actual entry we want
+ // to return.
//
// This redirection logic intentionally leaks the external name of a
// redirected file that uses 'use-external-name' in \a
@@ -292,11 +289,9 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
//
// FIXME: This is pretty complicated. It's also inconsistent with how
// "real" filesystems behave and confuses parts of clang expect to see the
- // name-as-accessed on the \a FileEntryRef. To remove this we should
- // implement the FIXME on `ExposesExternalVFSPath`, ie. update the
- // `FileEntryRef::getName()` path to *always* be the virtual path and have
- // clients request the external path only when required through a separate
- // API.
+ // name-as-accessed on the \a FileEntryRef. Maybe the returned \a
+ // FileEntryRef::getName() could return the accessed name unmodified, but
+ // make the external name available via a separate API.
auto &Redirection =
*SeenFileEntries
.insert({Status.getName(), FileEntryRef::MapValue(*UFE, DirInfo)})
@@ -317,16 +312,13 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
FileEntryRef ReturnedRef(*NamedFileEnt);
if (ReusingEntry) { // Already have an entry with this inode, return it.
- // FIXME: This hack ensures that `getDir()` will use the path that was
- // used to lookup this file, even if we found a file by different path
- // first. This is required in order to find a module's structure when its
- // headers/module map are mapped in the VFS.
- //
- // This should be removed once `HeaderSearch` is updated to use `*Ref`s
- // *and* the redirection hack above is removed. The removal of the latter
- // is required since otherwise the ref will have the exposed external VFS
- // path still.
- if (&DirInfo.getDirEntry() != UFE->Dir && Status.ExposesExternalVFSPath)
+ // 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.getDirEntry() != UFE->Dir && Status.IsVFSMapped)
UFE->Dir = &DirInfo.getDirEntry();
// Always update LastRef to the last name by which a file was accessed.