Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -32,6 +32,7 @@ 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 @@ -712,6 +712,9 @@ def warn_quoted_include_in_framework_header : Warning< "double-quoted include \"%0\" in framework header, expected system style include" >, 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,10 +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: // @@ -636,16 +638,28 @@ // and some other variations among these lines. int FoundComp = 0; while (I != E) { - if (I->endswith(".framework")) + 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; } return FoundComp >= 2; } +static bool isFrameworkStylePath(StringRef Path) { + bool IsPrivateHdr = false; + SmallString<128> FrameworkName; + return isFrameworkStylePath(Path, IsPrivateHdr, FrameworkName); +} + /// LookupFile - Given a "foo" or \ reference, look up the indicated file, /// return null on failure. isAngled indicates whether the file reference is /// for system \#include's or not (i.e. using <> instead of ""). Includers, if @@ -864,12 +878,30 @@ return MSFE; } + SmallString<128> FromFramework; bool FoundByHeaderMap = !IsMapped ? false : *IsMapped; - if (!Includers.empty() && !isAngled && - isFrameworkStylePath(Includers.front().second->getName()) && - !FoundByHeaderMap) - Diags.Report(IncludeLoc, diag::warn_quoted_include_in_framework_header) - << Filename; + bool IsFrameworkPrivateHeader = false; + bool IsIncluderFromFramework = + isFrameworkStylePath(Includers.front().second->getName(), + IsFrameworkPrivateHeader, FromFramework); + if (!Includers.empty() && IsIncluderFromFramework) { + if (!isAngled && !FoundByHeaderMap) + Diags.Report(IncludeLoc, diag::warn_quoted_include_in_framework_header) + << Filename; + // 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. + SmallString<128> ToFramework; + bool IncludeIsFrameworkPrivateHeader = false; + if (!IsFrameworkPrivateHeader && + isFrameworkStylePath(FE->getName(), + IncludeIsFrameworkPrivateHeader, ToFramework) && + IncludeIsFrameworkPrivateHeader && FromFramework == ToFramework) { + Diags.Report(IncludeLoc, + diag::warn_framework_include_private_from_public) + << Filename; + } + } // Remember this location for the next lookup we do. CacheLookup.HitIdx = i; 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'}}