diff --git a/clang/docs/Modules.rst b/clang/docs/Modules.rst index 439cb2445e79..df471a45fb88 100644 --- a/clang/docs/Modules.rst +++ b/clang/docs/Modules.rst @@ -445,9 +445,8 @@ A header declaration specifies that a particular header is associated with the e .. parsed-literal:: *header-declaration*: - ``umbrella``:sub:`opt` ``header`` *string-literal* - ``private`` ``header`` *string-literal* - ``textual`` ``header`` *string-literal* + ``private``:sub:`opt` ``textual``:sub:`opt` ``header`` *string-literal* + ``umbrella`` ``header`` *string-literal* ``exclude`` ``header`` *string-literal* A header declaration that does not contain ``exclude`` nor ``textual`` specifies a header that contributes to the enclosing module. Specifically, when the module is built, the named header will be parsed and its declarations will be (logically) placed into the enclosing submodule. @@ -464,14 +463,14 @@ A header with the ``private`` specifier may not be included from outside the mod A header with the ``textual`` specifier will not be included when the module is built, and will be textually included if it is named by a ``#include`` directive. However, it is considered to be part of the module for the purpose of checking *use-declaration*\s. -A header with the ``exclude`` specifier is excluded from the module. It will not be included when the module is built, nor will it be considered to be part of the module. +A header with the ``exclude`` specifier is excluded from the module. It will not be included when the module is built, nor will it be considered to be part of the module, even if an ``umbrella`` header or directory would otherwise make it part of the module. -**Example**: The C header ``assert.h`` is an excellent candidate for an excluded header, because it is meant to be included multiple times (possibly with different ``NDEBUG`` settings). +**Example**: The C header ``assert.h`` is an excellent candidate for a textual header, because it is meant to be included multiple times (possibly with different ``NDEBUG`` settings). However, declarations within it should typically be split into a separate modular header. .. parsed-literal:: module std [system] { - exclude header "assert.h" + textual header "assert.h" } A given header shall not be referenced by more than one *header-declaration*. diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 419e0082eda9..e46be9edca5e 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -84,9 +84,6 @@ public: /// \brief The headers that are part of this module. SmallVector NormalHeaders; - /// \brief The headers that are explicitly excluded from this module. - SmallVector ExcludedHeaders; - /// \brief The headers that are logically part of this module but /// must be textually included. SmallVector TextualHeaders; @@ -94,6 +91,13 @@ public: /// \brief The headers that are private to this module. SmallVector PrivateHeaders; + /// \brief The headers that are private to this module and are to be + /// included textually. + SmallVector PrivateTextualHeaders; + + /// \brief The headers that are explicitly excluded from this module. + SmallVector ExcludedHeaders; + /// \brief Information about a header directive as found in the module map /// file. struct HeaderDirective { diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 2742b0f6c39b..553716b8f65a 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -65,20 +65,21 @@ private: llvm::StringMap Modules; public: - /// \brief Describes the role of a module header. + /// \brief Flags describing the role of a module header. enum ModuleHeaderRole { /// \brief This header is normally included in the module. - NormalHeader, + NormalHeader = 0x0, /// \brief This header is included but private. - PrivateHeader, + PrivateHeader = 0x1, /// \brief This header is part of the module (for layering purposes) but /// should be textually included. - TextualHeader, + TextualHeader = 0x2, // Caution: Adding an enumerator needs other changes. // Adjust the number of bits for KnownHeader::Storage. // Adjust the bitfield HeaderFileInfo::HeaderRole size. // Adjust the HeaderFileInfoTrait::ReadData streaming. // Adjust the HeaderFileInfoTrait::EmitData streaming. + // Adjust ModuleMap::addHeader. }; /// \brief A header that is known to reside within a given module, diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 241c62e4ced3..1b70cfd0654b 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -645,6 +645,9 @@ namespace clang { /// \brief Specifies a header that is part of the module but must be /// textually included. SUBMODULE_TEXTUAL_HEADER = 14, + /// \brief Specifies a header that is private to this submodule but + /// must be textually included. + SUBMODULE_PRIVATE_TEXTUAL_HEADER = 15, }; /// \brief Record types used within a comments block. diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index c191e0199a96..fb4135c2bc1e 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -202,7 +202,7 @@ ModuleMap::findHeaderInUmbrellaDirs(const FileEntry *File, return KnownHeader(); } -// Returns 'true' if 'RequestingModule directly uses 'RequestedModule'. +// Returns true if RequestingModule directly uses RequestedModule. static bool directlyUses(const Module *RequestingModule, const Module *RequestedModule) { return std::find(RequestingModule->DirectUses.begin(), @@ -214,19 +214,19 @@ static bool violatesPrivateInclude(Module *RequestingModule, const FileEntry *IncFileEnt, ModuleMap::ModuleHeaderRole Role, Module *RequestedModule) { - #ifndef NDEBUG + bool IsPrivateRole = Role & ModuleMap::PrivateHeader; +#ifndef NDEBUG // Check for consistency between the module header role // as obtained from the lookup and as obtained from the module. // This check is not cheap, so enable it only for debugging. - SmallVectorImpl &PvtHdrs - = RequestedModule->PrivateHeaders; - SmallVectorImpl::iterator Look - = std::find(PvtHdrs.begin(), PvtHdrs.end(), IncFileEnt); - bool IsPrivate = Look != PvtHdrs.end(); - assert((IsPrivate && Role == ModuleMap::PrivateHeader) - || (!IsPrivate && Role != ModuleMap::PrivateHeader)); - #endif - return Role == ModuleMap::PrivateHeader && + bool IsPrivate = false; + for (auto *Hdrs : {&RequestedModule->PrivateHeaders, + &RequestedModule->PrivateTextualHeaders}) + IsPrivate |= + std::find(Hdrs->begin(), Hdrs->end(), IncFileEnt) != Hdrs->end(); + assert(IsPrivate == IsPrivateRole && "inconsistent headers and roles"); +#endif + return IsPrivateRole && RequestedModule->getTopLevelModule() != RequestingModule; } @@ -316,13 +316,13 @@ ModuleMap::findModuleForHeader(const FileEntry *File, HeadersMap::iterator Known = findKnownHeader(File); auto MakeResult = [&](ModuleMap::KnownHeader R) -> ModuleMap::KnownHeader { - if (!IncludeTextualHeaders && R.getRole() == ModuleMap::TextualHeader) + if (!IncludeTextualHeaders && (R.getRole() & ModuleMap::TextualHeader)) return ModuleMap::KnownHeader(); return R; }; if (Known != Headers.end()) { - ModuleMap::KnownHeader Result = KnownHeader(); + ModuleMap::KnownHeader Result; // Iterate over all modules that 'File' is part of to find the best fit. for (SmallVectorImpl::iterator I = Known->second.begin(), @@ -343,14 +343,9 @@ ModuleMap::findModuleForHeader(const FileEntry *File, !directlyUses(RequestingModule, I->getModule())) continue; - Result = *I; - // If 'File' is a public header of this module, this is as good as we - // are going to get. - // FIXME: If we have a RequestingModule, we should prefer the header from - // that module. - if (I->getRole() == ModuleMap::NormalHeader || - I->getRole() == ModuleMap::TextualHeader) - break; + // Prefer a public header over a private header. + if (!Result || (Result.getRole() & ModuleMap::PrivateHeader)) + Result = *I; } return MakeResult(Result); } @@ -783,13 +778,12 @@ void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir) { void ModuleMap::addHeader(Module *Mod, const FileEntry *Header, ModuleHeaderRole Role) { - if (Role == TextualHeader) { - Mod->TextualHeaders.push_back(Header); - } else { - if (Role == PrivateHeader) - Mod->PrivateHeaders.push_back(Header); - else - Mod->NormalHeaders.push_back(Header); + auto HeaderLists = {&Mod->NormalHeaders, &Mod->PrivateHeaders, + &Mod->TextualHeaders, &Mod->PrivateTextualHeaders}; + assert(Role >= 0 && Role < HeaderLists.size() && "unknown header role"); + HeaderLists.begin()[Role]->push_back(Header); + + if (!(Role & TextualHeader)) { bool isCompilingModuleHeader = Mod->getTopLevelModule() == CompilingModule; HeaderInfo.MarkFileModuleHeader(Header, Role, isCompilingModuleHeader); } @@ -1475,16 +1469,9 @@ void ModuleMapParser::parseModuleDecl() { parseRequiresDecl(); break; - case MMToken::TextualKeyword: { - SourceLocation TextualLoc = consumeToken(); - if (Tok.is(MMToken::HeaderKeyword)) { - parseHeaderDecl(MMToken::TextualKeyword, TextualLoc); - } else { - Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header) - << "textual"; - } + case MMToken::TextualKeyword: + parseHeaderDecl(MMToken::TextualKeyword, consumeToken()); break; - } case MMToken::UmbrellaKeyword: { SourceLocation UmbrellaLoc = consumeToken(); @@ -1494,31 +1481,17 @@ void ModuleMapParser::parseModuleDecl() { parseUmbrellaDirDecl(UmbrellaLoc); break; } - - case MMToken::ExcludeKeyword: { - SourceLocation ExcludeLoc = consumeToken(); - if (Tok.is(MMToken::HeaderKeyword)) { - parseHeaderDecl(MMToken::ExcludeKeyword, ExcludeLoc); - } else { - Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header) - << "exclude"; - } + + case MMToken::ExcludeKeyword: + parseHeaderDecl(MMToken::ExcludeKeyword, consumeToken()); break; - } - - case MMToken::PrivateKeyword: { - SourceLocation PrivateLoc = consumeToken(); - if (Tok.is(MMToken::HeaderKeyword)) { - parseHeaderDecl(MMToken::PrivateKeyword, PrivateLoc); - } else { - Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header) - << "private"; - } + + case MMToken::PrivateKeyword: + parseHeaderDecl(MMToken::PrivateKeyword, consumeToken()); break; - } - + case MMToken::HeaderKeyword: - parseHeaderDecl(MMToken::HeaderKeyword, SourceLocation()); + parseHeaderDecl(MMToken::HeaderKeyword, consumeToken()); break; case MMToken::LinkKeyword: @@ -1673,16 +1646,37 @@ static void appendSubframeworkPaths(Module *Mod, /// \brief Parse a header declaration. /// /// header-declaration: -/// 'exclude'[opt] 'header' string-literal -/// 'private'[opt] 'header' string-literal /// 'textual'[opt] 'header' string-literal -/// 'umbrella'[opt] 'header' string-literal +/// 'private' 'textual'[opt] 'header' string-literal +/// 'exclude' 'header' string-literal +/// 'umbrella' 'header' string-literal /// /// FIXME: Support 'private textual header'. void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, SourceLocation LeadingLoc) { - assert(Tok.is(MMToken::HeaderKeyword)); - consumeToken(); + // We've already consumed the first token. + ModuleMap::ModuleHeaderRole Role = ModuleMap::NormalHeader; + if (LeadingToken == MMToken::PrivateKeyword) { + Role = ModuleMap::PrivateHeader; + // 'private' may optionally be followed by 'textual'. + if (Tok.is(MMToken::TextualKeyword)) { + LeadingToken = Tok.Kind; + consumeToken(); + } + } + if (LeadingToken == MMToken::TextualKeyword) + Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader); + + if (LeadingToken != MMToken::HeaderKeyword) { + if (!Tok.is(MMToken::HeaderKeyword)) { + Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header) + << (LeadingToken == MMToken::PrivateKeyword ? "private" : + LeadingToken == MMToken::ExcludeKeyword ? "exclude" : + LeadingToken == MMToken::TextualKeyword ? "textual" : "umbrella"); + return; + } + consumeToken(); + } // Parse the header name. if (!Tok.is(MMToken::StringLiteral)) { @@ -1770,21 +1764,13 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, } else if (LeadingToken == MMToken::ExcludeKeyword) { Map.excludeHeader(ActiveModule, File); } else { - // Record this header. - ModuleMap::ModuleHeaderRole Role = ModuleMap::NormalHeader; - if (LeadingToken == MMToken::PrivateKeyword) - Role = ModuleMap::PrivateHeader; - else if (LeadingToken == MMToken::TextualKeyword) - Role = ModuleMap::TextualHeader; - else - assert(LeadingToken == MMToken::HeaderKeyword); - // If there is a builtin counterpart to this file, add it now, before // the "real" header, so we build the built-in one first when building // the module. if (BuiltinFile) Map.addHeader(ActiveModule, BuiltinFile, Role); + // Record this header. Map.addHeader(ActiveModule, File, Role); } } else if (LeadingToken != MMToken::ExcludeKeyword) { diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 1fd39faab7fd..1d7efb81827f 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -4461,26 +4461,19 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; } - case SUBMODULE_HEADER: { - // We lazily associate headers with their modules via the HeaderInfoTable. + case SUBMODULE_HEADER: + case SUBMODULE_EXCLUDED_HEADER: + case SUBMODULE_PRIVATE_HEADER: + // We lazily associate headers with their modules via the HeaderInfo table. // FIXME: Re-evaluate this section; maybe only store InputFile IDs instead // of complete filenames or remove it entirely. - break; - } + break; - case SUBMODULE_EXCLUDED_HEADER: { - // We lazily associate headers with their modules via the HeaderInfoTable. - // FIXME: Re-evaluate this section; maybe only store InputFile IDs instead - // of complete filenames or remove it entirely. - break; - } - - case SUBMODULE_PRIVATE_HEADER: { - // We lazily associate headers with their modules via the HeaderInfoTable. - // FIXME: Re-evaluate this section; maybe only store InputFile IDs instead - // of complete filenames or remove it entirely. - break; - } + case SUBMODULE_TEXTUAL_HEADER: + case SUBMODULE_PRIVATE_TEXTUAL_HEADER: + // FIXME: Textual headers are not marked in the HeaderInfo table. Load + // them here. + break; case SUBMODULE_TOPHEADER: { CurrentModule->addTopHeaderFilename(Blob); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index e39e5bb0222b..000a7a9001f4 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -2370,7 +2370,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { } // Enter the submodule description block. - Stream.EnterSubblock(SUBMODULE_BLOCK_ID, /*bits for abbreviations*/4); + Stream.EnterSubblock(SUBMODULE_BLOCK_ID, /*bits for abbreviations*/5); // Write the abbreviations needed for the submodules block. using namespace llvm; @@ -2430,6 +2430,11 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name unsigned PrivateHeaderAbbrev = Stream.EmitAbbrev(Abbrev); + Abbrev = new BitCodeAbbrev(); + Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_PRIVATE_TEXTUAL_HEADER)); + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name + unsigned PrivateTextualHeaderAbbrev = Stream.EmitAbbrev(Abbrev); + Abbrev = new BitCodeAbbrev(); Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_LINK_LIBRARY)); Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework @@ -2504,40 +2509,25 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { } // Emit the headers. - for (unsigned I = 0, N = Mod->NormalHeaders.size(); I != N; ++I) { + struct { + unsigned Kind; + unsigned Abbrev; + ArrayRef Headers; + } HeaderLists[] = { + {SUBMODULE_HEADER, HeaderAbbrev, Mod->NormalHeaders}, + {SUBMODULE_TEXTUAL_HEADER, TextualHeaderAbbrev, Mod->TextualHeaders}, + {SUBMODULE_PRIVATE_HEADER, PrivateHeaderAbbrev, Mod->PrivateHeaders}, + {SUBMODULE_PRIVATE_TEXTUAL_HEADER, PrivateTextualHeaderAbbrev, + Mod->PrivateTextualHeaders}, + {SUBMODULE_EXCLUDED_HEADER, ExcludedHeaderAbbrev, Mod->ExcludedHeaders}, + {SUBMODULE_TOPHEADER, TopHeaderAbbrev, + Mod->getTopHeaders(PP->getFileManager())} + }; + for (auto &HL : HeaderLists) { Record.clear(); - Record.push_back(SUBMODULE_HEADER); - Stream.EmitRecordWithBlob(HeaderAbbrev, Record, - Mod->NormalHeaders[I]->getName()); - } - // Emit the excluded headers. - for (unsigned I = 0, N = Mod->ExcludedHeaders.size(); I != N; ++I) { - Record.clear(); - Record.push_back(SUBMODULE_EXCLUDED_HEADER); - Stream.EmitRecordWithBlob(ExcludedHeaderAbbrev, Record, - Mod->ExcludedHeaders[I]->getName()); - } - // Emit the textual headers. - for (unsigned I = 0, N = Mod->TextualHeaders.size(); I != N; ++I) { - Record.clear(); - Record.push_back(SUBMODULE_TEXTUAL_HEADER); - Stream.EmitRecordWithBlob(TextualHeaderAbbrev, Record, - Mod->TextualHeaders[I]->getName()); - } - // Emit the private headers. - for (unsigned I = 0, N = Mod->PrivateHeaders.size(); I != N; ++I) { - Record.clear(); - Record.push_back(SUBMODULE_PRIVATE_HEADER); - Stream.EmitRecordWithBlob(PrivateHeaderAbbrev, Record, - Mod->PrivateHeaders[I]->getName()); - } - ArrayRef - TopHeaders = Mod->getTopHeaders(PP->getFileManager()); - for (unsigned I = 0, N = TopHeaders.size(); I != N; ++I) { - Record.clear(); - Record.push_back(SUBMODULE_TOPHEADER); - Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, - TopHeaders[I]->getName()); + Record.push_back(HL.Kind); + for (auto *H : HL.Headers) + Stream.EmitRecordWithBlob(HL.Abbrev, Record, H->getName()); } // Emit the imports. diff --git a/clang/test/Modules/Inputs/declare-use/m.h b/clang/test/Modules/Inputs/declare-use/m.h new file mode 100644 index 000000000000..e9089ab725d5 --- /dev/null +++ b/clang/test/Modules/Inputs/declare-use/m.h @@ -0,0 +1,8 @@ +#ifdef GIMME_AN_M + +#ifndef M_H +#define M_H +const int m = 42; +#endif + +#endif diff --git a/clang/test/Modules/Inputs/declare-use/m2.h b/clang/test/Modules/Inputs/declare-use/m2.h new file mode 100644 index 000000000000..9e72835c3bce --- /dev/null +++ b/clang/test/Modules/Inputs/declare-use/m2.h @@ -0,0 +1 @@ +#include "m.h" diff --git a/clang/test/Modules/Inputs/declare-use/module.map b/clang/test/Modules/Inputs/declare-use/module.map index c6c6c951bd01..ae8615278aca 100644 --- a/clang/test/Modules/Inputs/declare-use/module.map +++ b/clang/test/Modules/Inputs/declare-use/module.map @@ -61,5 +61,10 @@ module XL { textual header "l.h" } +module XM { + private textual header "m.h" + textual header "m2.h" +} + module XS { } diff --git a/clang/test/Modules/textual-headers.cpp b/clang/test/Modules/textual-headers.cpp index d1be8ad0321a..cab9991e3291 100644 --- a/clang/test/Modules/textual-headers.cpp +++ b/clang/test/Modules/textual-headers.cpp @@ -1,6 +1,6 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -fmodule-maps -fmodules-cache-path=%t -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify -fno-modules-error-recovery #define GIMME_A_K #include "k.h" @@ -8,4 +8,11 @@ #define GIMME_AN_L #include "l.h" // expected-error {{module XG does not depend on a module exporting 'l.h'}} +#include "m2.h" // expected-error {{module XG does not depend on a module exporting 'm2.h'}} +const int use_m = m; // expected-error {{undeclared identifier}} + +#define GIMME_AN_M +#include "m.h" // expected-error {{use of private header from outside its module: 'm.h'}} +const int use_m_2 = m; + const int g = k + l;