diff options
author | Ben Barham <ben_barham@apple.com> | 2022-04-04 16:38:46 -0700 |
---|---|---|
committer | Ben Barham <ben_barham@apple.com> | 2022-04-05 14:24:40 -0700 |
commit | f65b0b5dcfeb04e9e6794b32a075432ce3de1ccd (patch) | |
tree | e71988b58638ca3123b773026406edda326e3351 /clang/lib/Basic/FileManager.cpp | |
parent | c2f6460145175d265cd1a7ad7906b778bb11fa3d (diff) | |
download | llvm-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.cpp | 32 |
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. |