oontvoo updated this revision to Diff 253162.
oontvoo added a comment.
Add tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75951/new/
https://reviews.llvm.org/D75951
Files:
clang/include/clang/Lex/HeaderSearch.h
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/Inputs/dummy_pragma_once.h
clang/test/Modules/Inputs/module.map
clang/test/Modules/import-pragma-once.c
Index: clang/test/Modules/import-pragma-once.c
===================================================================
--- /dev/null
+++ clang/test/Modules/import-pragma-once.c
@@ -0,0 +1,5 @@
+
+#include "dummy_pragma_once.h"
+#include "dummy_pragma_once.h" // Re-importing should be fine
+
+void *p = &x;
Index: clang/test/Modules/Inputs/module.map
===================================================================
--- clang/test/Modules/Inputs/module.map
+++ clang/test/Modules/Inputs/module.map
@@ -282,6 +282,10 @@
header "dummy.h"
}
+module dummy_pragma_once {
+ header "dummy_pragma_once.h"
+}
+
module builtin {
header "builtin.h"
explicit module sub {
Index: clang/test/Modules/Inputs/dummy_pragma_once.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_pragma_once.h
@@ -0,0 +1,3 @@
+//#pragma once | dont need this
+
+int x = 5;
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1619,7 +1619,7 @@
endian::Writer LE(Out, little);
unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
LE.write<uint16_t>(KeyLen);
- unsigned DataLen = 1 + 2 + 4 + 4;
+ unsigned DataLen = 1 + 2 + 4 + 4 + 4;
for (auto ModInfo : Data.KnownHeaders)
if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
DataLen += 4;
@@ -1676,6 +1676,9 @@
}
LE.write<uint32_t>(Offset);
+ // Write this file UID.
+ LE.write<uint32_t>(Data.HFI.UID);
+
auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1703,7 +1706,7 @@
/// Write the header search block for the list of files that
///
/// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch &HS) {
HeaderFileInfoTrait GeneratorTrait(*this);
llvm::OnDiskChainedHashTableGenerator<HeaderFileInfoTrait> Generator;
SmallVector<const char *, 4> SavedStrings;
@@ -1781,8 +1784,7 @@
// 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, /*WantExternal*/!Chain);
+ HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
continue;
@@ -1799,8 +1801,13 @@
HeaderFileInfoTrait::key_type Key = {
Filename, File->getSize(), getTimestampForOutput(File)
};
+ // Set the UID for this HFI so that its importers could use it
+ // when serializing.
+ HFI->UID = UID;
HeaderFileInfoTrait::data_type Data = {
- *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+ *HFI,
+ HS.getModuleMap().findAllModulesForHeader(File),
+ {},
};
Generator.insert(Key, Data, GeneratorTrait);
++NumHeaderSearchEntries;
@@ -2634,6 +2641,25 @@
Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
}
+ // Emit the imported header's UIDs.
+ {
+ auto it = PP->Submodules.find(Mod);
+ if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+ RecordData Record;
+ for (unsigned UID : it->second.IncludedFiles) {
+ // Only save it if the header is actually import/pragma once.
+ // FIXME When we first see a header, it always goes into the mod's
+ // list of included, regardless of whether it was pragma-once or not.
+ // Maybe better to fix that earlier?
+ auto HFI = PP->getHeaderSearchInfo().FileInfo[UID];
+ if (HFI.isImport || HFI.isPragmaOnce) {
+ Record.push_back(HFI.UID);
+ }
+ }
+ Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+ }
+ }
+
// Emit the exports.
if (!Mod->Exports.empty()) {
RecordData Record;
@@ -4715,6 +4741,24 @@
if (WritingModule)
WriteSubmodules(WritingModule);
+ // Write the imported headers, only for the precompiled header (ie, no
+ // modules) because the modules will have emitted their own imported headers.
+ if ((!WritingModule || PP.Submodules.empty()) &&
+ !PP.CurSubmoduleState->IncludedFiles.empty()) {
+ RecordData Record;
+ for (unsigned UID : PP.CurSubmoduleState->IncludedFiles) {
+ // Only save it if the header is actually import/pragma once.
+ // FIXME When we first see a header, it always goes into the
+ // list of included, regardless of whether it was pragma-once or not.
+ // Maybe better to fix that earlier?
+ auto HFI = PP.getHeaderSearchInfo().FileInfo[UID];
+ if (HFI.isImport || HFI.isPragmaOnce) {
+ Record.push_back(HFI.UID);
+ }
+ }
+ Stream.EmitRecord(PP_IMPORTED_HEADERS, Record);
+ }
+
// We need to have information about submodules to correctly deserialize
// decls from OpenCLExtensionDecls block
WriteOpenCLExtensionDecls(SemaRef);
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1,3 +1,4 @@
+
//===- ASTReader.cpp - AST File Reader ------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -1898,6 +1899,9 @@
HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
}
+ // Read the file old UID
+ HFI.UID = endian::readNext<uint32_t, little, unaligned>(d);
+
assert((End - d) % 4 == 0 &&
"Wrong data length in HeaderFileInfo deserialization");
while (d != End) {
@@ -3081,6 +3085,7 @@
case HEADER_SEARCH_TABLE:
case IMPORTED_MODULES:
case MACRO_OFFSET:
+ case PP_IMPORTED_HEADERS:
break;
default:
continue;
@@ -3530,6 +3535,12 @@
break;
}
+ case PP_IMPORTED_HEADERS: {
+ for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+ PendingImportedHeaders.push_back(Record[Idx]);
+ }
+ break;
+ }
case DECL_UPDATE_OFFSETS:
if (Record.size() % 2 != 0) {
Error("invalid DECL_UPDATE_OFFSETS block in AST file");
@@ -5615,6 +5626,11 @@
}
break;
+ case SUBMODULE_IMPORTED_HEADERS:
+ for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+ PendingImportedHeadersForModules[&F].push_back(Record[Idx]);
+ }
+ break;
case SUBMODULE_EXPORTS:
for (unsigned Idx = 0; Idx + 1 < Record.size(); Idx += 2) {
UnresolvedModuleRef Unresolved;
@@ -9271,6 +9287,62 @@
for (auto *ND : PendingMergedDefinitionsToDeduplicate)
getContext().deduplicateMergedDefinitonsFor(ND);
PendingMergedDefinitionsToDeduplicate.clear();
+
+ // Have to be done last because we need the modules
+ // and other PP info.
+ resolvePendingImportedHeaders();
+}
+
+void ASTReader::resolvePendingImportedHeaders() {
+ if (!PendingImportedHeadersForModules.empty() ||
+ !PendingImportedHeaders.empty()) {
+ // They cannot be both non-empty!
+ assert(PendingImportedHeadersForModules.empty() !=
+ PendingImportedHeaders.empty());
+
+ if (PP.getHeaderSearchInfo().FileInfo.empty()) {
+ return;
+ }
+ std::map<unsigned, unsigned> UIDToIndex;
+
+ // These HFIs were deserialized and assigned their "old"
+ // UID.
+ // We need to update them and populate the OldToIndex map
+ // for use next.
+ HeaderSearch &HS = PP.getHeaderSearchInfo();
+ for (unsigned Idx = 0; Idx < HS.FileInfo.size(); ++Idx) {
+ UIDToIndex[HS.FileInfo[Idx].UID] = Idx;
+ }
+
+ const auto &Iter = PendingImportedHeadersForModules.begin();
+ for (unsigned I = 0; I < PendingImportedHeadersForModules.size(); ++I) {
+ ModuleFile *ModFile = Iter[I].first;
+ auto &HeaderUIDs = Iter[I].second;
+ Module *M = HS.lookupModule(ModFile->ModuleName);
+
+ Preprocessor::SubmoduleState &SubState = PP.Submodules[M];
+ for (unsigned OldUID : HeaderUIDs) {
+ auto IdxIt = UIDToIndex.find(OldUID);
+ if (IdxIt == UIDToIndex.end())
+ continue;
+
+ SubState.IncludedFiles.insert(IdxIt->second);
+ }
+ }
+ PendingImportedHeadersForModules.clear();
+
+ if (!PendingImportedHeaders.empty()) {
+ llvm::errs() << " *** (PCH) found pending headers\n";
+ for (unsigned OldUID : PendingImportedHeaders) {
+ auto IdxIt = UIDToIndex.find(OldUID);
+ if (IdxIt == UIDToIndex.end())
+ continue;
+
+ PP.CurSubmoduleState->IncludedFiles.insert(IdxIt->second);
+ }
+ PendingImportedHeaders.clear();
+ }
+ }
}
void ASTReader::diagnoseOdrViolations() {
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1165,7 +1165,8 @@
FileInfo.resize(FE->getUID() + 1);
HeaderFileInfo *HFI = &FileInfo[FE->getUID()];
- // FIXME: Use a generation count to check whether this is really up to date.
+ HFI->UID = FE->getUID();
+
if (ExternalSource && !HFI->Resolved) {
HFI->Resolved = true;
auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE);
@@ -1182,9 +1183,8 @@
return *HFI;
}
-const HeaderFileInfo *
-HeaderSearch::getExistingFileInfo(const FileEntry *FE,
- bool WantExternal) const {
+HeaderFileInfo *HeaderSearch::getExistingFileInfo(const FileEntry *FE,
+ bool WantExternal) 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.
HeaderFileInfo *HFI;
@@ -1250,63 +1250,42 @@
bool ModulesEnabled, Module *M) {
++NumIncluded; // Count # of attempted #includes.
+ // Make sure everything is up to date.
+ ModMap.resolveHeaderDirectives(File);
+
+ const bool SeenBefore =
+ getExistingFileInfo(File, /*WantExernal=*/true) != nullptr;
+
// Get information about this file.
HeaderFileInfo &FileInfo = getFileInfo(File);
- // FIXME: this is a workaround for the lack of proper modules-aware support
- // for #import / #pragma once
- auto TryEnterImported = [&]() -> bool {
- if (!ModulesEnabled)
- return false;
- // Ensure FileInfo bits are up to date.
- ModMap.resolveHeaderDirectives(File);
- // Modules with builtins are special; multiple modules use builtins as
- // modular headers, example:
- //
- // module stddef { header "stddef.h" export * }
- //
- // After module map parsing, this expands to:
- //
- // module stddef {
- // header "/path_to_builtin_dirs/stddef.h"
- // textual "stddef.h"
- // }
- //
- // It's common that libc++ and system modules will both define such
- // submodules. Make sure cached results for a builtin header won't
- // prevent other builtin modules to potentially enter the builtin header.
- // Note that builtins are header guarded and the decision to actually
- // enter them is postponed to the controlling macros logic below.
- bool TryEnterHdr = false;
- if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
- TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
- ModuleMap::isBuiltinHeader(
- llvm::sys::path::filename(File->getName()));
-
- // Textual headers can be #imported from different modules. Since ObjC
- // headers find in the wild might rely only on #import and do not contain
- // controlling macros, be conservative and only try to enter textual headers
- // if such macro is present.
- if (!FileInfo.isModuleHeader &&
- FileInfo.getControllingMacro(ExternalLookup))
- TryEnterHdr = true;
- return TryEnterHdr;
- };
-
- // If this is a #import directive, check that we have not already imported
- // this header.
if (isImport) {
// If this has already been imported, don't import it again.
FileInfo.isImport = true;
+ }
- // Has this already been #import'ed or #include'd?
- if (FileInfo.NumIncludes && !TryEnterImported())
+ if (!SeenBefore || FileInfo.NumIncludes == 0 || FileInfo.isPragmaOnce ||
+ FileInfo.isImport) {
+ if (PP.isIncludeVisible(FileInfo.UID)) {
return false;
- } 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())
+ } else if (FileInfo.isModuleHeader && FileInfo.isCompilingModuleHeader &&
+ PP.isIncludeVisibleFromOuterModule(FileInfo.UID)) {
return false;
+ } else {
+ // Mark as 'included'.
+ PP.setIncludeVisible(FileInfo.UID);
+
+ // FIXME: This is kind of hack to handle textual-header in ObjC
+ // Textual headers can be #imported from different modules.
+ // ObjC headers rely only on #import and do not contain
+ // controlling macros. Hence we won't enter the header if
+ // macro is absent.
+ if (isImport && FileInfo.NumIncludes &&
+ (FileInfo.isModuleHeader ||
+ !FileInfo.getControllingMacro(ExternalLookup))) {
+ return false;
+ }
+ }
}
// Next, check to see if the file is wrapped with #ifndef guards. If so, and
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -463,7 +463,7 @@
void WriteSourceManagerBlock(SourceManager &SourceMgr,
const Preprocessor &PP);
void WritePreprocessor(const Preprocessor &PP, bool IsModule);
- void WriteHeaderSearch(const HeaderSearch &HS);
+ void WriteHeaderSearch(HeaderSearch &HS);
void WritePreprocessorDetail(PreprocessingRecord &PPRec);
void WriteSubmodules(Module *WritingModule);
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -736,6 +736,15 @@
/// IDs have not yet been deserialized to the global IDs of those macros.
PendingMacroIDsMap PendingMacroIDs;
+ /// Mapping from ModuleFile to a list of its imported headers' (old) UID.
+ /// These UIDs are yet to be mapped to their corresponding HeaderFileInfo
+ using PendingImportedHeadersMap =
+ llvm::MapVector<ModuleFile *, SmallVector<unsigned, 8>>;
+ PendingImportedHeadersMap PendingImportedHeadersForModules;
+
+ /// List of a PCH imported headers' (old)UID.
+ SmallVector<unsigned, 8> PendingImportedHeaders;
+
using GlobalPreprocessedEntityMapType =
ContinuousRangeMap<unsigned, ModuleFile *, 4>;
@@ -1410,6 +1419,8 @@
void PassInterestingDeclsToConsumer();
void PassInterestingDeclToConsumer(Decl *D);
+ void resolvePendingImportedHeaders();
+
void finishPendingActions();
void diagnoseOdrViolations();
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -650,7 +650,10 @@
PP_CONDITIONAL_STACK = 62,
/// A table of skipped ranges within the preprocessing record.
- PPD_SKIPPED_RANGES = 63
+ PPD_SKIPPED_RANGES = 63,
+
+ /// List of imported headers' UID for a PCH
+ PP_IMPORTED_HEADERS = 64,
};
/// Record types used within a source manager block.
@@ -781,6 +784,9 @@
/// Specifies the name of the module that will eventually
/// re-export the entities in this module.
SUBMODULE_EXPORT_AS = 17,
+
+ // Specifies the headers' UID imported by this submodule.
+ SUBMODULE_IMPORTED_HEADERS = 18,
};
/// Record types used within a comments block.
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -51,6 +51,7 @@
#include <cstdint>
#include <map>
#include <memory>
+#include <set>
#include <string>
#include <utility>
#include <vector>
@@ -128,6 +129,8 @@
class Preprocessor {
friend class VAOptDefinitionContext;
friend class VariadicMacroScopeGuard;
+ friend class ASTWriter;
+ friend class ASTReader;
llvm::unique_function<void(const clang::Token &)> OnToken;
std::shared_ptr<PreprocessorOptions> PPOpts;
@@ -735,6 +738,7 @@
};
SmallVector<BuildingSubmoduleInfo, 8> BuildingSubmoduleStack;
+ // struct HeaderFileInfo;
/// Information about a submodule's preprocessor state.
struct SubmoduleState {
/// The macros for the submodule.
@@ -743,6 +747,9 @@
/// The set of modules that are visible within the submodule.
VisibleModuleSet VisibleModules;
+ /// The set of the included headers for the submodule.
+ std::set<unsigned> IncludedFiles;
+
// FIXME: CounterValue?
// FIXME: PragmaPushMacroInfo?
};
@@ -1038,6 +1045,24 @@
OnToken = std::move(F);
}
+ void setIncludeVisible(unsigned UID) {
+ CurSubmoduleState->IncludedFiles.insert(UID);
+ }
+
+ bool isIncludeVisibleFromOuterModule(unsigned UID) {
+ if (BuildingSubmoduleStack.empty())
+ return false;
+
+ auto OuterState = BuildingSubmoduleStack.back().OuterSubmoduleState;
+ return OuterState->IncludedFiles.find(UID) !=
+ OuterState->IncludedFiles.end();
+ }
+
+ bool isIncludeVisible(unsigned UID) {
+ return CurSubmoduleState->IncludedFiles.find(UID) !=
+ CurSubmoduleState->IncludedFiles.end();
+ }
+
bool isMacroDefined(StringRef Id) {
return isMacroDefined(&Identifiers.get(Id));
}
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -110,10 +110,13 @@
/// of the framework.
StringRef Framework;
+ /// The file UID used during [de]serialization.
+ unsigned UID = 0;
+
HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
External(false), isModuleHeader(false), isCompilingModuleHeader(false),
- Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
+ Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
@@ -157,6 +160,8 @@
/// by a \#include or \#include_next, (sub-)framework lookup, etc.
class HeaderSearch {
friend class DirectoryLookup;
+ friend class ASTReader;
+ friend class ASTWriter;
/// Header-search options used to initialize this header search.
std::shared_ptr<HeaderSearchOptions> HSOpts;
@@ -664,8 +669,8 @@
/// if it has ever been filled in.
/// \param WantExternal Whether the caller wants purely-external header file
/// info (where \p External is true).
- const HeaderFileInfo *getExistingFileInfo(const FileEntry *FE,
- bool WantExternal = true) const;
+ HeaderFileInfo *getExistingFileInfo(const FileEntry *FE,
+ bool WantExternal = true) const;
// Used by external tools
using search_dir_iterator = std::vector<DirectoryLookup>::const_iterator;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
