This revision was automatically updated to reflect the committed changes.
Closed by commit rG586547687946: [clang][modules] Fix handling of
`ModuleHeaderRole::ExcludedHeader` (authored by jansvoboda11).
Changed prior to commit:
https://reviews.llvm.org/D135381?vs=465802&id=465921#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135381/new/
https://reviews.llvm.org/D135381
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/exclude-header-fw-umbrella.m
Index: clang/test/Modules/exclude-header-fw-umbrella.m
===================================================================
--- /dev/null
+++ 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 <A/Sub.h>
+//--- frameworks/A.framework/Headers/Sub.h
+//--- frameworks/A.framework/Headers/Excluded.h
+#import <A/Sub.h>
+@interface I
+@end
+
+//--- tu.m
+#import <A/Excluded.h>
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -iframework %t/frameworks -fsyntax-only %t/tu.m -verify
+// expected-no-diagnostics
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ 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())
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ 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.
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ 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)
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ 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<FileEntryRef>
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) {
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ 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) {
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ 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.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
