diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -152,6 +152,9 @@ /// Convert a header role to a kind. static Module::HeaderKind headerRoleToKind(ModuleHeaderRole Role); + /// Check if the header with the given role is a modular one. + static bool isModular(ModuleHeaderRole Role); + /// A header that is known to reside within a given module, /// whether it was included or excluded. class KnownHeader { @@ -176,7 +179,7 @@ /// Whether this header is available in the module. bool isAvailable() const { - return getModule()->isAvailable(); + return getRole() != ExcludedHeader && getModule()->isAvailable(); } /// Whether this header is accessible from the specified module. @@ -691,9 +694,6 @@ void addHeader(Module *Mod, Module::Header Header, ModuleHeaderRole Role, bool Imported = false); - /// Marks this header as being excluded from the given module. - void excludeHeader(Module *Mod, Module::Header Header); - /// Parse the given module map file, and record any modules we /// encounter. /// diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -1344,7 +1344,7 @@ void HeaderSearch::MarkFileModuleHeader(const FileEntry *FE, ModuleMap::ModuleHeaderRole Role, bool isCompilingModuleHeader) { - bool isModularHeader = !(Role & ModuleMap::TextualHeader); + bool isModularHeader = ModuleMap::isModular(Role); // Don't mark the file info as non-external if there's nothing to change. if (!isCompilingModuleHeader) { diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -75,7 +75,6 @@ Module::HeaderKind ModuleMap::headerRoleToKind(ModuleHeaderRole Role) { switch ((int)Role) { - default: llvm_unreachable("unknown header role"); case NormalHeader: return Module::HK_Normal; case PrivateHeader: @@ -84,7 +83,10 @@ return Module::HK_Textual; case PrivateHeader | TextualHeader: return Module::HK_PrivateTextual; + case ExcludedHeader: + return Module::HK_Excluded; } + llvm_unreachable("unknown header role"); } ModuleMap::ModuleHeaderRole @@ -99,11 +101,15 @@ case Module::HK_PrivateTextual: return ModuleHeaderRole(PrivateHeader | TextualHeader); case Module::HK_Excluded: - llvm_unreachable("unexpected header kind"); + return ExcludedHeader; } llvm_unreachable("unknown header kind"); } +bool ModuleMap::isModular(ModuleHeaderRole Role) { + return !(Role & (ModuleMap::TextualHeader | ModuleMap::ExcludedHeader)); +} + Module::ExportDecl ModuleMap::resolveExport(Module *Mod, const Module::UnresolvedExportDecl &Unresolved, @@ -264,10 +270,7 @@ } else { Module::Header H = {Header.FileName, std::string(RelativePathName.str()), *File}; - if (Header.Kind == Module::HK_Excluded) - excludeHeader(Mod, H); - else - addHeader(Mod, H, headerKindToRole(Header.Kind)); + addHeader(Mod, H, headerKindToRole(Header.Kind)); } } else if (Header.HasBuiltinHeader && !Header.Size && !Header.ModTime) { // There's a builtin header but no corresponding on-disk header. Assume @@ -489,6 +492,12 @@ HeadersMap::iterator Known = findKnownHeader(File); if (Known != Headers.end()) { for (const KnownHeader &Header : Known->second) { + // Excluded headers don't really belong to a module. + if (Header.getRole() == ModuleMap::ExcludedHeader) { + Excluded = true; + continue; + } + // Remember private headers for later printing of a diagnostic. if (violatesPrivateInclude(RequestingModule, File, Header)) { Private = Header.getModule(); @@ -562,6 +571,11 @@ (Old.getRole() & ModuleMap::TextualHeader)) return !(New.getRole() & ModuleMap::TextualHeader); + // Prefer a non-excluded header over an excluded header. + if ((New.getRole() == ModuleMap::ExcludedHeader) != + (Old.getRole() == ModuleMap::ExcludedHeader)) + return New.getRole() != ModuleMap::ExcludedHeader; + // Don't have a reason to choose between these. Just keep the first one. return false; } @@ -570,8 +584,7 @@ bool AllowTextual, bool AllowExcluded) { auto MakeResult = [&](ModuleMap::KnownHeader R) -> ModuleMap::KnownHeader { - if ((!AllowTextual && R.getRole() & ModuleMap::TextualHeader) || - (!AllowExcluded && R.getRole() & ModuleMap::ExcludedHeader)) + if (!AllowTextual && R.getRole() & ModuleMap::TextualHeader) return {}; return R; }; @@ -581,6 +594,9 @@ ModuleMap::KnownHeader Result; // Iterate over all modules that 'File' is part of to find the best fit. for (KnownHeader &H : Known->second) { + // Cannot use a module if the header is excluded in it. + if (!AllowExcluded && H.getRole() == ModuleMap::ExcludedHeader) + continue; // Prefer a header from the source module over all others. if (H.getModule()->getTopLevelModule() == SourceModule) return MakeResult(H); @@ -1267,18 +1283,6 @@ Cb->moduleMapAddHeader(Header.Entry->getName()); } -void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) { - KnownHeader KH(Mod, ModuleHeaderRole::ExcludedHeader); - - // Add this as a known header so we won't implicitly add it to any - // umbrella directory module. - // FIXME: Should we only exclude it from umbrella modules within the - // specified module? - Headers[Header.Entry].push_back(KH); - - Mod->Headers[Module::HK_Excluded].push_back(std::move(Header)); -} - Optional ModuleMap::getContainingModuleMapFile(const Module *Module) const { if (Module->DefinitionLoc.isInvalid()) @@ -2346,6 +2350,7 @@ SourceLocation LeadingLoc) { // 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'. @@ -2353,6 +2358,8 @@ LeadingToken = Tok.Kind; consumeToken(); } + } else if (LeadingToken == MMToken::ExcludeKeyword) { + Role = ModuleMap::ExcludedHeader; } if (LeadingToken == MMToken::TextualKeyword) @@ -2386,9 +2393,7 @@ Header.FileName = std::string(Tok.getString()); Header.FileNameLoc = consumeToken(); Header.IsUmbrella = LeadingToken == MMToken::UmbrellaKeyword; - Header.Kind = - (LeadingToken == MMToken::ExcludeKeyword ? Module::HK_Excluded - : Map.headerRoleToKind(Role)); + Header.Kind = Map.headerRoleToKind(Role); // Check whether we already have an umbrella. if (Header.IsUmbrella && ActiveModule->Umbrella) { diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -910,6 +910,10 @@ continue; } + // Don't suggest explicitly excluded headers. + if (Header.getRole() == ModuleMap::ExcludedHeader) + continue; + // We'll suggest including textual headers below if they're // include-guarded. if (Header.getRole() & ModuleMap::TextualHeader) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -1919,7 +1919,7 @@ Module::Header H = {std::string(key.Filename), "", *FileMgr.getFile(Filename)}; ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true); - HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader); + HFI.isModuleHeader |= ModuleMap::isModular(HeaderRole); } // This HeaderFileInfo was externally loaded. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1829,8 +1829,6 @@ } }; - // FIXME: If the header is excluded, we should write out some - // record of that fact. for (auto ModInfo : Data.KnownHeaders) EmitModule(ModInfo.getModule(), ModInfo.getRole()); if (Data.Unresolved.getPointer()) diff --git a/clang/test/Modules/exclude-header-fw-umbrella.m b/clang/test/Modules/exclude-header-fw-umbrella.m new file mode 100644 --- /dev/null +++ b/clang/test/Modules/exclude-header-fw-umbrella.m @@ -0,0 +1,25 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +//--- frameworks/A.framework/Modules/module.modulemap +framework module A { + umbrella header "A.h" + exclude header "Excluded.h" + + module Excluded { + header "Excluded.h" + } +} +//--- frameworks/A.framework/Headers/A.h +#import +//--- frameworks/A.framework/Headers/Sub.h +//--- frameworks/A.framework/Headers/Excluded.h +#import +@interface I +@end + +//--- tu.m +#import + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -iframework %t/frameworks -fsyntax-only %t/tu.m -verify +// expected-no-diagnostics