Skip to content

Commit 6a79e2f

Browse files
committed
[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
1 parent 446b61c commit 6a79e2f

File tree

4 files changed

+54
-13
lines changed

4 files changed

+54
-13
lines changed

clang/include/clang/Basic/FileEntry.h

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,21 @@ class FileEntry;
5959
/// accessed by the FileManager's client.
6060
class FileEntryRef {
6161
public:
62-
StringRef getName() const { return ME->first(); }
62+
/// The name of this FileEntry. If a VFS uses 'use-external-name', this is
63+
/// the redirected name. See getRequestedName().
64+
StringRef getName() const { return getBaseMapEntry().first(); }
65+
66+
/// The name of this FileEntry, as originally requested without applying any
67+
/// remappings for VFS 'use-external-name'.
68+
///
69+
/// FIXME: this should be the semantics of getName(). See comment in
70+
/// FileManager::getFileRef().
71+
StringRef getNameAsRequested() const { return ME->first(); }
72+
6373
const FileEntry &getFileEntry() const {
64-
return *ME->second->V.get<FileEntry *>();
74+
return *getBaseMapEntry().second->V.get<FileEntry *>();
6575
}
66-
DirectoryEntryRef getDir() const { return *ME->second->Dir; }
76+
DirectoryEntryRef getDir() const { return *getBaseMapEntry().second->Dir; }
6777

6878
inline off_t getSize() const;
6979
inline unsigned getUID() const;
@@ -150,13 +160,20 @@ class FileEntryRef {
150160
explicit FileEntryRef(const MapEntry &ME) : ME(&ME) {
151161
assert(ME.second && "Expected payload");
152162
assert(ME.second->V && "Expected non-null");
153-
assert(ME.second->V.is<FileEntry *>() && "Expected FileEntry");
154163
}
155164

156165
/// Expose the underlying MapEntry to simplify packing in a PointerIntPair or
157166
/// PointerUnion and allow construction in Optional.
158167
const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; }
159168

169+
/// Retrieve the base MapEntry after redirects.
170+
const MapEntry &getBaseMapEntry() const {
171+
const MapEntry *ME = this->ME;
172+
while (const void *Next = ME->second->V.dyn_cast<const void *>())
173+
ME = static_cast<const MapEntry *>(Next);
174+
return *ME;
175+
}
176+
160177
private:
161178
friend class FileMgr::MapEntryOptionalStorage<FileEntryRef>;
162179
struct optional_none_tag {};

clang/lib/Basic/FileManager.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -293,12 +293,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
293293
// name-as-accessed on the \a FileEntryRef.
294294
//
295295
// A potential plan to remove this is as follows -
296-
// - Expose the requested filename. One possibility would be to allow
297-
// redirection-FileEntryRefs to be returned, rather than returning
298-
// the pointed-at-FileEntryRef, and customizing `getName()` to look
299-
// through the indirection.
300296
// - Update callers such as `HeaderSearch::findUsableModuleForHeader()`
301-
// to explicitly use the requested filename rather than just using
297+
// to explicitly use the `getNameAsRequested()` rather than just using
302298
// `getName()`.
303299
// - Add a `FileManager::getExternalPath` API for explicitly getting the
304300
// remapped external filename when there is one available. Adopt it in
@@ -329,9 +325,6 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
329325
// Cache the redirection in the previously-inserted entry, still available
330326
// in the tentative return value.
331327
NamedFileEnt->second = FileEntryRef::MapValue(Redirection);
332-
333-
// Fix the tentative return value.
334-
NamedFileEnt = &Redirection;
335328
}
336329

337330
FileEntryRef ReturnedRef(*NamedFileEnt);

clang/unittests/Basic/FileEntryTest.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ class FileEntryTestHelper {
5050
const_cast<FileEntry &>(Base.getFileEntry()), DR)})
5151
.first);
5252
}
53+
FileEntryRef addFileRedirect(StringRef Name, FileEntryRef Base) {
54+
return FileEntryRef(
55+
*Files
56+
.insert({Name, FileEntryRef::MapValue(
57+
const_cast<FileEntryRef::MapEntry &>(
58+
Base.getMapEntry()))})
59+
.first);
60+
}
5361
};
5462

5563
namespace {
@@ -58,13 +66,23 @@ TEST(FileEntryTest, FileEntryRef) {
5866
FileEntryRef R1 = Refs.addFile("1");
5967
FileEntryRef R2 = Refs.addFile("2");
6068
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
69+
FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1);
70+
FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect);
6171

6272
EXPECT_EQ("1", R1.getName());
6373
EXPECT_EQ("2", R2.getName());
6474
EXPECT_EQ("1-also", R1Also.getName());
75+
EXPECT_EQ("1", R1Redirect.getName());
76+
EXPECT_EQ("1", R1Redirect2.getName());
77+
78+
EXPECT_EQ("1", R1.getNameAsRequested());
79+
EXPECT_EQ("1-redirect", R1Redirect.getNameAsRequested());
80+
EXPECT_EQ("1-redirect2", R1Redirect2.getNameAsRequested());
6581

6682
EXPECT_NE(&R1.getFileEntry(), &R2.getFileEntry());
6783
EXPECT_EQ(&R1.getFileEntry(), &R1Also.getFileEntry());
84+
EXPECT_EQ(&R1.getFileEntry(), &R1Redirect.getFileEntry());
85+
EXPECT_EQ(&R1Redirect.getFileEntry(), &R1Redirect2.getFileEntry());
6886

6987
const FileEntry *CE1 = R1;
7088
EXPECT_EQ(CE1, &R1.getFileEntry());
@@ -93,13 +111,17 @@ TEST(FileEntryTest, equals) {
93111
FileEntryRef R1 = Refs.addFile("1");
94112
FileEntryRef R2 = Refs.addFile("2");
95113
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
114+
FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1);
115+
FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect);
96116

97117
EXPECT_EQ(R1, &R1.getFileEntry());
98118
EXPECT_EQ(&R1.getFileEntry(), R1);
99119
EXPECT_EQ(R1, R1Also);
100120
EXPECT_NE(R1, &R2.getFileEntry());
101121
EXPECT_NE(&R2.getFileEntry(), R1);
102122
EXPECT_NE(R1, R2);
123+
EXPECT_EQ(R1, R1Redirect);
124+
EXPECT_EQ(R1, R1Redirect2);
103125

104126
OptionalFileEntryRefDegradesToFileEntryPtr M1 = R1;
105127

@@ -114,11 +136,16 @@ TEST(FileEntryTest, isSameRef) {
114136
FileEntryRef R1 = Refs.addFile("1");
115137
FileEntryRef R2 = Refs.addFile("2");
116138
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
139+
FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1);
140+
FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect);
117141

118142
EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1)));
119143
EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry())));
120144
EXPECT_FALSE(R1.isSameRef(R2));
121145
EXPECT_FALSE(R1.isSameRef(R1Also));
146+
EXPECT_FALSE(R1.isSameRef(R1Redirect));
147+
EXPECT_FALSE(R1.isSameRef(R1Redirect2));
148+
EXPECT_FALSE(R1Redirect.isSameRef(R1Redirect2));
122149
}
123150

124151
TEST(FileEntryTest, DenseMapInfo) {

clang/unittests/Basic/FileManagerTest.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,13 @@ TEST_F(FileManagerTest, getFileRefEquality) {
356356
EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
357357
EXPECT_EQ("dir/f2.cpp", F2->getName());
358358

359+
EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
360+
EXPECT_EQ("dir/f1-redirect.cpp", F1Redirect->getNameAsRequested());
361+
359362
// Compare against FileEntry*.
360363
EXPECT_EQ(&F1->getFileEntry(), *F1);
361364
EXPECT_EQ(*F1, &F1->getFileEntry());
365+
EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
362366
EXPECT_NE(&F2->getFileEntry(), *F1);
363367
EXPECT_NE(*F1, &F2->getFileEntry());
364368

@@ -374,7 +378,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
374378

375379
// Compare using isSameRef.
376380
EXPECT_TRUE(F1->isSameRef(*F1Again));
377-
EXPECT_TRUE(F1->isSameRef(*F1Redirect));
381+
EXPECT_FALSE(F1->isSameRef(*F1Redirect));
378382
EXPECT_FALSE(F1->isSameRef(*F1Also));
379383
EXPECT_FALSE(F1->isSameRef(*F2));
380384
}

0 commit comments

Comments
 (0)