Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -32,6 +32,8 @@ 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">; Index: include/clang/Basic/DiagnosticLexKinds.td =================================================================== --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -718,6 +718,9 @@ "double-quoted include \"%0\" in framework header, " "expected angle-bracketed instead" >, InGroup, DefaultIgnore; +def warn_framework_include_private_from_public : Warning< + "public framework header includes private framework header '%0'" + >, InGroup; def warn_auto_module_import : Warning< "treating #%select{include|import|include_next|__include_macros}0 as an " Index: lib/Lex/HeaderSearch.cpp =================================================================== --- lib/Lex/HeaderSearch.cpp +++ lib/Lex/HeaderSearch.cpp @@ -621,11 +621,12 @@ return CopyStr; } -static bool isFrameworkStylePath(StringRef Path, +static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader, SmallVectorImpl &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 @@ // 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 @@ 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 @@ << 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 \ reference, look up the indicated file, Index: test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h @@ -0,0 +1,5 @@ + +#include +#include "APriv2.h" +#include +int foo(); Index: test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ + +framework module A { + header "A.h" +} Index: test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap @@ -0,0 +1,3 @@ +framework module A_Private { + header "APriv.h" +} Index: test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h @@ -0,0 +1 @@ +// APriv.h Index: test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h @@ -0,0 +1 @@ +// APriv.h Index: test/Modules/Inputs/framework-public-includes-private/a.hmap.json =================================================================== --- /dev/null +++ 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" + } +} Index: test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h @@ -0,0 +1 @@ +#import "ZPriv.h" Index: test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap @@ -0,0 +1,3 @@ +framework module Z { + header "Z.h" +} Index: test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap @@ -0,0 +1,3 @@ +framework module Z_Private { + header "ZPriv.h" +} Index: test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h @@ -0,0 +1 @@ +// ZPriv.h Index: test/Modules/Inputs/framework-public-includes-private/z.hmap.json =================================================================== --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/z.hmap.json @@ -0,0 +1,7 @@ + +{ + "mappings" : + { + "ZPriv.h" : "Z/ZPriv.h" + } +} Index: test/Modules/Inputs/framework-public-includes-private/z.yaml =================================================================== --- /dev/null +++ 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" + } + ] + } + ] +} Index: test/Modules/framework-public-includes-private.m =================================================================== --- /dev/null +++ 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:2{{public framework header includes private framework header 'A/APriv.h'}} +// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}} +// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}