diff options
16 files changed, 147 insertions, 5 deletions
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 7cc9967738c..56384db1246 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -32,6 +32,8 @@ def Availability : DiagGroup<"availability">; def Section : DiagGroup<"section">; def AutoImport : DiagGroup<"auto-import">; def FrameworkHdrQuotedInclude : DiagGroup<"quoted-include-in-framework-header">; +def FrameworkIncludePrivateFromPublic : + DiagGroup<"framework-include-private-from-public">; def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">; def CXXPre14CompatBinaryLiteral : DiagGroup<"c++98-c++11-compat-binary-literal">; def GNUBinaryLiteral : DiagGroup<"gnu-binary-literal">; diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index de6bffcdcaa..5d1e149eab3 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -718,6 +718,9 @@ def warn_quoted_include_in_framework_header : Warning< "double-quoted include \"%0\" in framework header, " "expected angle-bracketed instead" >, InGroup<FrameworkHdrQuotedInclude>, DefaultIgnore; +def warn_framework_include_private_from_public : Warning< + "public framework header includes private framework header '%0'" + >, InGroup<FrameworkIncludePrivateFromPublic>; def warn_auto_module_import : Warning< "treating #%select{include|import|include_next|__include_macros}0 as an " diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 757cd097e4e..312bd2d0615 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -621,11 +621,12 @@ static const char *copyString(StringRef Str, llvm::BumpPtrAllocator &Alloc) { return CopyStr; } -static bool isFrameworkStylePath(StringRef Path, +static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader, SmallVectorImpl<char> &FrameworkName) { using namespace llvm::sys; path::const_iterator I = path::begin(Path); path::const_iterator E = path::end(Path); + IsPrivateHeader = false; // Detect different types of framework style paths: // @@ -637,12 +638,16 @@ static bool isFrameworkStylePath(StringRef Path, // and some other variations among these lines. int FoundComp = 0; while (I != E) { + if (*I == "Headers") + ++FoundComp; if (I->endswith(".framework")) { FrameworkName.append(I->begin(), I->end()); ++FoundComp; } - if (*I == "Headers" || *I == "PrivateHeaders") + if (*I == "PrivateHeaders") { ++FoundComp; + IsPrivateHeader = true; + } ++I; } @@ -654,11 +659,13 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc, StringRef Includer, StringRef IncludeFilename, const FileEntry *IncludeFE, bool isAngled = false, bool FoundByHeaderMap = false) { + bool IsIncluderPrivateHeader = false; SmallString<128> FromFramework, ToFramework; - if (!isFrameworkStylePath(Includer, FromFramework)) + if (!isFrameworkStylePath(Includer, IsIncluderPrivateHeader, FromFramework)) return; - bool IsIncludeeInFramework = - isFrameworkStylePath(IncludeFE->getName(), ToFramework); + bool IsIncludeePrivateHeader = false; + bool IsIncludeeInFramework = isFrameworkStylePath( + IncludeFE->getName(), IsIncludeePrivateHeader, ToFramework); if (!isAngled && !FoundByHeaderMap) { SmallString<128> NewInclude("<"); @@ -672,6 +679,14 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc, << IncludeFilename << FixItHint::CreateReplacement(IncludeLoc, NewInclude); } + + // Headers in Foo.framework/Headers should not include headers + // from Foo.framework/PrivateHeaders, since this violates public/private + // API boundaries and can cause modular dependency cycles. + if (!IsIncluderPrivateHeader && IsIncludeeInFramework && + IsIncludeePrivateHeader && FromFramework == ToFramework) + Diags.Report(IncludeLoc, diag::warn_framework_include_private_from_public) + << IncludeFilename; } /// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file, diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h new file mode 100644 index 00000000000..03bd90b4945 --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h @@ -0,0 +1,4 @@ +#include <A/APriv.h> +#include "APriv2.h" +#include <Z/Z.h> +int foo(); diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap new file mode 100644 index 00000000000..09639b2b50b --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +// framework-public-includes-private/A.framework/Modules/module.modulemap +framework module A { + header "A.h" +} diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap new file mode 100644 index 00000000000..8ede0b0d6ee --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap @@ -0,0 +1,4 @@ +// framework-public-includes-private/A.framework/Modules/module.private.modulemap +framework module A_Private { + header "APriv.h" +} diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h new file mode 100644 index 00000000000..34cc847512f --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h @@ -0,0 +1 @@ +// framework-public-includes-private/A.framework/PrivateHeaders/APriv.h diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h new file mode 100644 index 00000000000..8ffeb418374 --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h @@ -0,0 +1 @@ +// framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/a.hmap.json b/clang/test/Modules/Inputs/framework-public-includes-private/a.hmap.json new file mode 100644 index 00000000000..42aed4551c1 --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/a.hmap.json @@ -0,0 +1,8 @@ + +{ + "mappings" : + { + "A.h" : "A/A.h", + "APriv2.h" : "A/APriv2.h" + } +} diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h new file mode 100644 index 00000000000..157c6daa76e --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h @@ -0,0 +1,2 @@ +// framework-public-includes-private/flat-header-path/Z.h +#import "ZPriv.h" diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap new file mode 100644 index 00000000000..0a1a9710c4f --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap @@ -0,0 +1,4 @@ +// framework-public-includes-private/flat-header-path/Z.modulemap +framework module Z { + header "Z.h" +} diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap new file mode 100644 index 00000000000..f409af8a15a --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap @@ -0,0 +1,4 @@ +// framework-public-includes-private/flat-header-path/Z.private.modulemap +framework module Z_Private { + header "ZPriv.h" +} diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h new file mode 100644 index 00000000000..08532fedbfa --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h @@ -0,0 +1 @@ +// framework-public-includes-private/flat-header-path/ZPriv.h diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/z.hmap.json b/clang/test/Modules/Inputs/framework-public-includes-private/z.hmap.json new file mode 100644 index 00000000000..206b25ec738 --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/z.hmap.json @@ -0,0 +1,7 @@ + +{ + "mappings" : + { + "ZPriv.h" : "Z/ZPriv.h" + } +} diff --git a/clang/test/Modules/Inputs/framework-public-includes-private/z.yaml b/clang/test/Modules/Inputs/framework-public-includes-private/z.yaml new file mode 100644 index 00000000000..5793c1ff1f1 --- /dev/null +++ b/clang/test/Modules/Inputs/framework-public-includes-private/z.yaml @@ -0,0 +1,45 @@ +{ + 'version': 0, + 'case-sensitive': 'false', + 'use-external-names' : 'false', + 'roots': [ + { + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Headers", + 'contents': [ + { + 'type': 'file', + 'name': "Z.h", + 'external-contents': "TEST_DIR/flat-header-path/Z.h" + } + ] + }, + { + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/PrivateHeaders", + 'contents': [ + { + 'type': 'file', + 'name': "ZPriv.h", + 'external-contents': "TEST_DIR/flat-header-path/ZPriv.h" + } + ] + }, + { + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Modules", + 'contents': [ + { + 'type': 'file', + 'name': "module.modulemap", + 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap" + }, + { + 'type': 'file', + 'name': "module.private.modulemap", + 'external-contents': "TEST_DIR/flat-header-path/Z.private.modulemap" + } + ] + } + ] +} diff --git a/clang/test/Modules/framework-public-includes-private.m b/clang/test/Modules/framework-public-includes-private.m new file mode 100644 index 00000000000..e58492091e3 --- /dev/null +++ b/clang/test/Modules/framework-public-includes-private.m @@ -0,0 +1,37 @@ +// REQUIRES: shell + +// RUN: rm -rf %t +// RUN: mkdir %t + +// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap +// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap + +// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \ +// RUN: %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml + +// The output with and without modules should be the same, without modules first. +// RUN: %clang_cc1 \ +// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/framework-public-includes-private \ +// RUN: -fsyntax-only %s -verify + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \ +// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/framework-public-includes-private \ +// RUN: -fsyntax-only %s \ +// RUN: 2>%t/stderr + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s +// CHECK: public framework header includes private framework header 'A/APriv.h' +// CHECK: public framework header includes private framework header 'A/APriv2.h' +// CHECK: public framework header includes private framework header 'Z/ZPriv.h' + +#import "A.h" + +int bar() { return foo(); } + +// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:1{{public framework header includes private framework header 'A/APriv.h'}} +// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv2.h'}} +// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:2{{public framework header includes private framework header 'Z/ZPriv.h'}} |
