vsapsai created this revision.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Currently we are using `HeaderSearch` state from building a module and
if an invisible submodule imports a non-modular header, we would avoid
importing the header again, even if it wasn't entered earlier due to the
visibility rules.
Track `isImport` and `NumIncludes` according to the visibility of the
[sub]modules that included a header.
rdar://64773328
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104344
Files:
clang/include/clang/Lex/HeaderSearch.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Headers/Invisible.h
clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Headers/Visible.h
clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Modules/module.modulemap
clang/test/Modules/Inputs/import-nonmodular/Unmodularized.framework/Headers/Unmodularized.h
clang/test/Modules/import-nonmodular.m
Index: clang/test/Modules/import-nonmodular.m
===================================================================
--- /dev/null
+++ clang/test/Modules/import-nonmodular.m
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/import-nonmodular %s -verify
+// expected-no-diagnostics
+
+// Test non-modular headers are not skipped even if they are imported by
+// non-imported submodules of an imported module. Dependency graph is
+//
+// Unmodularized.h
+// ^ ^
+// | |
+// Modularized.Visible | Modularized.Invisible
+// ^ |
+// \ |
+// import-nonmodular.m
+
+#import <Modularized/Visible.h>
+#import <Unmodularized/Unmodularized.h>
+void test(UnmodularizedStruct *s) {}
Index: clang/test/Modules/Inputs/import-nonmodular/Unmodularized.framework/Headers/Unmodularized.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-nonmodular/Unmodularized.framework/Headers/Unmodularized.h
@@ -0,0 +1,2 @@
+typedef struct UnmodularizedStruct {
+} UnmodularizedStruct;
Index: clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Modules/module.modulemap
@@ -0,0 +1,11 @@
+framework module Modularized {
+ module Visible {
+ header "Visible.h"
+ export *
+ }
+
+ module Invisible {
+ header "Invisible.h"
+ export *
+ }
+}
Index: clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Headers/Visible.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Headers/Visible.h
@@ -0,0 +1 @@
+// Empty.
Index: clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Headers/Invisible.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-nonmodular/Modularized.framework/Headers/Invisible.h
@@ -0,0 +1 @@
+#import <Unmodularized/Unmodularized.h>
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1668,7 +1668,10 @@
std::pair<unsigned, unsigned>
EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
- unsigned DataLen = 1 + 2 + 4 + 4;
+ unsigned DataLen = 1 + 4 + 4 + 4;
+ for (auto ModuleIncluder : Data.HFI.ModuleIncludersMap)
+ if (Writer.getLocalOrImportedSubmoduleID(ModuleIncluder.first))
+ DataLen += 4;
for (auto ModInfo : Data.KnownHeaders)
if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
DataLen += 4;
@@ -1695,12 +1698,10 @@
endian::Writer LE(Out, little);
uint64_t Start = Out.tell(); (void)Start;
- unsigned char Flags = (Data.HFI.isImport << 5)
- | (Data.HFI.isPragmaOnce << 4)
- | (Data.HFI.DirInfo << 1)
- | Data.HFI.IndexHeaderMapHeader;
+ unsigned char Flags = (Data.HFI.isPragmaOnce << 4) |
+ (Data.HFI.DirInfo << 1) |
+ Data.HFI.IndexHeaderMapHeader;
LE.write<uint8_t>(Flags);
- LE.write<uint16_t>(Data.HFI.NumIncludes);
if (!Data.HFI.ControllingMacro)
LE.write<uint32_t>(Data.HFI.ControllingMacroID);
@@ -1723,6 +1724,18 @@
}
LE.write<uint32_t>(Offset);
+ for (auto ModuleIncluder : Data.HFI.ModuleIncludersMap) {
+ if (uint32_t ModID =
+ Writer.getLocalOrImportedSubmoduleID(ModuleIncluder.first)) {
+ uint32_t Value = (ModID << 1) | (bool)ModuleIncluder.second;
+ assert((Value >> 1) == ModID && "overflow in header module info");
+ LE.write<uint32_t>(Value);
+ }
+ }
+ // Using a stop value because cannot tell ahead of time how many includers
+ // are local or imported submodules.
+ LE.write<uint32_t>(0);
+
auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -2476,11 +2489,11 @@
}
}
-unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
+unsigned ASTWriter::getLocalOrImportedSubmoduleID(const Module *Mod) {
if (!Mod)
return 0;
- llvm::DenseMap<Module *, unsigned>::iterator Known = SubmoduleIDs.find(Mod);
+ auto Known = SubmoduleIDs.find(Mod);
if (Known != SubmoduleIDs.end())
return Known->second;
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1887,14 +1887,9 @@
HeaderFileInfo HFI;
unsigned Flags = *d++;
// FIXME: Refactor with mergeHeaderFileInfo in HeaderSearch.cpp.
- HFI.isImport |= (Flags >> 5) & 0x01;
HFI.isPragmaOnce |= (Flags >> 4) & 0x01;
HFI.DirInfo = (Flags >> 1) & 0x07;
HFI.IndexHeaderMapHeader = Flags & 0x01;
- // FIXME: Find a better way to handle this. Maybe just store a
- // "has been included" flag?
- HFI.NumIncludes = std::max(endian::readNext<uint16_t, little, unaligned>(d),
- HFI.NumIncludes);
HFI.ControllingMacroID = Reader.getGlobalIdentifierID(
M, endian::readNext<uint32_t, little, unaligned>(d));
if (unsigned FrameworkOffset =
@@ -1905,6 +1900,23 @@
HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
}
+ while (true) {
+ uint32_t LocalSMID = endian::readNext<uint32_t, little, unaligned>(d);
+ if (!LocalSMID)
+ break;
+ bool IsImport = static_cast<bool>(LocalSMID & 1);
+ LocalSMID >>= 1;
+
+ SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
+ Module *Mod = Reader.getSubmodule(GlobalSMID);
+ HFI.ModuleIncludersMap[Mod] = IsImport;
+ // Update isImport and NumIncludes based on including module.
+ if (Mod->NameVisibility == Module::NameVisibilityKind::AllVisible) {
+ HFI.isImport |= IsImport;
+ ++HFI.NumIncludes;
+ }
+ }
+
assert((End - d) % 4 == 0 &&
"Wrong data length in HeaderFileInfo deserialization");
while (d != End) {
@@ -1930,6 +1942,11 @@
HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
}
+ serialization::ModuleKind PrimaryModuleKind = Reader.getModuleManager().getPrimaryModule().Kind;
+ if ((PrimaryModuleKind == MK_PCH) || (PrimaryModuleKind == MK_Preamble)) {
+ ++HFI.NumIncludes;
+ }
+
// This HeaderFileInfo was externally loaded.
HFI.External = true;
HFI.IsValid = true;
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2144,6 +2144,15 @@
Action = (SuggestedModule && !getLangOpts().CompilingPCH) ? Import : Skip;
}
+ if (File) {
+ // Store extra data for inclusion from a modular header.
+ Module *RequestingModule = getModuleForLocation(HashLoc);
+ if (RequestingModule) {
+ HeaderInfo.MarkFileIncludedFromModule(&File->getFileEntry(),
+ RequestingModule, EnterOnce);
+ }
+ }
+
// Check for circular inclusion of the main file.
// We can't generate a consistent preamble with regard to the conditional
// stack if the main file is included again as due to the preamble bounds
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -91,7 +91,7 @@
<< FileInfo.size() << " files tracked.\n";
unsigned NumOnceOnlyFiles = 0, MaxNumIncludes = 0, NumSingleIncludedFiles = 0;
for (unsigned i = 0, e = FileInfo.size(); i != e; ++i) {
- NumOnceOnlyFiles += FileInfo[i].isImport;
+ NumOnceOnlyFiles += (FileInfo[i].isPragmaOnce || FileInfo[i].isImport);
if (MaxNumIncludes < FileInfo[i].NumIncludes)
MaxNumIncludes = FileInfo[i].NumIncludes;
NumSingleIncludedFiles += FileInfo[i].NumIncludes == 1;
@@ -1325,7 +1325,7 @@
} else {
// Otherwise, if this is a #include of a file that was previously #import'd
// or if this is the second #include of a #pragma once file, ignore it.
- if (FileInfo.isImport && !TryEnterImported())
+ if ((FileInfo.isPragmaOnce || FileInfo.isImport) && !TryEnterImported())
return false;
}
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -450,7 +450,7 @@
/// A mapping from each known submodule to its ID number, which will
/// be a positive integer.
- llvm::DenseMap<Module *, unsigned> SubmoduleIDs;
+ llvm::DenseMap<const Module *, unsigned> SubmoduleIDs;
/// A list of the module file extension writers.
std::vector<std::unique_ptr<ModuleFileExtensionWriter>>
@@ -671,7 +671,7 @@
/// Retrieve or create a submodule ID for this module, or return 0 if
/// the submodule is neither local (a submodle of the currently-written module)
/// nor from an imported module.
- unsigned getLocalOrImportedSubmoduleID(Module *Mod);
+ unsigned getLocalOrImportedSubmoduleID(const Module *Mod);
/// Note that the identifier II occurs at the given offset
/// within the identifier table.
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -41,7 +41,7 @@
/// Version 4 of AST files also requires that the version control branch and
/// revision match exactly, since there is no backward compatibility of
/// AST files at this time.
-const unsigned VERSION_MAJOR = 13;
+const unsigned VERSION_MAJOR = 14;
/// AST file minor version number supported by this version of
/// Clang.
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -48,7 +48,7 @@
/// The preprocessor keeps track of this information for each
/// file that is \#included.
struct HeaderFileInfo {
- /// True if this is a \#import'd or \#pragma once file.
+ /// True if this is a \#import'd file.
unsigned isImport : 1;
/// True if this is a \#pragma once file.
@@ -110,6 +110,10 @@
/// of the framework.
StringRef Framework;
+ /// Stores modules including this header.
+ /// Bool represents if the header was included or imported.
+ llvm::DenseMap<const Module *, bool> ModuleIncludersMap;
+
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
External(false), isModuleHeader(false), isCompilingModuleHeader(false),
@@ -439,11 +443,10 @@
return (SrcMgr::CharacteristicKind)getFileInfo(File).DirInfo;
}
- /// Mark the specified file as a "once only" file, e.g. due to
+ /// Mark the specified file as a "once only" file due to
/// \#pragma once.
void MarkFileIncludeOnce(const FileEntry *File) {
HeaderFileInfo &FI = getFileInfo(File);
- FI.isImport = true;
FI.isPragmaOnce = true;
}
@@ -458,6 +461,11 @@
ModuleMap::ModuleHeaderRole Role,
bool isCompilingModuleHeader);
+ void MarkFileIncludedFromModule(const FileEntry *File, const Module *M,
+ bool isImport) {
+ getFileInfo(File).ModuleIncludersMap[M] |= isImport;
+ }
+
/// Increment the count for the number of times the specified
/// FileEntry has been entered.
void IncrementIncludeCount(const FileEntry *File) {
@@ -485,8 +493,7 @@
/// This routine does not consider the effect of \#import
bool isFileMultipleIncludeGuarded(const FileEntry *File);
- /// Determine whether the given file is known to have ever been \#imported
- /// (or if it has been \#included and we've encountered a \#pragma once).
+ /// Determine whether the given file is known to have ever been \#imported.
bool hasFileBeenImported(const FileEntry *File) {
const HeaderFileInfo *FI = getExistingFileInfo(File);
return FI && FI->isImport;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
