[clang][modules] Fix handling of `ModuleHeaderRole::ExcludedHeader`

This is a follow-up to D134224. The original patch added new `ExcludedHeader` enumerator to `ModuleMap::ModuleHeaderRole` and started associating headers with the modules they were excluded from. This was necessary to consider their module maps as "affecting" in certain situations and in turn serialize them into the PCM.

The association of the header and module needs to be handled when deserializing the PCM as well, though. This patch fixes a potential assertion failure and a regression. This essentially reverts parts of feb54b6ded123f8118fdc20620d3f657dfeab485.

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D135381

1 files changed, 28 insertions, 23 deletions

diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index cbd3303f..2d89705 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -75,7 +75,6 @@ void ModuleMap::addLinkAsDependency(Module *Mod) {
 
 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 @@ Module::HeaderKind ModuleMap::headerRoleToKind(ModuleHeaderRole Role) {
     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 @@ ModuleMap::headerKindToRole(Module::HeaderKind Kind) {
   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 @@ void ModuleMap::resolveHeader(Module *Mod,
     } 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 @@ void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
   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 @@ static bool isBetterKnownHeader(const ModuleMap::KnownHeader &New,
       (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 @@ ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File,
                                                       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 ModuleMap::findModuleForHeader(const FileEntry *File,
     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 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header,
     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<FileEntryRef>
 ModuleMap::getContainingModuleMapFile(const Module *Module) const {
   if (Module->DefinitionLoc.isInvalid())
@@ -2346,6 +2350,7 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken,
                                       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 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken,
       LeadingToken = Tok.Kind;
       consumeToken();
     }
+  } else if (LeadingToken == MMToken::ExcludeKeyword) {
+    Role = ModuleMap::ExcludedHeader;
   }
 
   if (LeadingToken == MMToken::TextualKeyword)
@@ -2386,9 +2393,7 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken,
   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) {