Skip to content

Commit 32c501d

Browse files
dexonsmithdexonsmith
committed
Module: Use FileEntryRef and DirectoryEntryRef in Umbrella, Header, and DirectoryName, NFC
Push `FileEntryRef` and `DirectoryEntryRef` further, using it them `Module::Umbrella`, `Module::Header::Entry`, and `Module::DirectoryName::Entry`. - Add `DirectoryEntryRef::operator const DirectoryEntry *` and `OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr`, to get the same "degrades to `DirectoryEntry*` behaviour `FileEntryRef` enjoys (this avoids a bunch of churn in various clang tools). - Fix the `DirectoryEntryRef` constructor from `MapEntry` to take it by `const&`. Note that we cannot get rid of the `...AsWritten` names leveraging the new classes, since these need to be as written in the `ModuleMap` file and the module directory path is preprended for the lookup in the `FileManager`. Differential Revision: https://reviews.llvm.org/D90497
1 parent 327af6a commit 32c501d

File tree

7 files changed

+128
-30
lines changed

7 files changed

+128
-30
lines changed

‎clang/include/clang/Basic/DirectoryEntry.h

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,26 @@ class DirectoryEntryRef {
5151
const MapEntry &getMapEntry() const { return *ME; }
5252

5353
DirectoryEntryRef() = delete;
54-
DirectoryEntryRef(MapEntry &ME) : ME(&ME) {}
54+
DirectoryEntryRef(const MapEntry &ME) : ME(&ME) {}
55+
56+
/// Allow DirectoryEntryRef to degrade into 'const DirectoryEntry*' to
57+
/// facilitate incremental adoption.
58+
///
59+
/// The goal is to avoid code churn due to dances like the following:
60+
/// \code
61+
/// // Old code.
62+
/// lvalue = rvalue;
63+
///
64+
/// // Temporary code from an incremental patch.
65+
/// lvalue = &rvalue.getDirectoryEntry();
66+
///
67+
/// // Final code.
68+
/// lvalue = rvalue;
69+
/// \endcode
70+
///
71+
/// FIXME: Once DirectoryEntryRef is "everywhere" and DirectoryEntry::getName
72+
/// has been deleted, delete this implicit conversion.
73+
operator const DirectoryEntry *() const { return &getDirEntry(); }
5574

5675
private:
5776
friend class FileMgr::MapEntryOptionalStorage<DirectoryEntryRef>;
@@ -147,4 +166,76 @@ static_assert(
147166
} // end namespace optional_detail
148167
} // end namespace llvm
149168

169+
namespace clang {
170+
171+
/// Wrapper around Optional<DirectoryEntryRef> that degrades to 'const
172+
/// DirectoryEntry*', facilitating incremental patches to propagate
173+
/// DirectoryEntryRef.
174+
///
175+
/// This class can be used as return value or field where it's convenient for
176+
/// an Optional<DirectoryEntryRef> to degrade to a 'const DirectoryEntry*'. The
177+
/// purpose is to avoid code churn due to dances like the following:
178+
/// \code
179+
/// // Old code.
180+
/// lvalue = rvalue;
181+
///
182+
/// // Temporary code from an incremental patch.
183+
/// Optional<DirectoryEntryRef> MaybeF = rvalue;
184+
/// lvalue = MaybeF ? &MaybeF.getDirectoryEntry() : nullptr;
185+
///
186+
/// // Final code.
187+
/// lvalue = rvalue;
188+
/// \endcode
189+
///
190+
/// FIXME: Once DirectoryEntryRef is "everywhere" and DirectoryEntry::LastRef
191+
/// and DirectoryEntry::getName have been deleted, delete this class and
192+
/// replace instances with Optional<DirectoryEntryRef>.
193+
class OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr
194+
: public Optional<DirectoryEntryRef> {
195+
public:
196+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr() = default;
197+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(
198+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &&) = default;
199+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(
200+
const OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &) = default;
201+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
202+
operator=(OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &&) = default;
203+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
204+
operator=(const OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &) = default;
205+
206+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(llvm::NoneType) {}
207+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(DirectoryEntryRef Ref)
208+
: Optional<DirectoryEntryRef>(Ref) {}
209+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(Optional<DirectoryEntryRef> MaybeRef)
210+
: Optional<DirectoryEntryRef>(MaybeRef) {}
211+
212+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &operator=(llvm::NoneType) {
213+
Optional<DirectoryEntryRef>::operator=(None);
214+
return *this;
215+
}
216+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &operator=(DirectoryEntryRef Ref) {
217+
Optional<DirectoryEntryRef>::operator=(Ref);
218+
return *this;
219+
}
220+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
221+
operator=(Optional<DirectoryEntryRef> MaybeRef) {
222+
Optional<DirectoryEntryRef>::operator=(MaybeRef);
223+
return *this;
224+
}
225+
226+
/// Degrade to 'const DirectoryEntry *' to allow DirectoryEntry::LastRef and
227+
/// DirectoryEntry::getName have been deleted, delete this class and replace
228+
/// instances with Optional<DirectoryEntryRef>
229+
operator const DirectoryEntry *() const {
230+
return hasValue() ? &getValue().getDirEntry() : nullptr;
231+
}
232+
};
233+
234+
static_assert(std::is_trivially_copyable<
235+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr>::value,
236+
"OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr should be "
237+
"trivially copyable");
238+
239+
} // end namespace clang
240+
150241
#endif // LLVM_CLANG_BASIC_DIRECTORYENTRY_H

‎clang/include/clang/Basic/Module.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,9 @@ class Module {
133133
std::string PresumedModuleMapFile;
134134

135135
/// The umbrella header or directory.
136-
llvm::PointerUnion<const FileEntry *, const DirectoryEntry *> Umbrella;
136+
llvm::PointerUnion<const FileEntryRef::MapEntry *,
137+
const DirectoryEntryRef::MapEntry *>
138+
Umbrella;
137139

138140
/// The module signature.
139141
ASTFileSignature Signature;
@@ -188,18 +190,18 @@ class Module {
188190
/// file.
189191
struct Header {
190192
std::string NameAsWritten;
191-
const FileEntry *Entry;
193+
OptionalFileEntryRefDegradesToFileEntryPtr Entry;
192194

193-
explicit operator bool() { return Entry; }
195+
explicit operator bool() { return Entry != None; }
194196
};
195197

196198
/// Information about a directory name as found in the module map
197199
/// file.
198200
struct DirectoryName {
199201
std::string NameAsWritten;
200-
const DirectoryEntry *Entry;
202+
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr Entry;
201203

202-
explicit operator bool() { return Entry; }
204+
explicit operator bool() { return Entry != None; }
203205
};
204206

205207
/// The headers that are part of this module.
@@ -544,15 +546,15 @@ class Module {
544546
/// Retrieve the header that serves as the umbrella header for this
545547
/// module.
546548
Header getUmbrellaHeader() const {
547-
if (auto *FE = Umbrella.dyn_cast<const FileEntry *>())
548-
return Header{UmbrellaAsWritten, FE};
549+
if (auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
550+
return Header{UmbrellaAsWritten, FileEntryRef(*ME)};
549551
return Header{};
550552
}
551553

552554
/// Determine whether this module has an umbrella directory that is
553555
/// not based on an umbrella header.
554556
bool hasUmbrellaDir() const {
555-
return Umbrella && Umbrella.is<const DirectoryEntry *>();
557+
return Umbrella && Umbrella.is<const DirectoryEntryRef::MapEntry *>();
556558
}
557559

558560
/// Add a top-level header associated with this module.

‎clang/include/clang/Lex/ModuleMap.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#ifndef LLVM_CLANG_LEX_MODULEMAP_H
1515
#define LLVM_CLANG_LEX_MODULEMAP_H
1616

17+
#include "clang/Basic/FileEntry.h"
1718
#include "clang/Basic/IdentifierTable.h"
1819
#include "clang/Basic/LangOptions.h"
1920
#include "clang/Basic/Module.h"
@@ -37,7 +38,6 @@ namespace clang {
3738

3839
class DiagnosticsEngine;
3940
class DirectoryEntry;
40-
class FileEntry;
4141
class FileManager;
4242
class HeaderSearch;
4343
class SourceManager;
@@ -648,12 +648,12 @@ class ModuleMap {
648648

649649
/// Sets the umbrella header of the given module to the given
650650
/// header.
651-
void setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
651+
void setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
652652
Twine NameAsWritten);
653653

654654
/// Sets the umbrella directory of the given module to the given
655655
/// directory.
656-
void setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
656+
void setUmbrellaDir(Module *Mod, DirectoryEntryRef UmbrellaDir,
657657
Twine NameAsWritten);
658658

659659
/// Adds this header to the given module.

‎clang/lib/Basic/Module.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,10 @@ Module::DirectoryName Module::getUmbrellaDir() const {
247247
if (Header U = getUmbrellaHeader())
248248
return {"", U.Entry->getDir()};
249249

250-
return {UmbrellaAsWritten, Umbrella.dyn_cast<const DirectoryEntry *>()};
250+
if (auto *ME = Umbrella.dyn_cast<const DirectoryEntryRef::MapEntry *>())
251+
return {UmbrellaAsWritten, DirectoryEntryRef(*ME)};
252+
253+
return {"", None};
251254
}
252255

253256
void Module::addTopHeader(const FileEntry *File) {

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ bool GenerateHeaderModuleAction::BeginSourceFileAction(
304304
<< Name;
305305
continue;
306306
}
307-
Headers.push_back({std::string(Name), &FE->getFileEntry()});
307+
Headers.push_back({std::string(Name), *FE});
308308
}
309309
HS.getModuleMap().createHeaderModule(CI.getLangOpts().CurrentModule, Headers);
310310

‎clang/lib/Lex/ModuleMap.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ bool ModuleMap::resolveAsBuiltinHeader(
300300
// supplied by Clang. Find that builtin header.
301301
SmallString<128> Path;
302302
llvm::sys::path::append(Path, BuiltinIncludeDir->getName(), Header.FileName);
303-
auto File = SourceMgr.getFileManager().getFile(Path);
303+
auto File = SourceMgr.getFileManager().getOptionalFileRef(Path);
304304
if (!File)
305305
return false;
306306

@@ -1012,7 +1012,7 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
10121012
// Look for an umbrella header.
10131013
SmallString<128> UmbrellaName = StringRef(FrameworkDir->getName());
10141014
llvm::sys::path::append(UmbrellaName, "Headers", ModuleName + ".h");
1015-
auto UmbrellaHeader = FileMgr.getFile(UmbrellaName);
1015+
auto UmbrellaHeader = FileMgr.getOptionalFileRef(UmbrellaName);
10161016

10171017
// FIXME: If there's no umbrella header, we could probably scan the
10181018
// framework to load *everything*. But, it's not clear that this is a good
@@ -1121,21 +1121,21 @@ Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
11211121
return Result;
11221122
}
11231123

1124-
void ModuleMap::setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
1124+
void ModuleMap::setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
11251125
Twine NameAsWritten) {
11261126
Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader));
1127-
Mod->Umbrella = UmbrellaHeader;
1127+
Mod->Umbrella = &UmbrellaHeader.getMapEntry();
11281128
Mod->UmbrellaAsWritten = NameAsWritten.str();
1129-
UmbrellaDirs[UmbrellaHeader->getDir()] = Mod;
1129+
UmbrellaDirs[UmbrellaHeader.getDir()] = Mod;
11301130

11311131
// Notify callbacks that we just added a new header.
11321132
for (const auto &Cb : Callbacks)
11331133
Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
11341134
}
11351135

1136-
void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
1136+
void ModuleMap::setUmbrellaDir(Module *Mod, DirectoryEntryRef UmbrellaDir,
11371137
Twine NameAsWritten) {
1138-
Mod->Umbrella = UmbrellaDir;
1138+
Mod->Umbrella = &UmbrellaDir.getMapEntry();
11391139
Mod->UmbrellaAsWritten = NameAsWritten.str();
11401140
UmbrellaDirs[UmbrellaDir] = Mod;
11411141
}
@@ -2416,15 +2416,15 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
24162416
}
24172417

24182418
// Look for this file.
2419-
const DirectoryEntry *Dir = nullptr;
2419+
Optional<DirectoryEntryRef> Dir;
24202420
if (llvm::sys::path::is_absolute(DirName)) {
2421-
if (auto D = SourceMgr.getFileManager().getDirectory(DirName))
2421+
if (auto D = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName))
24222422
Dir = *D;
24232423
} else {
24242424
SmallString<128> PathName;
24252425
PathName = Directory->getName();
24262426
llvm::sys::path::append(PathName, DirName);
2427-
if (auto D = SourceMgr.getFileManager().getDirectory(PathName))
2427+
if (auto D = SourceMgr.getFileManager().getOptionalDirectoryRef(PathName))
24282428
Dir = *D;
24292429
}
24302430

@@ -2445,7 +2445,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
24452445
SourceMgr.getFileManager().getVirtualFileSystem();
24462446
for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
24472447
I != E && !EC; I.increment(EC)) {
2448-
if (auto FE = SourceMgr.getFileManager().getFile(I->path())) {
2448+
if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
24492449
Module::Header Header = {std::string(I->path()), *FE};
24502450
Headers.push_back(std::move(Header));
24512451
}
@@ -2459,15 +2459,15 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
24592459
return;
24602460
}
24612461

2462-
if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {
2462+
if (Module *OwningModule = Map.UmbrellaDirs[*Dir]) {
24632463
Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)
24642464
<< OwningModule->getFullModuleName();
24652465
HadError = true;
24662466
return;
24672467
}
24682468

24692469
// Record this umbrella directory.
2470-
Map.setUmbrellaDir(ActiveModule, Dir, DirName);
2470+
Map.setUmbrellaDir(ActiveModule, *Dir, DirName);
24712471
}
24722472

24732473
/// Parse a module export declaration.

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,7 +1915,8 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
19151915
// FIXME: This is not always the right filename-as-written, but we're not
19161916
// going to use this information to rebuild the module, so it doesn't make
19171917
// a lot of difference.
1918-
Module::Header H = {std::string(key.Filename), *FileMgr.getFile(Filename)};
1918+
Module::Header H = {std::string(key.Filename),
1919+
*FileMgr.getOptionalFileRef(Filename)};
19191920
ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
19201921
HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
19211922
}
@@ -5564,7 +5565,7 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
55645565
case SUBMODULE_UMBRELLA_HEADER: {
55655566
std::string Filename = std::string(Blob);
55665567
ResolveImportedPath(F, Filename);
5567-
if (auto Umbrella = PP.getFileManager().getFile(Filename)) {
5568+
if (auto Umbrella = PP.getFileManager().getOptionalFileRef(Filename)) {
55685569
if (!CurrentModule->getUmbrellaHeader())
55695570
ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob);
55705571
else if (CurrentModule->getUmbrellaHeader().Entry != *Umbrella) {
@@ -5597,7 +5598,8 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
55975598
case SUBMODULE_UMBRELLA_DIR: {
55985599
std::string Dirname = std::string(Blob);
55995600
ResolveImportedPath(F, Dirname);
5600-
if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
5601+
if (auto Umbrella =
5602+
PP.getFileManager().getOptionalDirectoryRef(Dirname)) {
56015603
if (!CurrentModule->getUmbrellaDir())
56025604
ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob);
56035605
else if (CurrentModule->getUmbrellaDir().Entry != *Umbrella) {

0 commit comments

Comments
 (0)