[clang] e6830b6 - [clang][modules] NFCI: Extract optionality out of `Module::{Header,DirectoryName}`
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Tue May 30 21:24:35 PDT 2023
Author: Jan Svoboda
Date: 2023-05-30T21:06:51-07:00
New Revision: e6830b6028ec5434ccf8dbebdd992918f67b1751
URL: https://github.com/llvm/llvm-project/commit/e6830b6028ec5434ccf8dbebdd992918f67b1751
DIFF: https://github.com/llvm/llvm-project/commit/e6830b6028ec5434ccf8dbebdd992918f67b1751.diff
LOG: [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
Added:
Modified:
clang-tools-extra/modularize/CoverageChecker.cpp
clang-tools-extra/modularize/ModularizeUtilities.cpp
clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Lex/PPLexerChange.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/modularize/CoverageChecker.cpp b/clang-tools-extra/modularize/CoverageChecker.cpp
index 83c39e495c31a..d8445053872bf 100644
--- a/clang-tools-extra/modularize/CoverageChecker.cpp
+++ b/clang-tools-extra/modularize/CoverageChecker.cpp
@@ -207,25 +207,25 @@ void CoverageChecker::collectModuleHeaders() {
// FIXME: Doesn't collect files from umbrella header.
bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
- if (const FileEntry *UmbrellaHeader =
- Mod.getUmbrellaHeaderAsWritten().Entry) {
+ if (std::optional<Module::Header> UmbrellaHeader =
+ Mod.getUmbrellaHeaderAsWritten()) {
// Collect umbrella header.
- ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
- UmbrellaHeader->getName()));
+ ModuleMapHeadersSet.insert(
+ ModularizeUtilities::getCanonicalPath(UmbrellaHeader->Entry.getName()));
// Preprocess umbrella header and collect the headers it references.
- if (!collectUmbrellaHeaderHeaders(UmbrellaHeader->getName()))
+ if (!collectUmbrellaHeaderHeaders(UmbrellaHeader->Entry.getName()))
return false;
- } else if (const DirectoryEntry *UmbrellaDir =
- Mod.getUmbrellaDirAsWritten().Entry) {
+ } else if (std::optional<Module::DirectoryName> UmbrellaDir =
+ Mod.getUmbrellaDirAsWritten()) {
// Collect headers in umbrella directory.
- if (!collectUmbrellaHeaders(UmbrellaDir->getName()))
+ if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName()))
return false;
}
for (auto &HeaderKind : Mod.Headers)
for (auto &Header : HeaderKind)
- ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
- Header.Entry->getName()));
+ ModuleMapHeadersSet.insert(
+ ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));
for (auto *Submodule : Mod.submodules())
collectModuleHeaders(*Submodule);
diff --git a/clang-tools-extra/modularize/ModularizeUtilities.cpp b/clang-tools-extra/modularize/ModularizeUtilities.cpp
index 5b09c916606d9..3ef808d204c61 100644
--- a/clang-tools-extra/modularize/ModularizeUtilities.cpp
+++ b/clang-tools-extra/modularize/ModularizeUtilities.cpp
@@ -348,19 +348,20 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
for (auto *Submodule : Mod.submodules())
collectModuleHeaders(*Submodule);
- if (const FileEntry *UmbrellaHeader =
- Mod.getUmbrellaHeaderAsWritten().Entry) {
- std::string HeaderPath = getCanonicalPath(UmbrellaHeader->getName());
+ if (std::optional<Module::Header> UmbrellaHeader =
+ Mod.getUmbrellaHeaderAsWritten()) {
+ std::string HeaderPath = getCanonicalPath(UmbrellaHeader->Entry.getName());
// Collect umbrella header.
HeaderFileNames.push_back(HeaderPath);
// FUTURE: When needed, umbrella header header collection goes here.
- } else if (const DirectoryEntry *UmbrellaDir =
- Mod.getUmbrellaDirAsWritten().Entry) {
+ } else if (std::optional<Module::DirectoryName> UmbrellaDir =
+ Mod.getUmbrellaDirAsWritten()) {
// If there normal headers, assume these are umbrellas and skip collection.
if (Mod.Headers->size() == 0) {
// Collect headers in umbrella directory.
- if (!collectUmbrellaHeaders(UmbrellaDir->getName(), UmbrellaDependents))
+ if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName(),
+ UmbrellaDependents))
return false;
}
}
@@ -377,7 +378,7 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
// Collect normal header.
const clang::Module::Header &Header(
Mod.Headers[clang::Module::HK_Normal][Index]);
- std::string HeaderPath = getCanonicalPath(Header.Entry->getName());
+ std::string HeaderPath = getCanonicalPath(Header.Entry.getName());
HeaderFileNames.push_back(HeaderPath);
}
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 3ecab422bc42c..9625a682c3549 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -243,9 +243,7 @@ class alignas(8) Module {
struct Header {
std::string NameAsWritten;
std::string PathRelativeToRootModuleDirectory;
- OptionalFileEntryRefDegradesToFileEntryPtr Entry;
-
- explicit operator bool() { return Entry.has_value(); }
+ FileEntryRef Entry;
};
/// Information about a directory name as found in the module map
@@ -253,9 +251,7 @@ class alignas(8) Module {
struct DirectoryName {
std::string NameAsWritten;
std::string PathRelativeToRootModuleDirectory;
- OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr Entry;
-
- explicit operator bool() { return Entry.has_value(); }
+ DirectoryEntryRef Entry;
};
/// The headers that are part of this module.
@@ -653,21 +649,21 @@ class alignas(8) Module {
}
/// Retrieve the umbrella directory as written.
- DirectoryName getUmbrellaDirAsWritten() const {
+ std::optional<DirectoryName> getUmbrellaDirAsWritten() const {
if (const auto *ME =
Umbrella.dyn_cast<const DirectoryEntryRef::MapEntry *>())
return DirectoryName{UmbrellaAsWritten,
UmbrellaRelativeToRootModuleDirectory,
DirectoryEntryRef(*ME)};
- return DirectoryName{};
+ return std::nullopt;
}
/// Retrieve the umbrella header as written.
- Header getUmbrellaHeaderAsWritten() const {
+ std::optional<Header> getUmbrellaHeaderAsWritten() const {
if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
FileEntryRef(*ME)};
- return Header{};
+ return std::nullopt;
}
/// Get the effective umbrella directory for this module: either the one
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 3df376a32e53e..057fc77d0e993 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -483,15 +483,15 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {
OS << "\n";
}
- if (Header H = getUmbrellaHeaderAsWritten()) {
+ if (std::optional<Header> H = getUmbrellaHeaderAsWritten()) {
OS.indent(Indent + 2);
OS << "umbrella header \"";
- OS.write_escaped(H.NameAsWritten);
+ OS.write_escaped(H->NameAsWritten);
OS << "\"\n";
- } else if (DirectoryName D = getUmbrellaDirAsWritten()) {
+ } else if (std::optional<DirectoryName> D = getUmbrellaDirAsWritten()) {
OS.indent(Indent + 2);
OS << "umbrella \"";
- OS.write_escaped(D.NameAsWritten);
+ OS.write_escaped(D->NameAsWritten);
OS << "\"\n";
}
@@ -523,8 +523,8 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {
OS.indent(Indent + 2);
OS << K.Prefix << "header \"";
OS.write_escaped(H.NameAsWritten);
- OS << "\" { size " << H.Entry->getSize()
- << " mtime " << H.Entry->getModificationTime() << " }\n";
+ OS << "\" { size " << H.Entry.getSize()
+ << " mtime " << H.Entry.getModificationTime() << " }\n";
}
}
for (auto *Unresolved : {&UnresolvedHeaders, &MissingHeaders}) {
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index a8dcdb44b08df..c5893874e1d32 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -364,18 +364,19 @@ static std::error_code collectModuleHeaderIncludes(
}
// Note that Module->PrivateHeaders will not be a TopHeader.
- if (Module::Header UmbrellaHeader = Module->getUmbrellaHeaderAsWritten()) {
- Module->addTopHeader(UmbrellaHeader.Entry);
+ if (std::optional<Module::Header> UmbrellaHeader =
+ Module->getUmbrellaHeaderAsWritten()) {
+ Module->addTopHeader(UmbrellaHeader->Entry);
if (Module->Parent)
// Include the umbrella header for submodules.
- addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
+ addHeaderInclude(UmbrellaHeader->PathRelativeToRootModuleDirectory,
Includes, LangOpts, Module->IsExternC);
- } else if (Module::DirectoryName UmbrellaDir =
+ } else if (std::optional<Module::DirectoryName> UmbrellaDir =
Module->getUmbrellaDirAsWritten()) {
// Add all of the headers we find in this subdirectory.
std::error_code EC;
SmallString<128> DirNative;
- llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
+ llvm::sys::path::native(UmbrellaDir->Entry.getName(), DirNative);
llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
SmallVector<
@@ -407,7 +408,7 @@ static std::error_code collectModuleHeaderIncludes(
for (int I = 0; I != Dir.level() + 1; ++I, ++PathIt)
Components.push_back(*PathIt);
SmallString<128> RelativeHeader(
- UmbrellaDir.PathRelativeToRootModuleDirectory);
+ UmbrellaDir->PathRelativeToRootModuleDirectory);
for (auto It = Components.rbegin(), End = Components.rend(); It != End;
++It)
llvm::sys::path::append(RelativeHeader, *It);
@@ -553,8 +554,9 @@ getInputBufferForModule(CompilerInstance &CI, Module *M) {
// Collect the set of #includes we need to build the module.
SmallString<256> HeaderContents;
std::error_code Err = std::error_code();
- if (Module::Header UmbrellaHeader = M->getUmbrellaHeaderAsWritten())
- addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
+ if (std::optional<Module::Header> UmbrellaHeader =
+ M->getUmbrellaHeaderAsWritten())
+ addHeaderInclude(UmbrellaHeader->PathRelativeToRootModuleDirectory,
HeaderContents, CI.getLangOpts(), M->IsExternC);
Err = collectModuleHeaderIncludes(
CI.getLangOpts(), FileMgr, CI.getDiagnostics(),
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 6808fdfdaf4f9..bfd4890e3a97b 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1289,7 +1289,7 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header,
// Notify callbacks that we just added a new header.
for (const auto &Cb : Callbacks)
- Cb->moduleMapAddHeader(Header.Entry->getName());
+ Cb->moduleMapAddHeader(Header.Entry.getName());
}
OptionalFileEntryRef
@@ -2541,7 +2541,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
I != E && !EC; I.increment(EC)) {
if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
- Module::Header Header = {"", std::string(I->path()), FE};
+ Module::Header Header = {"", std::string(I->path()), *FE};
Headers.push_back(std::move(Header));
}
}
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index 4103cfe178b29..e2dc532e6b708 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -289,9 +289,10 @@ static void collectAllSubModulesWithUmbrellaHeader(
}
void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
- Module::Header UmbrellaHeader = Mod.getUmbrellaHeaderAsWritten();
- assert(UmbrellaHeader.Entry && "Module must use umbrella header");
- const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
+ std::optional<Module::Header> UmbrellaHeader =
+ Mod.getUmbrellaHeaderAsWritten();
+ assert(UmbrellaHeader && "Module must use umbrella header");
+ const FileID &File = SourceMgr.translateFile(UmbrellaHeader->Entry);
SourceLocation ExpectedHeadersLoc = SourceMgr.getLocForEndOfFile(File);
if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header,
ExpectedHeadersLoc))
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index f6251fb03ccf2..a0ccc5aa4a741 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1973,10 +1973,11 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
std::string Filename = std::string(key.Filename);
if (key.Imported)
Reader.ResolveImportedPath(M, Filename);
- // FIXME: NameAsWritten
- Module::Header H = {std::string(key.Filename), "",
- FileMgr.getOptionalFileRef(Filename)};
- ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
+ if (auto FE = FileMgr.getOptionalFileRef(Filename)) {
+ // FIXME: NameAsWritten
+ Module::Header H = {std::string(key.Filename), "", *FE};
+ ModMap.addHeader(Mod, H, HeaderRole, /*Imported=*/true);
+ }
HFI.isModuleHeader |= ModuleMap::isModular(HeaderRole);
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 96b087ed57933..21528f8140eb1 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2846,15 +2846,16 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
}
// Emit the umbrella header, if there is one.
- if (Module::Header UmbrellaHeader = Mod->getUmbrellaHeaderAsWritten()) {
+ if (std::optional<Module::Header> UmbrellaHeader =
+ Mod->getUmbrellaHeaderAsWritten()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
- UmbrellaHeader.NameAsWritten);
- } else if (Module::DirectoryName UmbrellaDir =
+ UmbrellaHeader->NameAsWritten);
+ } else if (std::optional<Module::DirectoryName> UmbrellaDir =
Mod->getUmbrellaDirAsWritten()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
- UmbrellaDir.NameAsWritten);
+ UmbrellaDir->NameAsWritten);
}
// Emit the headers.
More information about the cfe-commits
mailing list