Skip to content

[clang][modules] Shrink the size of Module::Headers #113395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions clang-tools-extra/modularize/CoverageChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,9 @@ bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
return false;
}

for (auto &HeaderKind : Mod.Headers)
for (auto &Header : HeaderKind)
ModuleMapHeadersSet.insert(
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));
for (const auto &Header : Mod.getAllHeaders())
ModuleMapHeadersSet.insert(
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));

for (auto *Submodule : Mod.submodules())
collectModuleHeaders(*Submodule);
Expand Down
14 changes: 3 additions & 11 deletions clang-tools-extra/modularize/ModularizeUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
} else if (std::optional<clang::Module::DirectoryName> UmbrellaDir =
Mod.getUmbrellaDirAsWritten()) {
// If there normal headers, assume these are umbrellas and skip collection.
if (Mod.Headers->size() == 0) {
if (Mod.getHeaders(Module::HK_Normal).empty()) {
// Collect headers in umbrella directory.
if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName(),
UmbrellaDependents))
Expand All @@ -371,16 +371,8 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
// modules or because they are meant to be included by another header,
// and thus should be ignored by modularize.

int NormalHeaderCount = Mod.Headers[clang::Module::HK_Normal].size();

for (int Index = 0; Index < NormalHeaderCount; ++Index) {
DependentsVector NormalDependents;
// Collect normal header.
const clang::Module::Header &Header(
Mod.Headers[clang::Module::HK_Normal][Index]);
std::string HeaderPath = getCanonicalPath(Header.Entry.getName());
HeaderFileNames.push_back(HeaderPath);
}
for (const auto &Header : Mod.getHeaders(clang::Module::HK_Normal))
HeaderFileNames.push_back(getCanonicalPath(Header.Entry.getName()));

int MissingCountThisModule = Mod.MissingHeaders.size();

Expand Down
31 changes: 24 additions & 7 deletions clang/include/clang/Basic/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,6 @@ class alignas(8) Module {
HK_PrivateTextual,
HK_Excluded
};
static const int NumHeaderKinds = HK_Excluded + 1;

/// Information about a header directive as found in the module map
/// file.
struct Header {
Expand All @@ -263,17 +261,36 @@ class alignas(8) Module {
FileEntryRef Entry;
};

/// Information about a directory name as found in the module map
/// file.
private:
static const int NumHeaderKinds = HK_Excluded + 1;
// The begin index for a HeaderKind also acts the end index of HeaderKind - 1.
// The extra element at the end acts as the end index of the last HeaderKind.
unsigned HeaderKindBeginIndex[NumHeaderKinds + 1] = {};
SmallVector<Header, 2> HeadersStorage;

public:
ArrayRef<Header> getAllHeaders() const { return HeadersStorage; }
ArrayRef<Header> getHeaders(HeaderKind HK) const {
assert(HK < NumHeaderKinds && "Invalid Module::HeaderKind");
auto BeginIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK];
auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1];
return {BeginIt, EndIt};
}
void addHeader(HeaderKind HK, Header H) {
assert(HK < NumHeaderKinds && "Invalid Module::HeaderKind");
auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1];
HeadersStorage.insert(EndIt, std::move(H));
for (unsigned HKI = HK + 1; HKI != NumHeaderKinds + 1; ++HKI)
++HeaderKindBeginIndex[HKI];
}

/// Information about a directory name as found in the module map file.
struct DirectoryName {
std::string NameAsWritten;
std::string PathRelativeToRootModuleDirectory;
DirectoryEntryRef Entry;
};

/// The headers that are part of this module.
SmallVector<Header, 2> Headers[5];

/// Stored information about a header directive that was found in the
/// module map file but has not been resolved to a file.
struct UnresolvedHeaderDirective {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Basic/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {

for (auto &K : Kinds) {
assert(&K == &Kinds[K.Kind] && "kinds in wrong order");
for (auto &H : Headers[K.Kind]) {
for (auto &H : getHeaders(K.Kind)) {
OS.indent(Indent + 2);
OS << K.Prefix << "header \"";
OS.write_escaped(H.NameAsWritten);
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/FrontendAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ static std::error_code collectModuleHeaderIncludes(

// Add includes for each of these headers.
for (auto HK : {Module::HK_Normal, Module::HK_Private}) {
for (Module::Header &H : Module->Headers[HK]) {
for (const Module::Header &H : Module->getHeaders(HK)) {
Module->addTopHeader(H.Entry);
// Use the path as specified in the module map file. We'll look for this
// file relative to the module build directory (the directory containing
Expand Down
21 changes: 11 additions & 10 deletions clang/lib/Lex/ModuleMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,12 @@ static bool violatesPrivateInclude(Module *RequestingModule,
// as obtained from the lookup and as obtained from the module.
// This check is not cheap, so enable it only for debugging.
bool IsPrivate = false;
SmallVectorImpl<Module::Header> *HeaderList[] = {
&Header.getModule()->Headers[Module::HK_Private],
&Header.getModule()->Headers[Module::HK_PrivateTextual]};
for (auto *Hs : HeaderList)
ArrayRef<Module::Header> HeaderList[] = {
Header.getModule()->getHeaders(Module::HK_Private),
Header.getModule()->getHeaders(Module::HK_PrivateTextual)};
for (auto Hs : HeaderList)
IsPrivate |= llvm::any_of(
*Hs, [&](const Module::Header &H) { return H.Entry == IncFileEnt; });
Hs, [&](const Module::Header &H) { return H.Entry == IncFileEnt; });
assert(IsPrivate && "inconsistent headers and roles");
}
#endif
Expand Down Expand Up @@ -1296,27 +1296,28 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header,
ModuleHeaderRole Role, bool Imported) {
KnownHeader KH(Mod, Role);

FileEntryRef HeaderEntry = Header.Entry;

// Only add each header to the headers list once.
// FIXME: Should we diagnose if a header is listed twice in the
// same module definition?
auto &HeaderList = Headers[Header.Entry];
auto &HeaderList = Headers[HeaderEntry];
if (llvm::is_contained(HeaderList, KH))
return;

HeaderList.push_back(KH);
Mod->Headers[headerRoleToKind(Role)].push_back(Header);
Mod->addHeader(headerRoleToKind(Role), std::move(Header));

bool isCompilingModuleHeader = Mod->isForBuilding(LangOpts);
if (!Imported || isCompilingModuleHeader) {
// When we import HeaderFileInfo, the external source is expected to
// set the isModuleHeader flag itself.
HeaderInfo.MarkFileModuleHeader(Header.Entry, Role,
isCompilingModuleHeader);
HeaderInfo.MarkFileModuleHeader(HeaderEntry, Role, isCompilingModuleHeader);
}

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

FileID ModuleMap::getContainingModuleMapFileID(const Module *Module) const {
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3070,9 +3070,9 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
Module::HK_PrivateTextual},
{SUBMODULE_EXCLUDED_HEADER, ExcludedHeaderAbbrev, Module::HK_Excluded}
};
for (auto &HL : HeaderLists) {
for (const auto &HL : HeaderLists) {
RecordData::value_type Record[] = {HL.RecordKind};
for (auto &H : Mod->Headers[HL.HeaderKind])
for (const auto &H : Mod->getHeaders(HL.HeaderKind))
Stream.EmitRecordWithBlob(HL.Abbrev, Record, H.NameAsWritten);
}

Expand Down
Loading