[clang] 9249129 - [clang][modules] NFCI: Distinguish as-written and effective umbrella directories
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Fri May 26 15:14:23 PDT 2023
Author: Jan Svoboda
Date: 2023-05-26T15:14:16-07:00
New Revision: 924912956ed570e433440108cc50bd0ee65605b5
URL: https://github.com/llvm/llvm-project/commit/924912956ed570e433440108cc50bd0ee65605b5
DIFF: https://github.com/llvm/llvm-project/commit/924912956ed570e433440108cc50bd0ee65605b5.diff
LOG: [clang][modules] NFCI: Distinguish as-written and effective umbrella directories
For modules with umbrellas, we track how they were written in the module map. Unfortunately, the getter for the umbrella directory conflates the "as written" directory and the "effective" directory (either the written one or the parent of the written umbrella header).
This patch makes the distinction between "as written" and "effective" umbrella directories clearer. No functional change intended.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D151581
Added:
Modified:
clang-tools-extra/modularize/CoverageChecker.cpp
clang-tools-extra/modularize/ModularizeUtilities.cpp
clang/include/clang/Basic/Module.h
clang/include/clang/Lex/ModuleMap.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 900a88df68ca2..83c39e495c31a 100644
--- a/clang-tools-extra/modularize/CoverageChecker.cpp
+++ b/clang-tools-extra/modularize/CoverageChecker.cpp
@@ -207,15 +207,16 @@ void CoverageChecker::collectModuleHeaders() {
// FIXME: Doesn't collect files from umbrella header.
bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
- if (const FileEntry *UmbrellaHeader = Mod.getUmbrellaHeader().Entry) {
+ if (const FileEntry *UmbrellaHeader =
+ Mod.getUmbrellaHeaderAsWritten().Entry) {
// Collect umbrella header.
ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
UmbrellaHeader->getName()));
// Preprocess umbrella header and collect the headers it references.
if (!collectUmbrellaHeaderHeaders(UmbrellaHeader->getName()))
return false;
- }
- else if (const DirectoryEntry *UmbrellaDir = Mod.getUmbrellaDir().Entry) {
+ } else if (const DirectoryEntry *UmbrellaDir =
+ Mod.getUmbrellaDirAsWritten().Entry) {
// Collect headers in umbrella directory.
if (!collectUmbrellaHeaders(UmbrellaDir->getName()))
return false;
diff --git a/clang-tools-extra/modularize/ModularizeUtilities.cpp b/clang-tools-extra/modularize/ModularizeUtilities.cpp
index e3f9a6eba8f6a..a7cadf818664a 100644
--- a/clang-tools-extra/modularize/ModularizeUtilities.cpp
+++ b/clang-tools-extra/modularize/ModularizeUtilities.cpp
@@ -349,14 +349,15 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
for (auto *Submodule : Mod.submodules())
collectModuleHeaders(*Submodule);
- if (const FileEntry *UmbrellaHeader = Mod.getUmbrellaHeader().Entry) {
+ if (const FileEntry *UmbrellaHeader =
+ Mod.getUmbrellaHeaderAsWritten().Entry) {
std::string HeaderPath = getCanonicalPath(UmbrellaHeader->getName());
// Collect umbrella header.
HeaderFileNames.push_back(HeaderPath);
// FUTURE: When needed, umbrella header header collection goes here.
- }
- else if (const DirectoryEntry *UmbrellaDir = Mod.getUmbrellaDir().Entry) {
+ } else if (const DirectoryEntry *UmbrellaDir =
+ Mod.getUmbrellaDirAsWritten().Entry) {
// If there normal headers, assume these are umbrellas and skip collection.
if (Mod.Headers->size() == 0) {
// Collect headers in umbrella directory.
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 7ed9a2b2ca386..b9105441b6b62 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -651,19 +651,27 @@ class alignas(8) Module {
getTopLevelModule()->ASTFile = File;
}
- /// Retrieve the directory for which this module serves as the
- /// umbrella.
- DirectoryName getUmbrellaDir() const;
+ /// Retrieve the umbrella directory as written.
+ DirectoryName getUmbrellaDirAsWritten() const {
+ if (const auto *ME = Umbrella.dyn_cast<const DirectoryEntry *>())
+ return DirectoryName{UmbrellaAsWritten,
+ UmbrellaRelativeToRootModuleDirectory, ME};
+ return DirectoryName{};
+ }
- /// Retrieve the header that serves as the umbrella header for this
- /// module.
- Header getUmbrellaHeader() const {
- if (auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
+ /// Retrieve the umbrella header as written.
+ Header getUmbrellaHeaderAsWritten() const {
+ if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
FileEntryRef(*ME)};
return Header{};
}
+ /// Get the effective umbrella directory for this module: either the one
+ /// explicitly written in the module map file, or the parent of the umbrella
+ /// header.
+ const DirectoryEntry *getEffectiveUmbrellaDir() const;
+
/// Determine whether this module has an umbrella directory that is
/// not based on an umbrella header.
bool hasUmbrellaDir() const {
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index 8f8580fd11495..bd7e4e6ff8fe7 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -692,17 +692,16 @@ class ModuleMap {
/// false otherwise.
bool resolveConflicts(Module *Mod, bool Complain);
- /// Sets the umbrella header of the given module to the given
- /// header.
- void setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
- const Twine &NameAsWritten,
- const Twine &PathRelativeToRootModuleDirectory);
-
- /// Sets the umbrella directory of the given module to the given
- /// directory.
- void setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
- const Twine &NameAsWritten,
- const Twine &PathRelativeToRootModuleDirectory);
+ /// Sets the umbrella header of the given module to the given header.
+ void
+ setUmbrellaHeaderAsWritten(Module *Mod, FileEntryRef UmbrellaHeader,
+ const Twine &NameAsWritten,
+ const Twine &PathRelativeToRootModuleDirectory);
+
+ /// Sets the umbrella directory of the given module to the given directory.
+ void setUmbrellaDirAsWritten(Module *Mod, const DirectoryEntry *UmbrellaDir,
+ const Twine &NameAsWritten,
+ const Twine &PathRelativeToRootModuleDirectory);
/// Adds this header to the given module.
/// \param Role The role of the header wrt the module.
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 12ca19d8c6eb8..e0bce04412a2f 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -263,12 +263,12 @@ bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {
return nameParts.empty();
}
-Module::DirectoryName Module::getUmbrellaDir() const {
- if (Header U = getUmbrellaHeader())
- return {"", "", U.Entry->getDir()};
-
- return {UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
- Umbrella.dyn_cast<const DirectoryEntry *>()};
+const DirectoryEntry *Module::getEffectiveUmbrellaDir() const {
+ if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
+ return FileEntryRef(*ME).getDir();
+ if (const auto *ME = Umbrella.dyn_cast<const DirectoryEntry *>())
+ return ME;
+ return nullptr;
}
void Module::addTopHeader(const FileEntry *File) {
@@ -483,12 +483,12 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {
OS << "\n";
}
- if (Header H = getUmbrellaHeader()) {
+ if (Header H = getUmbrellaHeaderAsWritten()) {
OS.indent(Indent + 2);
OS << "umbrella header \"";
OS.write_escaped(H.NameAsWritten);
OS << "\"\n";
- } else if (DirectoryName D = getUmbrellaDir()) {
+ } else if (DirectoryName D = getUmbrellaDirAsWritten()) {
OS.indent(Indent + 2);
OS << "umbrella \"";
OS.write_escaped(D.NameAsWritten);
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index a785ad88f122f..bd6d1b03e8f30 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -364,13 +364,14 @@ static std::error_code collectModuleHeaderIncludes(
}
// Note that Module->PrivateHeaders will not be a TopHeader.
- if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader()) {
+ if (Module::Header UmbrellaHeader = Module->getUmbrellaHeaderAsWritten()) {
Module->addTopHeader(UmbrellaHeader.Entry);
if (Module->Parent)
// Include the umbrella header for submodules.
addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
Includes, LangOpts, Module->IsExternC);
- } else if (Module::DirectoryName UmbrellaDir = Module->getUmbrellaDir()) {
+ } else if (Module::DirectoryName UmbrellaDir =
+ Module->getUmbrellaDirAsWritten()) {
// Add all of the headers we find in this subdirectory.
std::error_code EC;
SmallString<128> DirNative;
@@ -550,7 +551,7 @@ 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->getUmbrellaHeader())
+ if (Module::Header UmbrellaHeader = M->getUmbrellaHeaderAsWritten())
addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
HeaderContents, CI.getLangOpts(), M->IsExternC);
Err = collectModuleHeaderIncludes(
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 66e9a7e1dbea7..9bc1ccd02eb13 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -266,7 +266,8 @@ void ModuleMap::resolveHeader(Module *Mod,
<< UmbrellaMod->getFullModuleName();
else
// Record this umbrella header.
- setUmbrellaHeader(Mod, *File, Header.FileName, RelativePathName.str());
+ setUmbrellaHeaderAsWritten(Mod, *File, Header.FileName,
+ RelativePathName.str());
} else {
Module::Header H = {Header.FileName, std::string(RelativePathName.str()),
*File};
@@ -622,7 +623,7 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
// Search up the module stack until we find a module with an umbrella
// directory.
Module *UmbrellaModule = Result;
- while (!UmbrellaModule->getUmbrellaDir() && UmbrellaModule->Parent)
+ while (!UmbrellaModule->getEffectiveUmbrellaDir() && UmbrellaModule->Parent)
UmbrellaModule = UmbrellaModule->Parent;
if (UmbrellaModule->InferSubmodules) {
@@ -760,7 +761,8 @@ ModuleMap::isHeaderUnavailableInModule(const FileEntry *Header,
// Search up the module stack until we find a module with an umbrella
// directory.
Module *UmbrellaModule = Found;
- while (!UmbrellaModule->getUmbrellaDir() && UmbrellaModule->Parent)
+ while (!UmbrellaModule->getEffectiveUmbrellaDir() &&
+ UmbrellaModule->Parent)
UmbrellaModule = UmbrellaModule->Parent;
if (UmbrellaModule->InferSubmodules) {
@@ -1089,7 +1091,8 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
RelativePath = llvm::sys::path::relative_path(RelativePath);
// umbrella header "umbrella-header-name"
- setUmbrellaHeader(Result, *UmbrellaHeader, ModuleName + ".h", RelativePath);
+ setUmbrellaHeaderAsWritten(Result, *UmbrellaHeader, ModuleName + ".h",
+ RelativePath);
// export *
Result->Exports.push_back(Module::ExportDecl(nullptr, true));
@@ -1167,7 +1170,7 @@ Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
return Result;
}
-void ModuleMap::setUmbrellaHeader(
+void ModuleMap::setUmbrellaHeaderAsWritten(
Module *Mod, FileEntryRef UmbrellaHeader, const Twine &NameAsWritten,
const Twine &PathRelativeToRootModuleDirectory) {
Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader));
@@ -1182,9 +1185,9 @@ void ModuleMap::setUmbrellaHeader(
Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
}
-void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
- const Twine &NameAsWritten,
- const Twine &PathRelativeToRootModuleDirectory) {
+void ModuleMap::setUmbrellaDirAsWritten(
+ Module *Mod, const DirectoryEntry *UmbrellaDir, const Twine &NameAsWritten,
+ const Twine &PathRelativeToRootModuleDirectory) {
Mod->Umbrella = UmbrellaDir;
Mod->UmbrellaAsWritten = NameAsWritten.str();
Mod->UmbrellaRelativeToRootModuleDirectory =
@@ -2563,7 +2566,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
}
// Record this umbrella directory.
- Map.setUmbrellaDir(ActiveModule, Dir, DirNameAsWritten, DirName);
+ Map.setUmbrellaDirAsWritten(ActiveModule, Dir, DirNameAsWritten, DirName);
}
/// Parse a module export declaration.
@@ -2827,7 +2830,7 @@ void ModuleMapParser::parseInferredModuleDecl(bool Framework, bool Explicit) {
if (ActiveModule) {
// Inferred modules must have umbrella directories.
if (!Failed && ActiveModule->IsAvailable &&
- !ActiveModule->getUmbrellaDir()) {
+ !ActiveModule->getEffectiveUmbrellaDir()) {
Diags.Report(StarLoc, diag::err_mmap_inferred_no_umbrella);
Failed = true;
}
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index e82e47317127b..3c4a1386dfc8e 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -282,14 +282,14 @@ const char *Preprocessor::getCurLexerEndPos() {
static void collectAllSubModulesWithUmbrellaHeader(
const Module &Mod, SmallVectorImpl<const Module *> &SubMods) {
- if (Mod.getUmbrellaHeader())
+ if (Mod.getUmbrellaHeaderAsWritten())
SubMods.push_back(&Mod);
for (auto *M : Mod.submodules())
collectAllSubModulesWithUmbrellaHeader(*M, SubMods);
}
void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
- const Module::Header &UmbrellaHeader = Mod.getUmbrellaHeader();
+ Module::Header UmbrellaHeader = Mod.getUmbrellaHeaderAsWritten();
assert(UmbrellaHeader.Entry && "Module must use umbrella header");
const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
SourceLocation ExpectedHeadersLoc = SourceMgr.getLocForEndOfFile(File);
@@ -298,7 +298,7 @@ void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
return;
ModuleMap &ModMap = getHeaderSearchInfo().getModuleMap();
- const DirectoryEntry *Dir = Mod.getUmbrellaDir().Entry;
+ const DirectoryEntry *Dir = Mod.getEffectiveUmbrellaDir();
llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
std::error_code EC;
for (llvm::vfs::recursive_directory_iterator Entry(FS, Dir->getName(), EC),
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 69c410d94c5a5..5b53aa3c496bb 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5713,9 +5713,9 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
std::string Filename = std::string(Blob);
ResolveImportedPath(F, Filename);
if (auto Umbrella = PP.getFileManager().getOptionalFileRef(Filename)) {
- if (!CurrentModule->getUmbrellaHeader()) {
+ if (!CurrentModule->getUmbrellaHeaderAsWritten()) {
// FIXME: NameAsWritten
- ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob, "");
+ ModMap.setUmbrellaHeaderAsWritten(CurrentModule, *Umbrella, Blob, "");
}
// Note that it's too late at this point to return out of date if the
// name from the PCM doesn't match up with the one in the module map,
@@ -5751,9 +5751,9 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
std::string Dirname = std::string(Blob);
ResolveImportedPath(F, Dirname);
if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
- if (!CurrentModule->getUmbrellaDir()) {
+ if (!CurrentModule->getUmbrellaDirAsWritten()) {
// FIXME: NameAsWritten
- ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob, "");
+ ModMap.setUmbrellaDirAsWritten(CurrentModule, *Umbrella, Blob, "");
}
}
break;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4efc27b3d2434..96b087ed57933 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2846,11 +2846,12 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
}
// Emit the umbrella header, if there is one.
- if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
+ if (Module::Header UmbrellaHeader = Mod->getUmbrellaHeaderAsWritten()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
UmbrellaHeader.NameAsWritten);
- } else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
+ } else if (Module::DirectoryName UmbrellaDir =
+ Mod->getUmbrellaDirAsWritten()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
UmbrellaDir.NameAsWritten);
More information about the cfe-commits
mailing list