Author: Duncan P. N. Exon Smith Date: 2021-09-01T15:55:33-04:00 New Revision: 8976a1e111393aab7b4965196364ad734a17f2d5
URL: https://github.com/llvm/llvm-project/commit/8976a1e111393aab7b4965196364ad734a17f2d5 DIFF: https://github.com/llvm/llvm-project/commit/8976a1e111393aab7b4965196364ad734a17f2d5.diff LOG: VFS: Document goals of 'use-external-name' and related logic, NFC Document 'use-external-name' and the various bits of logic that make it work, to avoid others having to repeat the archival work (given that I added getFileRefReturnsCorrectNameForDifferentStatPath to FileManagerTest, seems possible I understood this once before!). - b59cf679e81483cbb3a9252056b7528f4c49586c added 'use-external-name' to RedirectingFileSystem. This causes `stat`s to return the external name for a redirected file instead of the name it was accessed by, leaking it through the VFS. - d066d4c849be06a01c0d17e8dc206913f4e7bfe3 propagated the external name further through clang::FileManager. - 4dc5573acc0d2e7c59d8bac2543eb25cb4b32984, which added clang::FileEntryRef to clang::FileManager, has complicated concession to account for this as well (since refactored a bit). The goal of 'use-external-name' is to enable Clang to report "real" file paths to users (via diagnostics) and to external tools (such as debuggers reading debug info and build systems reading `.d` files). I've added FIXMEs to look at other channels for communicating the external names, since the current implementation adds complexity to FileManager and exposes an inconsistent interface to clients. Besides that, the FileManager logic appears to be kicking in outside of 'use-external-name'. Seems that *some* vfs::FileSystem implementations canonicalize some paths returned by `stat` in *some* cases (the bug isn't fully understood yet). Volodymyr Sapsai is investigating, this at least better documents what *is* understood. Added: Modified: clang/lib/Basic/FileManager.cpp clang/unittests/Basic/FileManagerTest.cpp llvm/include/llvm/Support/VirtualFileSystem.h Removed: ################################################################################ diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index 74cd2f295be60..c4eae6acd7b04 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -276,6 +276,18 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { } else { // 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 + // vfs::RedirectionFileSystem. This allows clang to report the external + // name to users (in diagnostics) and to tools that don't have access to + // the VFS (in debug info and dependency '.d' files). + // + // 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. 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)}) diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index 0a1f58f3bb90d..b40ba01121f8f 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -276,9 +276,9 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) { TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { // Inject files with the same inode, but where some files have a stat that - // gives a diff erent name. This is adding coverage for weird stat behaviour - // triggered by the RedirectingFileSystem that FileManager::getFileRef has - // special logic for. + // gives a diff erent name. This is adding coverage for stat behaviour + // triggered by the RedirectingFileSystem for 'use-external-name' that + // FileManager::getFileRef has special logic for. auto StatCache = std::make_unique<FakeStatCache>(); StatCache->InjectDirectory("dir", 40); StatCache->InjectFile("dir/f1.cpp", 41); diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 323e6719645d9..e43da9c94f355 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -596,6 +596,17 @@ class RedirectingFileSystemParser; /// contain multiple path components (e.g. /path/to/file). However, any /// directory in such a path that contains more than one child must be uniquely /// represented by a 'directory' entry. +/// +/// When the 'use-external-name' field is set, calls to \a vfs::File::status() +/// give the external (remapped) filesystem name instead of the name the file +/// was accessed by. This is an intentional leak through the \a +/// RedirectingFileSystem abstraction layer. It enables clients to discover +/// (and use) the external file location when communicating with users or tools +/// that don't use the same VFS overlay. +/// +/// FIXME: 'use-external-name' causes behaviour that's inconsistent with how +/// "real" filesystems behave. Maybe there should be a separate channel for +/// this information. class RedirectingFileSystem : public vfs::FileSystem { public: enum EntryKind { EK_Directory, EK_DirectoryRemap, EK_File }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits