Skip to content

Commit

Permalink
[clang][modules] NFCI: Distinguish as-written and effective umbrella …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@jansvoboda11
jansvoboda11 committed May 26, 2023
1 parent 7adff65 commit 9249129
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 54 deletions.
7 changes: 4 additions & 3 deletions clang-tools-extra/modularize/CoverageChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions clang-tools-extra/modularize/ModularizeUtilities.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 15 additions & 7 deletions clang/include/clang/Basic/Module.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 10 additions & 11 deletions clang/include/clang/Lex/ModuleMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 8 additions & 8 deletions clang/lib/Basic/Module.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Frontend/FrontendAction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
23 changes: 13 additions & 10 deletions clang/lib/Lex/ModuleMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand All @@ -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 =
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Lex/PPLexerChange.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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),
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/Serialization/ASTReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Serialization/ASTWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 9249129

Please sign in to comment.