Skip to content

Commit e6830b6

Browse files
jansvoboda11jansvoboda11
committed
[clang][modules] NFCI: Extract optionality out of Module::{Header,DirectoryName}
Most users of `Module::Header` already assume its `Entry` is populated. Enforce this assumption in the type system and handle the only case where this is not the case by wrapping the whole struct in `std::optional`. Do the same for `Module::DirectoryName`. Depends on D151584. Reviewed By: benlangmuir Differential Revision: https://reviews.llvm.org/D151586
1 parent 0442d08 commit e6830b6

File tree

9 files changed

+56
-54
lines changed

9 files changed

+56
-54
lines changed

‎clang-tools-extra/modularize/CoverageChecker.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -207,25 +207,25 @@ void CoverageChecker::collectModuleHeaders() {
207207
// FIXME: Doesn't collect files from umbrella header.
208208
bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
209209

210-
if (const FileEntry *UmbrellaHeader =
211-
Mod.getUmbrellaHeaderAsWritten().Entry) {
210+
if (std::optional<Module::Header> UmbrellaHeader =
211+
Mod.getUmbrellaHeaderAsWritten()) {
212212
// Collect umbrella header.
213-
ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
214-
UmbrellaHeader->getName()));
213+
ModuleMapHeadersSet.insert(
214+
ModularizeUtilities::getCanonicalPath(UmbrellaHeader->Entry.getName()));
215215
// Preprocess umbrella header and collect the headers it references.
216-
if (!collectUmbrellaHeaderHeaders(UmbrellaHeader->getName()))
216+
if (!collectUmbrellaHeaderHeaders(UmbrellaHeader->Entry.getName()))
217217
return false;
218-
} else if (const DirectoryEntry *UmbrellaDir =
219-
Mod.getUmbrellaDirAsWritten().Entry) {
218+
} else if (std::optional<Module::DirectoryName> UmbrellaDir =
219+
Mod.getUmbrellaDirAsWritten()) {
220220
// Collect headers in umbrella directory.
221-
if (!collectUmbrellaHeaders(UmbrellaDir->getName()))
221+
if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName()))
222222
return false;
223223
}
224224

225225
for (auto &HeaderKind : Mod.Headers)
226226
for (auto &Header : HeaderKind)
227-
ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
228-
Header.Entry->getName()));
227+
ModuleMapHeadersSet.insert(
228+
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));
229229

230230
for (auto *Submodule : Mod.submodules())
231231
collectModuleHeaders(*Submodule);

‎clang-tools-extra/modularize/ModularizeUtilities.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -348,19 +348,20 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
348348
for (auto *Submodule : Mod.submodules())
349349
collectModuleHeaders(*Submodule);
350350

351-
if (const FileEntry *UmbrellaHeader =
352-
Mod.getUmbrellaHeaderAsWritten().Entry) {
353-
std::string HeaderPath = getCanonicalPath(UmbrellaHeader->getName());
351+
if (std::optional<Module::Header> UmbrellaHeader =
352+
Mod.getUmbrellaHeaderAsWritten()) {
353+
std::string HeaderPath = getCanonicalPath(UmbrellaHeader->Entry.getName());
354354
// Collect umbrella header.
355355
HeaderFileNames.push_back(HeaderPath);
356356

357357
// FUTURE: When needed, umbrella header header collection goes here.
358-
} else if (const DirectoryEntry *UmbrellaDir =
359-
Mod.getUmbrellaDirAsWritten().Entry) {
358+
} else if (std::optional<Module::DirectoryName> UmbrellaDir =
359+
Mod.getUmbrellaDirAsWritten()) {
360360
// If there normal headers, assume these are umbrellas and skip collection.
361361
if (Mod.Headers->size() == 0) {
362362
// Collect headers in umbrella directory.
363-
if (!collectUmbrellaHeaders(UmbrellaDir->getName(), UmbrellaDependents))
363+
if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName(),
364+
UmbrellaDependents))
364365
return false;
365366
}
366367
}
@@ -377,7 +378,7 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
377378
// Collect normal header.
378379
const clang::Module::Header &Header(
379380
Mod.Headers[clang::Module::HK_Normal][Index]);
380-
std::string HeaderPath = getCanonicalPath(Header.Entry->getName());
381+
std::string HeaderPath = getCanonicalPath(Header.Entry.getName());
381382
HeaderFileNames.push_back(HeaderPath);
382383
}
383384

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -243,19 +243,15 @@ class alignas(8) Module {
243243
struct Header {
244244
std::string NameAsWritten;
245245
std::string PathRelativeToRootModuleDirectory;
246-
OptionalFileEntryRefDegradesToFileEntryPtr Entry;
247-
248-
explicit operator bool() { return Entry.has_value(); }
246+
FileEntryRef Entry;
249247
};
250248

251249
/// Information about a directory name as found in the module map
252250
/// file.
253251
struct DirectoryName {
254252
std::string NameAsWritten;
255253
std::string PathRelativeToRootModuleDirectory;
256-
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr Entry;
257-
258-
explicit operator bool() { return Entry.has_value(); }
254+
DirectoryEntryRef Entry;
259255
};
260256

261257
/// The headers that are part of this module.
@@ -653,21 +649,21 @@ class alignas(8) Module {
653649
}
654650

655651
/// Retrieve the umbrella directory as written.
656-
DirectoryName getUmbrellaDirAsWritten() const {
652+
std::optional<DirectoryName> getUmbrellaDirAsWritten() const {
657653
if (const auto *ME =
658654
Umbrella.dyn_cast<const DirectoryEntryRef::MapEntry *>())
659655
return DirectoryName{UmbrellaAsWritten,
660656
UmbrellaRelativeToRootModuleDirectory,
661657
DirectoryEntryRef(*ME)};
662-
return DirectoryName{};
658+
return std::nullopt;
663659
}
664660

665661
/// Retrieve the umbrella header as written.
666-
Header getUmbrellaHeaderAsWritten() const {
662+
std::optional<Header> getUmbrellaHeaderAsWritten() const {
667663
if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
668664
return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
669665
FileEntryRef(*ME)};
670-
return Header{};
666+
return std::nullopt;
671667
}
672668

673669
/// Get the effective umbrella directory for this module: either the one

‎clang/lib/Basic/Module.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -483,15 +483,15 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {
483483
OS << "\n";
484484
}
485485

486-
if (Header H = getUmbrellaHeaderAsWritten()) {
486+
if (std::optional<Header> H = getUmbrellaHeaderAsWritten()) {
487487
OS.indent(Indent + 2);
488488
OS << "umbrella header \"";
489-
OS.write_escaped(H.NameAsWritten);
489+
OS.write_escaped(H->NameAsWritten);
490490
OS << "\"\n";
491-
} else if (DirectoryName D = getUmbrellaDirAsWritten()) {
491+
} else if (std::optional<DirectoryName> D = getUmbrellaDirAsWritten()) {
492492
OS.indent(Indent + 2);
493493
OS << "umbrella \"";
494-
OS.write_escaped(D.NameAsWritten);
494+
OS.write_escaped(D->NameAsWritten);
495495
OS << "\"\n";
496496
}
497497

@@ -523,8 +523,8 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {
523523
OS.indent(Indent + 2);
524524
OS << K.Prefix << "header \"";
525525
OS.write_escaped(H.NameAsWritten);
526-
OS << "\" { size " << H.Entry->getSize()
527-
<< " mtime " << H.Entry->getModificationTime() << " }\n";
526+
OS << "\" { size " << H.Entry.getSize()
527+
<< " mtime " << H.Entry.getModificationTime() << " }\n";
528528
}
529529
}
530530
for (auto *Unresolved : {&UnresolvedHeaders, &MissingHeaders}) {

‎clang/lib/Frontend/FrontendAction.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -364,18 +364,19 @@ static std::error_code collectModuleHeaderIncludes(
364364
}
365365
// Note that Module->PrivateHeaders will not be a TopHeader.
366366

367-
if (Module::Header UmbrellaHeader = Module->getUmbrellaHeaderAsWritten()) {
368-
Module->addTopHeader(UmbrellaHeader.Entry);
367+
if (std::optional<Module::Header> UmbrellaHeader =
368+
Module->getUmbrellaHeaderAsWritten()) {
369+
Module->addTopHeader(UmbrellaHeader->Entry);
369370
if (Module->Parent)
370371
// Include the umbrella header for submodules.
371-
addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
372+
addHeaderInclude(UmbrellaHeader->PathRelativeToRootModuleDirectory,
372373
Includes, LangOpts, Module->IsExternC);
373-
} else if (Module::DirectoryName UmbrellaDir =
374+
} else if (std::optional<Module::DirectoryName> UmbrellaDir =
374375
Module->getUmbrellaDirAsWritten()) {
375376
// Add all of the headers we find in this subdirectory.
376377
std::error_code EC;
377378
SmallString<128> DirNative;
378-
llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
379+
llvm::sys::path::native(UmbrellaDir->Entry.getName(), DirNative);
379380

380381
llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
381382
SmallVector<
@@ -407,7 +408,7 @@ static std::error_code collectModuleHeaderIncludes(
407408
for (int I = 0; I != Dir.level() + 1; ++I, ++PathIt)
408409
Components.push_back(*PathIt);
409410
SmallString<128> RelativeHeader(
410-
UmbrellaDir.PathRelativeToRootModuleDirectory);
411+
UmbrellaDir->PathRelativeToRootModuleDirectory);
411412
for (auto It = Components.rbegin(), End = Components.rend(); It != End;
412413
++It)
413414
llvm::sys::path::append(RelativeHeader, *It);
@@ -553,8 +554,9 @@ getInputBufferForModule(CompilerInstance &CI, Module *M) {
553554
// Collect the set of #includes we need to build the module.
554555
SmallString<256> HeaderContents;
555556
std::error_code Err = std::error_code();
556-
if (Module::Header UmbrellaHeader = M->getUmbrellaHeaderAsWritten())
557-
addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
557+
if (std::optional<Module::Header> UmbrellaHeader =
558+
M->getUmbrellaHeaderAsWritten())
559+
addHeaderInclude(UmbrellaHeader->PathRelativeToRootModuleDirectory,
558560
HeaderContents, CI.getLangOpts(), M->IsExternC);
559561
Err = collectModuleHeaderIncludes(
560562
CI.getLangOpts(), FileMgr, CI.getDiagnostics(),

‎clang/lib/Lex/ModuleMap.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,7 +1289,7 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header,
12891289

12901290
// Notify callbacks that we just added a new header.
12911291
for (const auto &Cb : Callbacks)
1292-
Cb->moduleMapAddHeader(Header.Entry->getName());
1292+
Cb->moduleMapAddHeader(Header.Entry.getName());
12931293
}
12941294

12951295
OptionalFileEntryRef
@@ -2541,7 +2541,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
25412541
for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
25422542
I != E && !EC; I.increment(EC)) {
25432543
if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
2544-
Module::Header Header = {"", std::string(I->path()), FE};
2544+
Module::Header Header = {"", std::string(I->path()), *FE};
25452545
Headers.push_back(std::move(Header));
25462546
}
25472547
}

‎clang/lib/Lex/PPLexerChange.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,10 @@ static void collectAllSubModulesWithUmbrellaHeader(
289289
}
290290

291291
void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
292-
Module::Header UmbrellaHeader = Mod.getUmbrellaHeaderAsWritten();
293-
assert(UmbrellaHeader.Entry && "Module must use umbrella header");
294-
const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
292+
std::optional<Module::Header> UmbrellaHeader =
293+
Mod.getUmbrellaHeaderAsWritten();
294+
assert(UmbrellaHeader && "Module must use umbrella header");
295+
const FileID &File = SourceMgr.translateFile(UmbrellaHeader->Entry);
295296
SourceLocation ExpectedHeadersLoc = SourceMgr.getLocForEndOfFile(File);
296297
if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header,
297298
ExpectedHeadersLoc))

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,10 +1973,11 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
19731973
std::string Filename = std::string(key.Filename);
19741974
if (key.Imported)
19751975
Reader.ResolveImportedPath(M, Filename);
1976-
// FIXME: NameAsWritten
1977-
Module::Header H = {std::string(key.Filename), "",
1978-
FileMgr.getOptionalFileRef(Filename)};
1979-
ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
1976+
if (auto FE = FileMgr.getOptionalFileRef(Filename)) {
1977+
// FIXME: NameAsWritten
1978+
Module::Header H = {std::string(key.Filename), "", *FE};
1979+
ModMap.addHeader(Mod, H, HeaderRole, /*Imported=*/true);
1980+
}
19801981
HFI.isModuleHeader |= ModuleMap::isModular(HeaderRole);
19811982
}
19821983

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2846,15 +2846,16 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
28462846
}
28472847

28482848
// Emit the umbrella header, if there is one.
2849-
if (Module::Header UmbrellaHeader = Mod->getUmbrellaHeaderAsWritten()) {
2849+
if (std::optional<Module::Header> UmbrellaHeader =
2850+
Mod->getUmbrellaHeaderAsWritten()) {
28502851
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
28512852
Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
2852-
UmbrellaHeader.NameAsWritten);
2853-
} else if (Module::DirectoryName UmbrellaDir =
2853+
UmbrellaHeader->NameAsWritten);
2854+
} else if (std::optional<Module::DirectoryName> UmbrellaDir =
28542855
Mod->getUmbrellaDirAsWritten()) {
28552856
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
28562857
Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
2857-
UmbrellaDir.NameAsWritten);
2858+
UmbrellaDir->NameAsWritten);
28582859
}
28592860

28602861
// Emit the headers.

0 commit comments

Comments
 (0)