https://github.com/torarnv created https://github.com/llvm/llvm-project/pull/223706
Module maps could drop a single header from an umbrella with `exclude header "path"`, but had no way to drop a whole subdirectory. A directory of private headers meant listing every file by exact path, with no globbing. Add `exclude umbrella "path"`, which drops a subdirectory from the enclosing umbrella directory for the declaring module, the same way `exclude header` drops a single header. Headers under the directory are not part of the module, but clang still recognizes them as deliberately excluded rather than stray non-modular headers. Other headers under the umbrella still resolve, and the exclusion is scoped to the module that declares it, so a separate module whose umbrella also covers the directory is unaffected. The grammar mirrors the umbrella declarations it undoes: `umbrella "dir"` is dropped by `exclude umbrella "dir"`, reusing the existing `umbrella` keyword and its string-means-directory form. This leaves room for an `exclude umbrella header` counterpart later. Resolution records excluded directories in ModuleMap::ExcludedDirs. findHeaderInUmbrellaDirs reports a header under such a directory with the ExcludedHeader role, and findKnownHeader materializes that into the Headers map on first lookup. Every Headers map consumer then treats the header exactly like one dropped by `exclude header`, including staying quiet about non-modular includes. isHeaderUnavailableInModule repeats the check directly because it is const and runs during the umbrella build enumeration, before any lookup has recorded the header. Fixes #162847 Assisted-by: Claude Opus 4.8 From 0104ab948e1168b78474f5144b3322f79e0cb345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= <[email protected]> Date: Tue, 15 Sep 2026 14:09:55 +0200 Subject: [PATCH] [clang][modules] Add 'exclude umbrella' module map directive Module maps could drop a single header from an umbrella with `exclude header "path"`, but had no way to drop a whole subdirectory. A directory of private headers meant listing every file by exact path, with no globbing. Add `exclude umbrella "path"`, which drops a subdirectory from the enclosing umbrella directory for the declaring module, the same way `exclude header` drops a single header. Headers under the directory are not part of the module, but clang still recognizes them as deliberately excluded rather than stray non-modular headers. Other headers under the umbrella still resolve, and the exclusion is scoped to the module that declares it, so a separate module whose umbrella also covers the directory is unaffected. The grammar mirrors the umbrella declarations it undoes: `umbrella "dir"` is dropped by `exclude umbrella "dir"`, reusing the existing `umbrella` keyword and its string-means-directory form. This leaves room for an `exclude umbrella header` counterpart later. Resolution records excluded directories in ModuleMap::ExcludedDirs. findHeaderInUmbrellaDirs reports a header under such a directory with the ExcludedHeader role, and findKnownHeader materializes that into the Headers map on first lookup. Every Headers map consumer then treats the header exactly like one dropped by `exclude header`, including staying quiet about non-modular includes. isHeaderUnavailableInModule repeats the check directly because it is const and runs during the umbrella build enumeration, before any lookup has recorded the header. Fixes #162847 Assisted-by: Claude Opus 4.8 --- clang/docs/Modules.md | 16 +++ .../include/clang/Basic/DiagnosticLexKinds.td | 3 + clang/include/clang/Basic/Module.h | 5 + clang/include/clang/Lex/ModuleMap.h | 7 ++ clang/include/clang/Lex/ModuleMapFile.h | 12 +- clang/lib/Basic/Module.cpp | 7 ++ clang/lib/Lex/ModuleMap.cpp | 88 ++++++++++++- clang/lib/Lex/ModuleMapFile.cpp | 32 +++++ clang/test/Modules/exclude-umbrella-dir.m | 116 ++++++++++++++++++ 9 files changed, 281 insertions(+), 5 deletions(-) create mode 100644 clang/test/Modules/exclude-umbrella-dir.m diff --git a/clang/docs/Modules.md b/clang/docs/Modules.md index 234124cc22183..f8f9c86b21e7f 100644 --- a/clang/docs/Modules.md +++ b/clang/docs/Modules.md @@ -535,6 +535,7 @@ Modules can have a number of different kinds of members, each of which is descri *requires-declaration* *header-declaration* *umbrella-dir-declaration* + *exclude-umbrella-dir-declaration* *submodule-declaration* *export-declaration* *export-as-declaration* @@ -775,6 +776,21 @@ An *umbrella-dir-declaration* shall not refer to the same directory as the locat Umbrella directories are useful for libraries that have a large number of headers but do not have an umbrella header. ::: +#### Exclude umbrella directory declaration + +An exclude umbrella directory declaration drops a whole subdirectory from the enclosing umbrella directory, the same way `exclude header` drops a single header. + +```{eval-rst} +.. parsed-literal:: + + *exclude-umbrella-dir-declaration*: + ``exclude`` ``umbrella`` *string-literal* +``` + +The *string-literal* refers to a directory. Headers in that directory (and its subdirectories) are not part of the module, while every other header under the umbrella directory still is. + +The exclusion applies only to the module that declares it. A separate module whose umbrella directory also covers those headers is unaffected. + #### Submodule declaration Submodule declarations describe modules that are nested within their enclosing module. diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index ff51485a1810b..261d50542424e 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -997,6 +997,9 @@ def warn_uncovered_module_header : Warning< def warn_mmap_umbrella_dir_not_found : Warning< "umbrella directory '%0' not found">, InGroup<IncompleteUmbrella>; +def warn_mmap_exclude_dir_not_found : Warning< + "excluded directory '%0' not found">, + InGroup<IncompleteUmbrella>; def err_expected_id_building_module : Error< "expected a module name in '__building_module' expression">; def warn_use_of_private_header_outside_module : Warning< diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 453ad2b7e7600..afbf7765b067e 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -412,6 +412,11 @@ class alignas(8) Module { // The path to the umbrella entry relative to the root module's \c Directory. std::string UmbrellaRelativeToRootModuleDirectory; + /// Directories excluded from this module's umbrella directory, as written in + /// the module map's `exclude umbrella` declarations. Kept for printing the module + /// map back out; resolution uses ModuleMap::ExcludedDirs. + std::vector<std::string> ExcludedDirsAsWritten; + /// The module through which entities defined in this module will /// eventually be exposed, for use in "private" modules. std::string ExportAsModule; diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 12f8dbb0b6090..186956efa0bb2 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -219,6 +219,13 @@ class ModuleMap { /// header. llvm::DenseMap<const DirectoryEntry *, Module *> UmbrellaDirs; + /// Directories that a module has excluded from its umbrella directory via + /// an `exclude umbrella` declaration, mapped to the module that excluded them. + /// + /// A header under an excluded directory does not resolve to the excluding + /// module through that module's umbrella, as if the directory were absent. + llvm::DenseMap<const DirectoryEntry *, Module *> ExcludedDirs; + /// Mapping from (header, (sub)module) pairs to the source location where /// the header was added to the module (the header directive location). /// TODO: Consider moving this into Module::Header and serializing it into diff --git a/clang/include/clang/Lex/ModuleMapFile.h b/clang/include/clang/Lex/ModuleMapFile.h index 59389cd85a928..3226062f4cce3 100644 --- a/clang/include/clang/Lex/ModuleMapFile.h +++ b/clang/include/clang/Lex/ModuleMapFile.h @@ -31,9 +31,10 @@ struct ExportDecl; /// All declarations that can appear in a `module` declaration. using Decl = std::variant<struct RequiresDecl, struct HeaderDecl, struct UmbrellaDirDecl, - struct ModuleDecl, struct ExcludeDecl, struct ExportDecl, - struct ExportAsDecl, struct ExternModuleDecl, struct UseDecl, - struct LinkDecl, struct ConfigMacrosDecl, struct ConflictDecl>; + struct ExcludeDirDecl, struct ModuleDecl, struct ExcludeDecl, + struct ExportDecl, struct ExportAsDecl, struct ExternModuleDecl, + struct UseDecl, struct LinkDecl, struct ConfigMacrosDecl, + struct ConflictDecl>; struct RequiresFeature { StringRef Feature; @@ -67,6 +68,11 @@ struct UmbrellaDirDecl { SourceLocation Location; }; +struct ExcludeDirDecl { + StringRef Path; + SourceLocation Location; +}; + struct ModuleDecl { ModuleId Id; SourceLocation Location; /// Points to the first keyword in the decl. diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 3e2f66f27a518..f136812d31d93 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -497,6 +497,13 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const { OS << "\"\n"; } + for (StringRef Dir : ExcludedDirsAsWritten) { + OS.indent(Indent + 2); + OS << "exclude umbrella \""; + OS.write_escaped(Dir); + OS << "\"\n"; + } + if (!ConfigMacros.empty() || ConfigMacrosExhaustive) { OS.indent(Indent + 2); OS << "config_macros "; diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 9e0202409dfb2..e280078deafc1 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -423,8 +423,21 @@ ModuleMap::HeadersMap::iterator ModuleMap::findKnownHeader(FileEntryRef File) { if (HeaderInfo.getHeaderSearchOpts().ImplicitModuleMaps && Known == Headers.end() && ModuleMap::isBuiltinHeader(File)) { HeaderInfo.loadTopLevelSystemModules(); - return Headers.find(File); + Known = Headers.find(File); + } + + // A header under a directory excluded from its umbrella module has no explicit + // Headers entry. Record one lazily with the ExcludedHeader role so every + // Headers map consumer treats it the same way as an `exclude header` header. + if (Known == Headers.end() && !ExcludedDirs.empty()) { + SmallVector<DirectoryEntryRef, 2> IntermediateDirs; + KnownHeader H = findHeaderInUmbrellaDirs(File, IntermediateDirs); + if (H && H.getRole() == ExcludedHeader) { + Headers[File].push_back(H); + Known = Headers.find(File); + } } + return Known; } @@ -441,12 +454,28 @@ ModuleMap::KnownHeader ModuleMap::findHeaderInUmbrellaDirs( // and we need to resolve lookups as if we had found the embedded location. StringRef DirName = SourceMgr.getFileManager().getCanonicalName(*Dir); + // Modules that excluded a directory on the walk from the header up to its + // umbrella directory. If the umbrella we land on belongs to one of them, the + // header is invisible to that module, as if the excluded directory were + // absent. + llvm::SmallPtrSet<const Module *, 2> ExcludedByModules; + // Keep walking up the directory hierarchy, looking for a directory with // an umbrella header. do { + auto ExcludedDir = ExcludedDirs.find(*Dir); + if (ExcludedDir != ExcludedDirs.end()) + ExcludedByModules.insert(ExcludedDir->second); + auto KnownDir = UmbrellaDirs.find(*Dir); - if (KnownDir != UmbrellaDirs.end()) + if (KnownDir != UmbrellaDirs.end()) { + // A header under a directory that this umbrella's module excluded is + // reported with the ExcludedHeader role, so callers treat it the same way + // as a header dropped by an `exclude header` directive. + if (ExcludedByModules.contains(KnownDir->second)) + return KnownHeader(KnownDir->second, ExcludedHeader); return KnownHeader(KnownDir->second, NormalHeader); + } IntermediateDirs.push_back(*Dir); @@ -754,6 +783,10 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) { SmallVector<DirectoryEntryRef, 2> SkippedDirs; KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs); + // An excluded header is not part of the umbrella module, so don't infer a + // submodule for it. findKnownHeader records the exclusion in the Headers map. + if (H && H.getRole() == ExcludedHeader) + return {}; if (H) { Module *Result = H.getModule(); @@ -881,12 +914,26 @@ bool ModuleMap::isHeaderUnavailableInModule( M->isSubModuleOf(RequestingModule)); }; + // Modules that excluded a directory on the walk up to the umbrella directory. + // This repeats the ExcludedDirs check that findKnownHeader materializes into + // the Headers map, because this method is const and runs during the umbrella + // build enumeration, before any lookup has recorded the excluded header. + llvm::SmallPtrSet<const Module *, 2> ExcludedByModules; + // Keep walking up the directory hierarchy, looking for a directory with // an umbrella header. do { + if (auto ExcludedDir = ExcludedDirs.find(*Dir); + ExcludedDir != ExcludedDirs.end()) + ExcludedByModules.insert(ExcludedDir->second); + auto KnownDir = UmbrellaDirs.find(*Dir); if (KnownDir != UmbrellaDirs.end()) { Module *Found = KnownDir->second; + // A header under a directory that this umbrella's module excluded is not + // part of that module, as if the directory were absent. + if (ExcludedByModules.contains(Found)) + return true; if (IsUnavailable(Found)) return true; @@ -1744,6 +1791,7 @@ class ModuleMapLoader { void handleRequiresDecl(const modulemap::RequiresDecl &RD); void handleHeaderDecl(const modulemap::HeaderDecl &HD); void handleUmbrellaDirDecl(const modulemap::UmbrellaDirDecl &UDD); + void handleExcludeDirDecl(const modulemap::ExcludeDirDecl &EDD); void handleExportDecl(const modulemap::ExportDecl &ED); void handleExportAsDecl(const modulemap::ExportAsDecl &EAD); void handleUseDecl(const modulemap::UseDecl &UD); @@ -1994,6 +2042,9 @@ void ModuleMapLoader::handleModuleDecl(const modulemap::ModuleDecl &MD) { [&](const modulemap::UmbrellaDirDecl &UDD) { handleUmbrellaDirDecl(UDD); }, + [&](const modulemap::ExcludeDirDecl &EDD) { + handleExcludeDirDecl(EDD); + }, [&](const modulemap::ModuleDecl &MD) { handleModuleDecl(MD); }, [&](const modulemap::ExportDecl &ED) { handleExportDecl(ED); }, [&](const modulemap::ExportAsDecl &EAD) { @@ -2245,6 +2296,39 @@ void ModuleMapLoader::handleUmbrellaDirDecl( UDD.Location); } +void ModuleMapLoader::handleExcludeDirDecl( + const modulemap::ExcludeDirDecl &EDD) { + std::string DirName = std::string(EDD.Path); + + if (ImplicitlyDiscovered) { + SmallString<128> NormalizedPath(EDD.Path); + llvm::sys::path::remove_dots(NormalizedPath, /*remove_dot_dot=*/true); + if (NormalizedPath.starts_with("..")) + Diags.Report(EDD.Location, diag::warn_mmap_path_outside_directory); + } + + // Look for this directory. + OptionalDirectoryEntryRef Dir; + if (llvm::sys::path::is_absolute(DirName)) { + Dir = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName); + } else { + SmallString<128> PathName; + PathName = Directory.getName(); + llvm::sys::path::append(PathName, DirName); + Dir = SourceMgr.getFileManager().getOptionalDirectoryRef(PathName); + } + + if (!Dir) { + Diags.Report(EDD.Location, diag::warn_mmap_exclude_dir_not_found) << DirName; + return; + } + + // Record this excluded directory, scoped to the active module. Its headers + // will not resolve to this module through its umbrella directory. + Map.ExcludedDirs[*Dir] = ActiveModule; + ActiveModule->ExcludedDirsAsWritten.push_back(std::move(DirName)); +} + void ModuleMapLoader::handleExportDecl(const modulemap::ExportDecl &ED) { Module::UnresolvedExportDecl Unresolved = {ED.Location, ED.Id, ED.Wildcard}; ActiveModule->UnresolvedExports.push_back(Unresolved); diff --git a/clang/lib/Lex/ModuleMapFile.cpp b/clang/lib/Lex/ModuleMapFile.cpp index 4ca33cb86ddd3..d8ba08125f0e7 100644 --- a/clang/lib/Lex/ModuleMapFile.cpp +++ b/clang/lib/Lex/ModuleMapFile.cpp @@ -116,6 +116,8 @@ struct ModuleMapFileParser { std::optional<HeaderDecl> parseHeaderDecl(MMToken::TokenKind LeadingToken, SourceLocation LeadingLoc); std::optional<ExcludeDecl> parseExcludeDecl(clang::SourceLocation LeadingLoc); + std::optional<ExcludeDirDecl> + parseExcludeUmbrellaDecl(clang::SourceLocation LeadingLoc); std::optional<UmbrellaDirDecl> parseUmbrellaDirDecl(SourceLocation UmbrellaLoc); std::optional<LinkDecl> parseLinkDecl(); @@ -393,6 +395,8 @@ std::optional<ModuleDecl> ModuleMapFileParser::parseModuleDecl(bool TopLevel) { SourceLocation ExcludeLoc = consumeToken(); if (Tok.is(MMToken::HeaderKeyword)) SubDecl = parseHeaderDecl(MMToken::ExcludeKeyword, ExcludeLoc); + else if (Tok.is(MMToken::UmbrellaKeyword)) + SubDecl = parseExcludeUmbrellaDecl(ExcludeLoc); else SubDecl = parseExcludeDecl(ExcludeLoc); break; @@ -799,6 +803,30 @@ ModuleMapFileParser::parseExcludeDecl(clang::SourceLocation LeadingLoc) { return std::move(ED); } +/// Parse an exclude umbrella directory declaration. +/// +/// exclude-umbrella-dir-declaration: +/// 'exclude' 'umbrella' string-literal +std::optional<ExcludeDirDecl> +ModuleMapFileParser::parseExcludeUmbrellaDecl(clang::SourceLocation LeadingLoc) { + assert(Tok.is(MMToken::UmbrellaKeyword)); + consumeToken(); // 'umbrella' keyword + + ExcludeDirDecl EDD; + EDD.Location = LeadingLoc; + // Parse the directory name. + if (!Tok.is(MMToken::StringLiteral)) { + Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header) + << "exclude umbrella"; + HadError = true; + return std::nullopt; + } + + EDD.Path = Tok.getString(); + consumeToken(); + return std::move(EDD); +} + /// Parse an umbrella directory declaration. /// /// umbrella-dir-declaration: @@ -1181,6 +1209,10 @@ static void dumpDecls(ArrayRef<Decl> Decls, llvm::raw_ostream &out, int depth) { out.indent(depth * 2); out << "umbrella\n"; }, + [&](const ExcludeDirDecl &EDD) { + out.indent(depth * 2); + out << "exclude umbrella \"" << EDD.Path << "\"\n"; + }, [&](const ModuleDecl &MD) { dumpModule(MD, out, depth); }, [&](const ExcludeDecl &ED) { out.indent(depth * 2); diff --git a/clang/test/Modules/exclude-umbrella-dir.m b/clang/test/Modules/exclude-umbrella-dir.m new file mode 100644 index 0000000000000..8f68a416c34ff --- /dev/null +++ b/clang/test/Modules/exclude-umbrella-dir.m @@ -0,0 +1,116 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// A directory excluded via `exclude umbrella` is left out of the umbrella +// module, while other directories under the same umbrella still resolve. If the +// excluded directory were built into the module, secret.h's #error would fire. +// RUN: %clang_cc1 -x objective-c -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t/cache -I%t/basic %t/basic/tu.m -verify + +// An exclude in one module does not remove a directory from a different module +// whose umbrella also covers it. +// RUN: %clang_cc1 -x objective-c -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t/cache-scope -I%t/scope %t/scope/tu.m -verify + +// A nonexistent excluded directory is a warning, not an error: the module +// still builds. +// RUN: %clang_cc1 -fmodules -fmodule-name=Missing -x c++-module-map \ +// RUN: %t/missing/module.modulemap -emit-module -o /dev/null -verify + +// The directive round-trips through the module map printer. +// RUN: %clang_cc1 -fmodules -fmodule-name=RoundTrip -x c++-module-map \ +// RUN: %t/roundtrip/module.modulemap -E | FileCheck %t/roundtrip/module.modulemap + +// A header under an excluded directory is treated like an `exclude header` +// header: including it from the module's own headers is not reported as a +// non-modular include. +// RUN: %clang_cc1 -x objective-c -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t/cache-quiet -I%t/quiet \ +// RUN: -Wnon-modular-include-in-module -Werror %t/quiet/tu.m -verify + +//--- basic/module.modulemap +module Basic { + umbrella "inc" + exclude umbrella "inc/private" +} + +//--- basic/inc/root.h +typedef int basic_root; + +//--- basic/inc/pub/pub.h +typedef int basic_pub; + +//--- basic/inc/private/secret.h +#error secret.h must be excluded from module Basic + +//--- basic/tu.m +@import Basic; +basic_root use_root; +basic_pub use_pub; +// expected-no-diagnostics + +//--- scope/module.modulemap +module Excluder { + umbrella "excluder" + exclude umbrella "shared" +} +module Sharer { + umbrella "shared" +} + +//--- scope/excluder/e.h +typedef int excluder_e; + +//--- scope/shared/s.h +typedef int sharer_s; + +//--- scope/tu.m +// Sharer's umbrella covers "shared" even though Excluder excludes it, so +// sharer_s is part of Sharer and visible after the import. +@import Sharer; +sharer_s use_s; +// expected-no-diagnostics + +//--- missing/module.modulemap +module Missing { + umbrella "inc" + exclude umbrella "inc/does-not-exist" +} +#pragma clang module contents +// expected-warning@3 {{excluded directory 'inc/does-not-exist' not found}} + +//--- missing/inc/h.h +typedef int missing_h; + +//--- roundtrip/module.modulemap +// CHECK: module RoundTrip { +// CHECK: umbrella "inc" +// CHECK: exclude umbrella "inc/priv" +module RoundTrip { + umbrella "inc" + exclude umbrella "inc/priv" +} + +//--- roundtrip/inc/a.h +typedef int rt_a; + +//--- roundtrip/inc/priv/b.h +typedef int rt_b; + +//--- quiet/module.modulemap +module Quiet { + umbrella "inc" + exclude umbrella "inc/private" +} + +//--- quiet/inc/pub.h +#include "inc/private/priv.h" +typedef int quiet_pub; + +//--- quiet/inc/private/priv.h +typedef int quiet_priv; + +//--- quiet/tu.m +@import Quiet; +quiet_pub use_pub; +// expected-no-diagnostics _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
