Author: Ben Langmuir Date: 2022-08-03T09:41:08-07:00 New Revision: 6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37
URL: https://github.com/llvm/llvm-project/commit/6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37 DIFF: https://github.com/llvm/llvm-project/commit/6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37.diff LOG: [clang] Add FileEntryRef::getNameAsRequested() As progress towards having FileManager::getFileRef() return the path as-requested by default, return a FileEntryRef that can use getNameAsRequested() to retrieve this path, with the ultimate goal that this should be the behaviour of getName() and clients should explicitly request the "external" name if they need to (see comment in FileManager::getFileRef). For now, getName() continues to return the external path by looking through the redirects. For now, the new function is only used in unit tests. Differential Revision: https://reviews.llvm.org/D131004 Added: Modified: clang/include/clang/Basic/FileEntry.h clang/lib/Basic/FileManager.cpp clang/unittests/Basic/FileEntryTest.cpp clang/unittests/Basic/FileManagerTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h index 8676604b48364..3ca07cb422b64 100644 --- a/clang/include/clang/Basic/FileEntry.h +++ b/clang/include/clang/Basic/FileEntry.h @@ -59,11 +59,21 @@ class FileEntry; /// accessed by the FileManager's client. class FileEntryRef { public: - StringRef getName() const { return ME->first(); } + /// The name of this FileEntry. If a VFS uses 'use-external-name', this is + /// the redirected name. See getRequestedName(). + StringRef getName() const { return getBaseMapEntry().first(); } + + /// The name of this FileEntry, as originally requested without applying any + /// remappings for VFS 'use-external-name'. + /// + /// FIXME: this should be the semantics of getName(). See comment in + /// FileManager::getFileRef(). + StringRef getNameAsRequested() const { return ME->first(); } + const FileEntry &getFileEntry() const { - return *ME->second->V.get<FileEntry *>(); + return *getBaseMapEntry().second->V.get<FileEntry *>(); } - DirectoryEntryRef getDir() const { return *ME->second->Dir; } + DirectoryEntryRef getDir() const { return *getBaseMapEntry().second->Dir; } inline off_t getSize() const; inline unsigned getUID() const; @@ -150,13 +160,20 @@ class FileEntryRef { explicit FileEntryRef(const MapEntry &ME) : ME(&ME) { assert(ME.second && "Expected payload"); assert(ME.second->V && "Expected non-null"); - assert(ME.second->V.is<FileEntry *>() && "Expected FileEntry"); } /// Expose the underlying MapEntry to simplify packing in a PointerIntPair or /// PointerUnion and allow construction in Optional. const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; } + /// Retrieve the base MapEntry after redirects. + const MapEntry &getBaseMapEntry() const { + const MapEntry *ME = this->ME; + while (const void *Next = ME->second->V.dyn_cast<const void *>()) + ME = static_cast<const MapEntry *>(Next); + return *ME; + } + private: friend class FileMgr::MapEntryOptionalStorage<FileEntryRef>; struct optional_none_tag {}; diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index 451c7c3b8a0dc..cb719762ec7c8 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -293,12 +293,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { // name-as-accessed on the \a FileEntryRef. // // A potential plan to remove this is as follows - - // - 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 - // through the indirection. // - Update callers such as `HeaderSearch::findUsableModuleForHeader()` - // to explicitly use the requested filename rather than just using + // to explicitly use the `getNameAsRequested()` rather than just using // `getName()`. // - Add a `FileManager::getExternalPath` API for explicitly getting the // remapped external filename when there is one available. Adopt it in @@ -329,9 +325,6 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { // Cache the redirection in the previously-inserted entry, still available // in the tentative return value. NamedFileEnt->second = FileEntryRef::MapValue(Redirection); - - // Fix the tentative return value. - NamedFileEnt = &Redirection; } FileEntryRef ReturnedRef(*NamedFileEnt); diff --git a/clang/unittests/Basic/FileEntryTest.cpp b/clang/unittests/Basic/FileEntryTest.cpp index 6c070fb2469df..4249a3d550148 100644 --- a/clang/unittests/Basic/FileEntryTest.cpp +++ b/clang/unittests/Basic/FileEntryTest.cpp @@ -50,6 +50,14 @@ class FileEntryTestHelper { const_cast<FileEntry &>(Base.getFileEntry()), DR)}) .first); } + FileEntryRef addFileRedirect(StringRef Name, FileEntryRef Base) { + return FileEntryRef( + *Files + .insert({Name, FileEntryRef::MapValue( + const_cast<FileEntryRef::MapEntry &>( + Base.getMapEntry()))}) + .first); + } }; namespace { @@ -58,13 +66,23 @@ TEST(FileEntryTest, FileEntryRef) { FileEntryRef R1 = Refs.addFile("1"); FileEntryRef R2 = Refs.addFile("2"); FileEntryRef R1Also = Refs.addFileAlias("1-also", R1); + FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1); + FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect); EXPECT_EQ("1", R1.getName()); EXPECT_EQ("2", R2.getName()); EXPECT_EQ("1-also", R1Also.getName()); + EXPECT_EQ("1", R1Redirect.getName()); + EXPECT_EQ("1", R1Redirect2.getName()); + + EXPECT_EQ("1", R1.getNameAsRequested()); + EXPECT_EQ("1-redirect", R1Redirect.getNameAsRequested()); + EXPECT_EQ("1-redirect2", R1Redirect2.getNameAsRequested()); EXPECT_NE(&R1.getFileEntry(), &R2.getFileEntry()); EXPECT_EQ(&R1.getFileEntry(), &R1Also.getFileEntry()); + EXPECT_EQ(&R1.getFileEntry(), &R1Redirect.getFileEntry()); + EXPECT_EQ(&R1Redirect.getFileEntry(), &R1Redirect2.getFileEntry()); const FileEntry *CE1 = R1; EXPECT_EQ(CE1, &R1.getFileEntry()); @@ -93,6 +111,8 @@ TEST(FileEntryTest, equals) { FileEntryRef R1 = Refs.addFile("1"); FileEntryRef R2 = Refs.addFile("2"); FileEntryRef R1Also = Refs.addFileAlias("1-also", R1); + FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1); + FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect); EXPECT_EQ(R1, &R1.getFileEntry()); EXPECT_EQ(&R1.getFileEntry(), R1); @@ -100,6 +120,8 @@ TEST(FileEntryTest, equals) { EXPECT_NE(R1, &R2.getFileEntry()); EXPECT_NE(&R2.getFileEntry(), R1); EXPECT_NE(R1, R2); + EXPECT_EQ(R1, R1Redirect); + EXPECT_EQ(R1, R1Redirect2); OptionalFileEntryRefDegradesToFileEntryPtr M1 = R1; @@ -114,11 +136,16 @@ TEST(FileEntryTest, isSameRef) { FileEntryRef R1 = Refs.addFile("1"); FileEntryRef R2 = Refs.addFile("2"); FileEntryRef R1Also = Refs.addFileAlias("1-also", R1); + FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1); + FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect); EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1))); EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry()))); EXPECT_FALSE(R1.isSameRef(R2)); EXPECT_FALSE(R1.isSameRef(R1Also)); + EXPECT_FALSE(R1.isSameRef(R1Redirect)); + EXPECT_FALSE(R1.isSameRef(R1Redirect2)); + EXPECT_FALSE(R1Redirect.isSameRef(R1Redirect2)); } TEST(FileEntryTest, DenseMapInfo) { diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index 6e60bba9b2a99..419c497e95d68 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -356,9 +356,13 @@ TEST_F(FileManagerTest, getFileRefEquality) { EXPECT_EQ("dir/f1.cpp", F1Redirect->getName()); EXPECT_EQ("dir/f2.cpp", F2->getName()); + EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested()); + EXPECT_EQ("dir/f1-redirect.cpp", F1Redirect->getNameAsRequested()); + // Compare against FileEntry*. EXPECT_EQ(&F1->getFileEntry(), *F1); EXPECT_EQ(*F1, &F1->getFileEntry()); + EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry()); EXPECT_NE(&F2->getFileEntry(), *F1); EXPECT_NE(*F1, &F2->getFileEntry()); @@ -374,7 +378,7 @@ TEST_F(FileManagerTest, getFileRefEquality) { // Compare using isSameRef. EXPECT_TRUE(F1->isSameRef(*F1Again)); - EXPECT_TRUE(F1->isSameRef(*F1Redirect)); + EXPECT_FALSE(F1->isSameRef(*F1Redirect)); EXPECT_FALSE(F1->isSameRef(*F1Also)); EXPECT_FALSE(F1->isSameRef(*F2)); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
