r333718 - [Modules] Warning for module declarations lacking 'framework' qualifier
Bruno Cardoso Lopes via cfe-commits
cfe-commits at lists.llvm.org
Thu May 31 18:26:18 PDT 2018
Author: bruno
Date: Thu May 31 18:26:18 2018
New Revision: 333718
URL: http://llvm.org/viewvc/llvm-project?rev=333718&view=rev
Log:
[Modules] Warning for module declarations lacking 'framework' qualifier
When a module declaration for a framework lacks the 'framework'
qualifier, the listed headers aren't found (because there's no
trigger for the special framework style path lookup) and the module
is silently not built. This leads to frameworks not being modularized
by accident, which is pretty bad.
Add a warning and suggest the user to add the 'framework' qualifier
when we can prove that it's the case.
rdar://problem/39193062
Added:
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/
cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap
cfe/trunk/test/Modules/incomplete-framework-module.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/Lex/ModuleMap.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=333718&r1=333717&r2=333718&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu May 31 18:26:18 2018
@@ -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",
Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=333718&r1=333717&r2=333718&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Thu May 31 18:26:18 2018
@@ -694,6 +694,11 @@ def warn_mmap_mismatched_private_module_
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">;
Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=333718&r1=333717&r2=333718&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Thu May 31 18:26:18 2018
@@ -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.
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=333718&r1=333717&r2=333718&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Thu May 31 18:26:18 2018
@@ -170,10 +170,13 @@ static void appendSubframeworkPaths(Modu
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 *M
}
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(Modu
// 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(
}
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(MM
}
}
- 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,
Added: cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h?rev=333718&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h (added)
+++ cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h Thu May 31 18:26:18 2018
@@ -0,0 +1 @@
+// Inputs/incomplete-framework-module/Foo.framework/Headers/Foo.h
Added: cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h?rev=333718&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h (added)
+++ cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Headers/FooB.h Thu May 31 18:26:18 2018
@@ -0,0 +1 @@
+// FooB.h
Added: cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap?rev=333718&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap Thu May 31 18:26:18 2018
@@ -0,0 +1,4 @@
+module Foo {
+ umbrella header "Foo.h"
+ header "FooB.h"
+}
Added: cfe/trunk/test/Modules/incomplete-framework-module.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/incomplete-framework-module.m?rev=333718&view=auto
==============================================================================
--- cfe/trunk/test/Modules/incomplete-framework-module.m (added)
+++ cfe/trunk/test/Modules/incomplete-framework-module.m Thu May 31 18:26:18 2018
@@ -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 at Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:2{{skipping 'Foo.h' because module declaration}}
+// expected-warning at Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:3{{skipping 'FooB.h' because module declaration}}
+// expected-note at Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:1{{use 'framework module' to declare module 'Foo'}}
+// expected-note at Inputs/incomplete-framework-module/Foo.framework/Modules/module.modulemap:1{{use 'framework module' to declare module 'Foo'}}
More information about the cfe-commits
mailing list