Index: include/clang/Basic/DiagnosticLexKinds.td =================================================================== --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -642,6 +642,9 @@ def err_mmap_expected_attribute : Error<"expected an attribute name">; def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">, InGroup; +def warn_mmap_mismatched_top_level_private : Warning< + "top-level module '%0' in private module map, expected a submodule of '%1'">, + InGroup>; def warn_auto_module_import : Warning< "treating #%select{include|import|include_next|__include_macros}0 as an " Index: include/clang/Lex/HeaderSearch.h =================================================================== --- include/clang/Lex/HeaderSearch.h +++ include/clang/Lex/HeaderSearch.h @@ -547,6 +547,19 @@ void loadTopLevelSystemModules(); private: + + /// \brief Lookup a module with the given module name and search-name. + /// + /// \param ModuleName The name of the module we're looking for. + /// + /// \param SearchName The "search-name" to derive filesystem paths from + /// when looking for the module map; this is usually equal to ModuleName, + /// but for compatibility with some buggy frameworks, additional attempts + /// may be made to find the module under a related-but-different search-name. + /// + /// \returns The module named ModuleName. + Module *lookupModule(StringRef ModuleName, StringRef SearchName); + /// \brief Retrieve a module with the given name, which may be part of the /// given framework. /// Index: lib/Lex/HeaderSearch.cpp =================================================================== --- lib/Lex/HeaderSearch.cpp +++ lib/Lex/HeaderSearch.cpp @@ -194,16 +194,39 @@ Module *Module = ModMap.findModule(ModuleName); if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps) return Module; - + + StringRef SearchName = ModuleName; + Module = lookupModule(ModuleName, SearchName); + + // The facility for "private modules" -- adjacent, optional module maps named + // module.private.modulemap that are supposed to define private submodules -- + // is sometimes misused by frameworks that name their associated private + // module FooPrivate, rather than as a submodule named Foo.Private as + // intended. Here we compensate for such cases by looking in directories named + // Foo.framework, when we previously looked and failed to find a + // FooPrivate.framework. + if (!Module && SearchName.consume_back("Private")) { + Module = lookupModule(ModuleName, SearchName); + } + + return Module; +} + +Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName) { + + Module *Module = nullptr; + // Look through the various header search paths to load any available module // maps, searching for a module map that describes this module. for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) { if (SearchDirs[Idx].isFramework()) { - // Search for or infer a module map for a framework. + // Search for or infer a module map for a framework. Here we use + // SearchName rather than ModuleName, to permit finding private modules + // named FooPrivate in buggy frameworks named Foo. SmallString<128> FrameworkDirName; FrameworkDirName += SearchDirs[Idx].getFrameworkDir()->getName(); - llvm::sys::path::append(FrameworkDirName, ModuleName + ".framework"); - if (const DirectoryEntry *FrameworkDir + llvm::sys::path::append(FrameworkDirName, SearchName + ".framework"); + if (const DirectoryEntry *FrameworkDir = FileMgr.getDirectory(FrameworkDirName)) { bool IsSystem = SearchDirs[Idx].getDirCharacteristic() != SrcMgr::C_User; Index: lib/Lex/ModuleMap.cpp =================================================================== --- lib/Lex/ModuleMap.cpp +++ lib/Lex/ModuleMap.cpp @@ -1490,6 +1490,28 @@ ActiveModule->NoUndeclaredIncludes = true; ActiveModule->Directory = Directory; + if (!ActiveModule->Parent) { + StringRef MapFileName(ModuleMapFile->getName()); + if (MapFileName.endswith("module.private.modulemap") || + MapFileName.endswith("module_private.map")) { + // Adding a top-level module from a private modulemap is likely a + // user error; we check to see if there's another top-level module + // defined in the non-private map in the same dir, and if so emit a + // warning. + for (auto E = Map.module_begin(); E != Map.module_end(); ++E) { + auto const *M = E->getValue(); + if (!M->Parent && + M->Directory == ActiveModule->Directory && + M->Name != ActiveModule->Name) { + Diags.Report(ActiveModule->DefinitionLoc, + diag::warn_mmap_mismatched_top_level_private) + << ActiveModule->Name << M->Name; + break; + } + } + } + } + bool Done = false; do { switch (Tok.Kind) { Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h =================================================================== --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h @@ -0,0 +1 @@ +extern int APUBLIC; Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h =================================================================== --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h @@ -0,0 +1 @@ +extern int APRIVATE; Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +framework module A { + header "a.h" + export * +} Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap @@ -0,0 +1,4 @@ +framework module APrivate { + header "aprivate.h" + export * +} Index: test/Modules/implicit-private-with-different-name.m =================================================================== --- /dev/null +++ test/Modules/implicit-private-with-different-name.m @@ -0,0 +1,15 @@ +// RUN: rm -rf %t + +// Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced +// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s + +// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only + +// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{expected a submodule}} + +#ifndef HEADER +#define HEADER +#import "A/aprivate.h" +const int *y = &APRIVATE; +#endif