diff options
10 files changed, 110 insertions, 4 deletions
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index c1b831b771a..ba82b36cea3 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -243,6 +243,7 @@ def NonModularIncludeInModule : DiagGroup<"non-modular-include-in-module", [NonModularIncludeInFrameworkModule]>; def IncompleteModule : DiagGroup<"incomplete-module", [IncompleteUmbrella, NonModularIncludeInModule]>; +def PrivateModule : DiagGroup<"private-module">; def CXX11InlineNamespace : DiagGroup<"c++11-inline-namespace">; def InvalidNoreturn : DiagGroup<"invalid-noreturn">; diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 8bcbc21c801..3b602a16c83 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -642,6 +642,12 @@ def err_mmap_expected_feature : Error<"expected a feature name">; def err_mmap_expected_attribute : Error<"expected an attribute name">; def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">, InGroup<IgnoredAttributes>; +def warn_mmap_mismatched_top_level_private : Warning< + "top-level module '%0' in private module map, expected a submodule of '%1'">, + InGroup<PrivateModule>; +def note_mmap_rename_top_level_private_as_submodule : Note< + "make '%0' a submodule of '%1' to ensure it can be found by name">, + InGroup<PrivateModule>; def warn_auto_module_import : Warning< "treating #%select{include|import|include_next|__include_macros}0 as an " diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 60214df7835..b145d7bae15 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -547,6 +547,19 @@ public: 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. /// diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 72e8b456822..b5228fc6c8c 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -194,16 +194,36 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch) { 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; diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index c1b7b33c31e..b7bba7ea9b8 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -1490,6 +1490,42 @@ void ModuleMapParser::parseModuleDecl() { 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; + // The pattern we're defending against here is typically due to + // a module named FooPrivate which is supposed to be a submodule + // called Foo.Private. Emit a fixit in that case. + auto D = + Diags.Report(ActiveModule->DefinitionLoc, + diag::note_mmap_rename_top_level_private_as_submodule); + D << ActiveModule->Name << M->Name; + StringRef Bad(ActiveModule->Name); + if (Bad.consume_back("Private")) { + SmallString<128> Fixed = Bad; + Fixed.append(".Private"); + D << FixItHint::CreateReplacement(ActiveModule->DefinitionLoc, + Fixed); + } + break; + } + } + } + } + bool Done = false; do { switch (Tok.Kind) { diff --git a/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h new file mode 100644 index 00000000000..8b4b1987450 --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h @@ -0,0 +1 @@ +extern int APUBLIC; diff --git a/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h new file mode 100644 index 00000000000..760d901aa36 --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h @@ -0,0 +1 @@ +extern int APRIVATE; diff --git a/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap new file mode 100644 index 00000000000..95eabf90a96 --- /dev/null +++ b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +framework module A { + header "a.h" + export * +} diff --git a/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap b/clang/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap new file mode 100644 index 00000000000..020d5d31d85 --- /dev/null +++ b/clang/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 * +} diff --git a/clang/test/Modules/implicit-private-with-different-name.m b/clang/test/Modules/implicit-private-with-different-name.m new file mode 100644 index 00000000000..8da331cb6a2 --- /dev/null +++ b/clang/test/Modules/implicit-private-with-different-name.m @@ -0,0 +1,20 @@ +// 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 + +// Check the fixit +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{top-level module 'APrivate' in private module map, expected a submodule of 'A'}} +// expected-note@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{make 'APrivate' a submodule of 'A' to ensure it can be found by name}} +// CHECK: fix-it:"{{.*}}/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap":{1:18-1:26}:"A.Private" + +#ifndef HEADER +#define HEADER +#import "A/aprivate.h" +const int *y = &APRIVATE; +#endif |
