[modules] Fix HeaderFileInfo serialization to store all the known owning modules for a header, not just the current favourite.
llvm-svn: 245390
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index eea4719..e1c8fa2 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -975,65 +975,72 @@
if (OtherHFI.External) {
HFI.DirInfo = OtherHFI.DirInfo;
- HFI.External = OtherHFI.External;
+ HFI.External = OtherHFI.External && (!HFI.IsValid || HFI.External);
HFI.IndexHeaderMapHeader = OtherHFI.IndexHeaderMapHeader;
}
if (HFI.Framework.empty())
HFI.Framework = OtherHFI.Framework;
-
- HFI.Resolved = true;
}
/// getFileInfo - Return the HeaderFileInfo structure for the specified
/// FileEntry.
HeaderFileInfo &HeaderSearch::getFileInfo(const FileEntry *FE) {
if (FE->getUID() >= FileInfo.size())
- FileInfo.resize(FE->getUID()+1);
-
+ FileInfo.resize(FE->getUID() + 1);
+
HeaderFileInfo &HFI = FileInfo[FE->getUID()];
- if (ExternalSource && !HFI.Resolved)
+ // FIXME: Use a generation count to check whether this is really up to date.
+ if (ExternalSource && !HFI.Resolved) {
+ HFI.Resolved = true;
mergeHeaderFileInfo(HFI, ExternalSource->GetHeaderFileInfo(FE));
- HFI.IsValid = 1;
+ }
+
+ HFI.IsValid = true;
+ // We have local information about this header file, so it's no longer
+ // strictly external.
+ HFI.External = false;
return HFI;
}
-bool HeaderSearch::tryGetFileInfo(const FileEntry *FE,
- HeaderFileInfo &Result) const {
- if (FE->getUID() >= FileInfo.size())
- return false;
- const HeaderFileInfo &HFI = FileInfo[FE->getUID()];
- if (HFI.IsValid) {
- Result = HFI;
- return true;
+const HeaderFileInfo *
+HeaderSearch::getExistingFileInfo(const FileEntry *FE) const {
+ // If we have an external source, ensure we have the latest information.
+ // FIXME: Use a generation count to check whether this is really up to date.
+ if (ExternalSource &&
+ (FE->getUID() >= FileInfo.size() || !FileInfo[FE->getUID()].Resolved)) {
+ auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE);
+ if (ExternalHFI.External) {
+ if (FE->getUID() >= FileInfo.size())
+ FileInfo.resize(FE->getUID() + 1);
+ mergeHeaderFileInfo(FileInfo[FE->getUID()], ExternalHFI);
+ }
}
- return false;
+
+ if (FE->getUID() >= FileInfo.size())
+ return nullptr;
+
+ HeaderFileInfo &HFI = FileInfo[FE->getUID()];
+ if (!HFI.IsValid)
+ return nullptr;
+
+ return &HFI;
}
bool HeaderSearch::isFileMultipleIncludeGuarded(const FileEntry *File) {
// Check if we've ever seen this file as a header.
- if (File->getUID() >= FileInfo.size())
- return false;
-
- // Resolve header file info from the external source, if needed.
- HeaderFileInfo &HFI = FileInfo[File->getUID()];
- if (ExternalSource && !HFI.Resolved)
- mergeHeaderFileInfo(HFI, ExternalSource->GetHeaderFileInfo(File));
-
- return HFI.isPragmaOnce || HFI.isImport ||
- HFI.ControllingMacro || HFI.ControllingMacroID;
+ if (auto *HFI = getExistingFileInfo(File))
+ return HFI->isPragmaOnce || HFI->isImport || HFI->ControllingMacro ||
+ HFI->ControllingMacroID;
+ return false;
}
void HeaderSearch::MarkFileModuleHeader(const FileEntry *FE,
ModuleMap::ModuleHeaderRole Role,
bool isCompilingModuleHeader) {
- if (FE->getUID() >= FileInfo.size())
- FileInfo.resize(FE->getUID()+1);
-
- HeaderFileInfo &HFI = FileInfo[FE->getUID()];
- HFI.isModuleHeader = true;
+ auto &HFI = getFileInfo(FE);
+ HFI.isModuleHeader |= !(Role & ModuleMap::TextualHeader);
HFI.isCompilingModuleHeader |= isCompilingModuleHeader;
- HFI.setHeaderRole(Role);
}
bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
@@ -1143,7 +1150,7 @@
if (ExternalSource) {
// Make sure the external source has handled header info about this file,
// which includes whether the file is part of a module.
- (void)getFileInfo(File);
+ (void)getExistingFileInfo(File);
}
return ModMap.findModuleForHeader(File);
}
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 9554d3b..d619b52 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -359,6 +359,13 @@
return MakeResult(Result);
}
+ return MakeResult(findOrCreateModuleForHeaderInUmbrellaDir(File));
+}
+
+ModuleMap::KnownHeader
+ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
+ assert(!Headers.count(File) && "already have a module for this header");
+
SmallVector<const DirectoryEntry *, 2> SkippedDirs;
KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs);
if (H) {
@@ -419,19 +426,22 @@
UmbrellaDirs[SkippedDirs[I]] = Result;
}
- Headers[File].push_back(KnownHeader(Result, NormalHeader));
-
- // If a header corresponds to an unavailable module, don't report
- // that it maps to anything.
- if (!Result->isAvailable())
- return KnownHeader();
-
- return MakeResult(Headers[File].back());
+ KnownHeader Header(Result, NormalHeader);
+ Headers[File].push_back(Header);
+ return Header;
}
return KnownHeader();
}
+ArrayRef<ModuleMap::KnownHeader>
+ModuleMap::findAllModulesForHeader(const FileEntry *File) const {
+ auto It = Headers.find(File);
+ if (It == Headers.end())
+ return None;
+ return It->second;
+}
+
bool ModuleMap::isHeaderInUnavailableModule(const FileEntry *Header) const {
return isHeaderUnavailableInModule(Header, nullptr);
}
@@ -787,14 +797,21 @@
void ModuleMap::addHeader(Module *Mod, Module::Header Header,
ModuleHeaderRole Role) {
- if (!(Role & TextualHeader)) {
- bool isCompilingModuleHeader = Mod->getTopLevelModule() == CompilingModule;
- HeaderInfo.MarkFileModuleHeader(Header.Entry, Role,
- isCompilingModuleHeader);
- }
- Headers[Header.Entry].push_back(KnownHeader(Mod, Role));
+ KnownHeader KH(Mod, Role);
+ // 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];
+ for (auto H : HeaderList)
+ if (H == KH)
+ return;
+
+ HeaderList.push_back(KH);
Mod->Headers[headerRoleToKind(Role)].push_back(std::move(Header));
+
+ bool isCompilingModuleHeader = Mod->getTopLevelModule() == CompilingModule;
+ HeaderInfo.MarkFileModuleHeader(Header.Entry, Role, isCompilingModuleHeader);
}
void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 6d41d15..fc9d29a 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1556,14 +1556,15 @@
using namespace llvm::support;
HeaderFileInfo HFI;
unsigned Flags = *d++;
- HFI.HeaderRole = static_cast<ModuleMap::ModuleHeaderRole>
- ((Flags >> 6) & 0x03);
- HFI.isImport = (Flags >> 5) & 0x01;
- HFI.isPragmaOnce = (Flags >> 4) & 0x01;
- HFI.DirInfo = (Flags >> 2) & 0x03;
- HFI.Resolved = (Flags >> 1) & 0x01;
+ // FIXME: Refactor with mergeHeaderFileInfo in HeaderSearch.cpp.
+ HFI.isImport |= (Flags >> 4) & 0x01;
+ HFI.isPragmaOnce |= (Flags >> 3) & 0x01;
+ HFI.DirInfo = (Flags >> 1) & 0x03;
HFI.IndexHeaderMapHeader = Flags & 0x01;
- HFI.NumIncludes = endian::readNext<uint16_t, little, unaligned>(d);
+ // 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 =
@@ -1573,32 +1574,32 @@
StringRef FrameworkName(FrameworkStrings + FrameworkOffset - 1);
HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
}
-
- if (d != End) {
+
+ assert((End - d) % 4 == 0 &&
+ "Wrong data length in HeaderFileInfo deserialization");
+ while (d != End) {
uint32_t LocalSMID = endian::readNext<uint32_t, little, unaligned>(d);
- if (LocalSMID) {
- // This header is part of a module. Associate it with the module to enable
- // implicit module import.
- SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
- Module *Mod = Reader.getSubmodule(GlobalSMID);
- HFI.isModuleHeader = true;
- FileManager &FileMgr = Reader.getFileManager();
- ModuleMap &ModMap =
- Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
- // FIXME: This information should be propagated through the
- // SUBMODULE_HEADER etc records rather than from here.
- // FIXME: We don't ever mark excluded headers.
- std::string Filename = key.Filename;
- if (key.Imported)
- Reader.ResolveImportedPath(M, Filename);
- Module::Header H = { key.Filename, FileMgr.getFile(Filename) };
- ModMap.addHeader(Mod, H, HFI.getHeaderRole());
- }
+ auto HeaderRole = static_cast<ModuleMap::ModuleHeaderRole>(LocalSMID & 3);
+ LocalSMID >>= 2;
+
+ // This header is part of a module. Associate it with the module to enable
+ // implicit module import.
+ SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
+ Module *Mod = Reader.getSubmodule(GlobalSMID);
+ FileManager &FileMgr = Reader.getFileManager();
+ ModuleMap &ModMap =
+ Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
+
+ std::string Filename = key.Filename;
+ if (key.Imported)
+ Reader.ResolveImportedPath(M, Filename);
+ // FIXME: This is not always the right filename-as-written, but we're not
+ // going to use this information to rebuild the module, so it doesn't make
+ // a lot of difference.
+ Module::Header H = { key.Filename, FileMgr.getFile(Filename) };
+ ModMap.addHeader(Mod, H, HeaderRole);
}
- assert(End == d && "Wrong data length in HeaderFileInfo deserialization");
- (void)End;
-
// This HeaderFileInfo was externally loaded.
HFI.External = true;
return HFI;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 0aceb08..b554f5f 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1637,13 +1637,14 @@
std::pair<unsigned,unsigned>
EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
using namespace llvm::support;
- endian::Writer<little> Writer(Out);
+ endian::Writer<little> LE(Out);
unsigned KeyLen = strlen(key.Filename) + 1 + 8 + 8;
- Writer.write<uint16_t>(KeyLen);
+ LE.write<uint16_t>(KeyLen);
unsigned DataLen = 1 + 2 + 4 + 4;
- if (Data.isModuleHeader)
- DataLen += 4;
- Writer.write<uint8_t>(DataLen);
+ for (auto ModInfo : HS.getModuleMap().findAllModulesForHeader(key.FE))
+ if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
+ DataLen += 4;
+ LE.write<uint8_t>(DataLen);
return std::make_pair(KeyLen, DataLen);
}
@@ -1663,11 +1664,9 @@
endian::Writer<little> LE(Out);
uint64_t Start = Out.tell(); (void)Start;
- unsigned char Flags = (Data.HeaderRole << 6)
- | (Data.isImport << 5)
- | (Data.isPragmaOnce << 4)
- | (Data.DirInfo << 2)
- | (Data.Resolved << 1)
+ unsigned char Flags = (Data.isImport << 4)
+ | (Data.isPragmaOnce << 3)
+ | (Data.DirInfo << 1)
| Data.IndexHeaderMapHeader;
LE.write<uint8_t>(Flags);
LE.write<uint16_t>(Data.NumIncludes);
@@ -1694,9 +1693,15 @@
}
LE.write<uint32_t>(Offset);
- if (Data.isModuleHeader) {
- Module *Mod = HS.findModuleForHeader(key.FE).getModule();
- LE.write<uint32_t>(Writer.getExistingSubmoduleID(Mod));
+ // FIXME: If the header is excluded, we should write out some
+ // record of that fact.
+ for (auto ModInfo : HS.getModuleMap().findAllModulesForHeader(key.FE)) {
+ if (uint32_t ModID =
+ Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule())) {
+ uint32_t Value = (ModID << 2) | (unsigned)ModInfo.getRole();
+ assert((Value >> 2) == ModID && "overflow in header module info");
+ LE.write<uint32_t>(Value);
+ }
}
assert(Out.tell() - Start == DataLen && "Wrong data length");
@@ -1726,12 +1731,15 @@
if (!File)
continue;
- // Use HeaderSearch's getFileInfo to make sure we get the HeaderFileInfo
- // from the external source if it was not provided already.
- HeaderFileInfo HFI;
- if (!HS.tryGetFileInfo(File, HFI) ||
- (HFI.External && Chain) ||
- (HFI.isModuleHeader && !HFI.isCompilingModuleHeader))
+ // Get the file info. This will load info from the external source if
+ // necessary. Skip emitting this file if we have no information on it
+ // as a header file (in which case HFI will be null) or if it hasn't
+ // changed since it was loaded. Also skip it if it's for a modular header
+ // from a different module; in that case, we rely on the module(s)
+ // containing the header to provide this information.
+ const HeaderFileInfo *HFI = HS.getExistingFileInfo(File);
+ if (!HFI || (HFI->External && Chain) ||
+ (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
continue;
// Massage the file path into an appropriate form.
@@ -1745,7 +1753,7 @@
}
HeaderFileInfoTrait::key_type key = { File, Filename };
- Generator.insert(key, HFI, GeneratorTrait);
+ Generator.insert(key, *HFI, GeneratorTrait);
++NumHeaderSearchEntries;
}
@@ -2283,27 +2291,28 @@
}
}
-unsigned ASTWriter::getSubmoduleID(Module *Mod) {
+unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
if (!Mod)
return 0;
llvm::DenseMap<Module *, unsigned>::iterator Known = SubmoduleIDs.find(Mod);
if (Known != SubmoduleIDs.end())
return Known->second;
-
+
+ if (Mod->getTopLevelModule() != WritingModule)
+ return 0;
+
return SubmoduleIDs[Mod] = NextSubmoduleID++;
}
-unsigned ASTWriter::getExistingSubmoduleID(Module *Mod) const {
- if (!Mod)
- return 0;
-
- llvm::DenseMap<Module *, unsigned>::const_iterator
- Known = SubmoduleIDs.find(Mod);
- if (Known != SubmoduleIDs.end())
- return Known->second;
-
- return 0;
+unsigned ASTWriter::getSubmoduleID(Module *Mod) {
+ // FIXME: This can easily happen, if we have a reference to a submodule that
+ // did not result in us loading a module file for that submodule. For
+ // instance, a cross-top-level-module 'conflict' declaration will hit this.
+ unsigned ID = getLocalOrImportedSubmoduleID(Mod);
+ assert((ID || !Mod) &&
+ "asked for module ID for non-local, non-imported module");
+ return ID;
}
/// \brief Compute the number of modules within the given tree (including the
@@ -2542,9 +2551,6 @@
Stream.ExitBlock();
- // FIXME: This can easily happen, if we have a reference to a submodule that
- // did not result in us loading a module file for that submodule. For
- // instance, a cross-top-level-module 'conflict' declaration will hit this.
assert((NextSubmoduleID - FirstSubmoduleID ==
getNumberOfModules(WritingModule)) &&
"Wrong # of submodules; found a reference to a non-local, "