diff options
Diffstat (limited to 'clang')
8 files changed, 100 insertions, 29 deletions
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 604b2ebdd08..643c178743b 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -285,6 +285,8 @@ def IncompatiblePointerTypes [IncompatiblePointerTypesDiscardsQualifiers, IncompatibleFunctionPointerTypes]>; def IncompleteUmbrella : DiagGroup<"incomplete-umbrella">; +def IncompleteFrameworkModuleDeclaration + : DiagGroup<"incomplete-framework-module-declaration">; def NonModularIncludeInFrameworkModule : DiagGroup<"non-modular-include-in-framework-module">; def NonModularIncludeInModule : DiagGroup<"non-modular-include-in-module", diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index bf13b5baef2..9b7f7e67775 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -694,6 +694,11 @@ def warn_mmap_mismatched_private_module_name : Warning< InGroup<PrivateModule>; def note_mmap_rename_top_level_private_module : Note< "rename '%0' to ensure it can be found by name">; +def warn_mmap_incomplete_framework_module_declaration : Warning< + "skipping '%0' because module declaration of '%1' lacks the 'framework' qualifier">, + InGroup<IncompleteFrameworkModuleDeclaration>; +def note_mmap_add_framework_keyword : Note< + "use 'framework module' to declare module '%0'">; def err_mmap_duplicate_header_attribute : Error< "header attribute '%0' specified multiple times">; diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 406bac5c12a..577f4008978 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -303,8 +303,15 @@ private: Module *resolveModuleId(const ModuleId &Id, Module *Mod, bool Complain) const; /// Add an unresolved header to a module. + /// + /// \param Mod The module in which we're adding the unresolved header + /// directive. + /// \param Header The unresolved header directive. + /// \param NeedsFramework If Mod is not a framework but a missing header would + /// be found in case Mod was, set it to true. False otherwise. void addUnresolvedHeader(Module *Mod, - Module::UnresolvedHeaderDirective Header); + Module::UnresolvedHeaderDirective Header, + bool &NeedsFramework); /// Look up the given header directive to find an actual header file. /// @@ -312,14 +319,22 @@ private: /// \param Header The header directive to resolve. /// \param RelativePathName Filled in with the relative path name from the /// module to the resolved header. + /// \param NeedsFramework If M is not a framework but a missing header would + /// be found in case M was, set it to true. False otherwise. /// \return The resolved file, if any. const FileEntry *findHeader(Module *M, const Module::UnresolvedHeaderDirective &Header, - SmallVectorImpl<char> &RelativePathName); + SmallVectorImpl<char> &RelativePathName, + bool &NeedsFramework); /// Resolve the given header directive. - void resolveHeader(Module *M, - const Module::UnresolvedHeaderDirective &Header); + /// + /// \param M The module in which we're resolving the header directive. + /// \param Header The header directive to resolve. + /// \param NeedsFramework If M is not a framework but a missing header would + /// be found in case M was, set it to true. False otherwise. + void resolveHeader(Module *M, const Module::UnresolvedHeaderDirective &Header, + bool &NeedsFramework); /// Attempt to resolve the specified header directive as naming a builtin /// header. diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 8343c8bb6b0..6a1731ef6f2 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -170,10 +170,13 @@ static void appendSubframeworkPaths(Module *Mod, llvm::sys::path::append(Path, "Frameworks", Paths[I-1] + ".framework"); } -const FileEntry * -ModuleMap::findHeader(Module *M, - const Module::UnresolvedHeaderDirective &Header, - SmallVectorImpl<char> &RelativePathName) { +const FileEntry *ModuleMap::findHeader( + Module *M, const Module::UnresolvedHeaderDirective &Header, + SmallVectorImpl<char> &RelativePathName, bool &NeedsFramework) { + // Search for the header file within the module's home directory. + auto *Directory = M->Directory; + SmallString<128> FullPathName(Directory->getName()); + auto GetFile = [&](StringRef Filename) -> const FileEntry * { auto *File = SourceMgr.getFileManager().getFile(Filename); if (!File || @@ -183,18 +186,8 @@ ModuleMap::findHeader(Module *M, return File; }; - if (llvm::sys::path::is_absolute(Header.FileName)) { - RelativePathName.clear(); - RelativePathName.append(Header.FileName.begin(), Header.FileName.end()); - return GetFile(Header.FileName); - } - - // Search for the header file within the module's home directory. - auto *Directory = M->Directory; - SmallString<128> FullPathName(Directory->getName()); - unsigned FullPathLength = FullPathName.size(); - - if (M->isPartOfFramework()) { + auto GetFrameworkFile = [&]() -> const FileEntry * { + unsigned FullPathLength = FullPathName.size(); appendSubframeworkPaths(M, RelativePathName); unsigned RelativePathLength = RelativePathName.size(); @@ -219,18 +212,46 @@ ModuleMap::findHeader(Module *M, Header.FileName); llvm::sys::path::append(FullPathName, RelativePathName); return GetFile(FullPathName); + }; + + if (llvm::sys::path::is_absolute(Header.FileName)) { + RelativePathName.clear(); + RelativePathName.append(Header.FileName.begin(), Header.FileName.end()); + return GetFile(Header.FileName); } + if (M->isPartOfFramework()) + return GetFrameworkFile(); + // Lookup for normal headers. llvm::sys::path::append(RelativePathName, Header.FileName); llvm::sys::path::append(FullPathName, RelativePathName); - return GetFile(FullPathName); + auto *NormalHdrFile = GetFile(FullPathName); + + if (M && !NormalHdrFile && Directory->getName().endswith(".framework")) { + // The lack of 'framework' keyword in a module declaration it's a simple + // mistake we can diagnose when the header exists within the proper + // framework style path. + FullPathName.assign(Directory->getName()); + RelativePathName.clear(); + if (auto *FrameworkHdr = GetFrameworkFile()) { + Diags.Report(Header.FileNameLoc, + diag::warn_mmap_incomplete_framework_module_declaration) + << Header.FileName << M->getFullModuleName(); + NeedsFramework = true; + } + return nullptr; + } + + return NormalHdrFile; } void ModuleMap::resolveHeader(Module *Mod, - const Module::UnresolvedHeaderDirective &Header) { + const Module::UnresolvedHeaderDirective &Header, + bool &NeedsFramework) { SmallString<128> RelativePathName; - if (const FileEntry *File = findHeader(Mod, Header, RelativePathName)) { + if (const FileEntry *File = + findHeader(Mod, Header, RelativePathName, NeedsFramework)) { if (Header.IsUmbrella) { const DirectoryEntry *UmbrellaDir = File->getDir(); if (Module *UmbrellaMod = UmbrellaDirs[UmbrellaDir]) @@ -1056,7 +1077,8 @@ void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir, } void ModuleMap::addUnresolvedHeader(Module *Mod, - Module::UnresolvedHeaderDirective Header) { + Module::UnresolvedHeaderDirective Header, + bool &NeedsFramework) { // If there is a builtin counterpart to this file, add it now so it can // wrap the system header. if (resolveAsBuiltinHeader(Mod, Header)) { @@ -1087,7 +1109,7 @@ void ModuleMap::addUnresolvedHeader(Module *Mod, // We don't have stat information or can't defer looking this file up. // Perform the lookup now. - resolveHeader(Mod, Header); + resolveHeader(Mod, Header, NeedsFramework); } void ModuleMap::resolveHeaderDirectives(const FileEntry *File) const { @@ -1107,10 +1129,11 @@ void ModuleMap::resolveHeaderDirectives(const FileEntry *File) const { } void ModuleMap::resolveHeaderDirectives(Module *Mod) const { + bool NeedsFramework = false; for (auto &Header : Mod->UnresolvedHeaders) // This operation is logically const; we're just changing how we represent // the header information for this file. - const_cast<ModuleMap*>(this)->resolveHeader(Mod, Header); + const_cast<ModuleMap*>(this)->resolveHeader(Mod, Header, NeedsFramework); Mod->UnresolvedHeaders.clear(); } @@ -1323,7 +1346,10 @@ namespace clang { /// The current module map file. const FileEntry *ModuleMapFile; - + + /// Source location of most recent parsed module declaration + SourceLocation CurrModuleDeclLoc; + /// The directory that file names in this module map file should /// be resolved relative to. const DirectoryEntry *Directory; @@ -1743,7 +1769,7 @@ void ModuleMapParser::parseModuleDecl() { HadError = true; return; } - consumeToken(); // 'module' keyword + CurrModuleDeclLoc = consumeToken(); // 'module' keyword // If we have a wildcard for the module name, this is an inferred submodule. // Parse it. @@ -2267,7 +2293,13 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, } } - Map.addUnresolvedHeader(ActiveModule, std::move(Header)); + bool NeedsFramework = false; + Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework); + + if (NeedsFramework && ActiveModule) + Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword) + << ActiveModule->getFullModuleName() + << FixItHint::CreateReplacement(CurrModuleDeclLoc, "framework module"); } static int compareModuleHeaders(const Module::Header *A, diff --git a/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h b/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h new file mode 100644 index 00000000000..547f1bc6b08 --- /dev/null +++ b/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h @@ -0,0 +1 @@ +// Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h diff --git a/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h b/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h new file mode 100644 index 00000000000..eda7c170700 --- /dev/null +++ b/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h @@ -0,0 +1 @@ +// FooB.h diff --git a/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap new file mode 100644 index 00000000000..d928a53efcd --- /dev/null +++ b/clang/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +module Foo { + umbrella header "Foo.h" + header "FooB.h" +} diff --git a/clang/test/Modules/incomplete-framework-module.m b/clang/test/Modules/incomplete-framework-module.m new file mode 100644 index 00000000000..9034c1aab92 --- /dev/null +++ b/clang/test/Modules/incomplete-framework-module.m @@ -0,0 +1,11 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \ +// RUN: -F%S/Inputs/incomplete-framework-module \ +// RUN: -fsyntax-only %s -verify + +#import <Foo/Foo.h> + +// expected-warning@Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:2{{skipping 'Foo.h' because module declaration}} +// expected-warning@Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:3{{skipping 'FooB.h' because module declaration}} +// expected-note@Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:1{{use 'framework module' to declare module 'Foo'}} +// expected-note@Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:1{{use 'framework module' to declare module 'Foo'}} |
