[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno closed this revision.
bruno added a comment.

Committed r335542


https://reviews.llvm.org/D47301



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai accepted this revision.
vsapsai added a comment.
This revision is now accepted and ready to land.

Looks good to me. The only problem is I'm not entirely sure this warning will 
interact with real code the way I expect it to. We'll need to keep an eye on it 
and tweak if necessary.


https://reviews.llvm.org/D47301



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Ping!


https://reviews.llvm.org/D47301



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 152198.
bruno added a comment.

Address Volodymyr's comments


https://reviews.llvm.org/D47301

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
  test/Modules/Inputs/framework-public-includes-private/a.hmap.json
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
  test/Modules/Inputs/framework-public-includes-private/z.hmap.json
  test/Modules/Inputs/framework-public-includes-private/z.yaml
  test/Modules/framework-public-includes-private.m

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'}}
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/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.hmap.json
@@ -0,0 +1,7 @@
+
+{
+  

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: lib/Lex/HeaderSearch.cpp:683
+  // from Foo.framework/PrivateHeaders, since this violates public/private
+  // api boundaries and can cause modular dependency cycles.
+  if (!IsIncluderPrivateHeader && IsIncludeeInFramework &&

Please capitalize "api". The rest looks good.


https://reviews.llvm.org/D47301



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno marked an inline comment as done.
bruno added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:34-35
 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">;

vsapsai wrote:
> It might be convenient for users to have a warning group that will cover 
> different framework warnings, something like -Wframework-hygiene. But it's 
> out of scope for this review, more as an idea for future improvements.
Yep, that should come once we land all other bits.



Comment at: lib/Lex/HeaderSearch.cpp:625
+static bool isFrameworkStylePath(StringRef Path, bool ,
+ SmallVectorImpl ) {
   using namespace llvm::sys;

vsapsai wrote:
> In this function we accept some paths that aren't valid framework paths. Need 
> to think more if it is OK or if we want to be stricter.
It should be ok at this point, otherwise the framework style path would have 
failed before finding this header.



Comment at: test/Modules/framework-public-includes-private.m:33
+
+int bar() { return foo(); }
+

vsapsai wrote:
> I'm not entirely sure it's not covered by existing test. It might be worth 
> testing private header including public header and private header including 
> another private header.
The warning is on by default and clang already have the scenario you described 
in other module tests - those would fail if there's a bug in the logic here.


https://reviews.llvm.org/D47301



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 149361.
bruno added a comment.

Update patch after changes to https://reviews.llvm.org/D47157. Also address 
some of Volodymyr feedback.


https://reviews.llvm.org/D47301

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
  test/Modules/Inputs/framework-public-includes-private/a.hmap.json
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
  test/Modules/Inputs/framework-public-includes-private/z.hmap.json
  test/Modules/Inputs/framework-public-includes-private/z.yaml
  test/Modules/framework-public-includes-private.m

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'}}
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/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ 

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:34-35
 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">;

It might be convenient for users to have a warning group that will cover 
different framework warnings, something like -Wframework-hygiene. But it's out 
of scope for this review, more as an idea for future improvements.



Comment at: lib/Lex/HeaderSearch.cpp:625
+static bool isFrameworkStylePath(StringRef Path, bool ,
+ SmallVectorImpl ) {
   using namespace llvm::sys;

In this function we accept some paths that aren't valid framework paths. Need 
to think more if it is OK or if we want to be stricter.



Comment at: lib/Lex/HeaderSearch.cpp:893
+  // from Foo.framework/PrivateHeaders, since this violates public/private
+  // api boundaries and can cause modular dependency cycles.
+  SmallString<128> ToFramework;

s/api/API/



Comment at: lib/Lex/HeaderSearch.cpp:895
+  SmallString<128> ToFramework;
+  bool IncludeIsFrameworkPrivateHeader = false;
+  if (IsIncluderFromFramework && !IsFrameworkPrivateHeader &&

My opinion on readability is personal, so take it with a grain of salt. What if 
you make variable names more consistent, like
* IsIncluderPrivateHeader
* IsIncluderFromFramework
* IsIncludeePrivateHeader



Comment at: test/Modules/framework-public-includes-private.m:33
+
+int bar() { return foo(); }
+

I'm not entirely sure it's not covered by existing test. It might be worth 
testing private header including public header and private header including 
another private header.


https://reviews.llvm.org/D47301



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 148317.
bruno added a comment.

Update to a more recent version of the patch.


https://reviews.llvm.org/D47301

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
  test/Modules/Inputs/framework-public-includes-private/a.hmap.json
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
  test/Modules/Inputs/framework-public-includes-private/z.hmap.json
  test/Modules/Inputs/framework-public-includes-private/z.yaml
  test/Modules/framework-public-includes-private.m

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'}}
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/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.hmap.json
@@ -0,0 +1,7 

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
bruno added reviewers: ributzka, vsapsai, dexonsmith.

Framework vendors usually layout their framework headers in the following way:

  Foo.framework/Headers -> "public" headers
  Foo.framework/PrivateHeader -> "private" headers

Since both headers in both directories can be found with #import 
, it's easy to make mistakes and include headers in 
Foo.framework/PrivateHeader from headers in Foo.framework/Headers, which 
usually configures a layering violation on Darwin ecosystems. One of the 
problem this causes is dep cycles when modules are used, since it's very common 
for "private" modules to include from the "public" ones; adding an edge the 
other way around will trigger cycles.

Add a warning to catch those cases such that:

In file included from test.m:1:
./A.framework/Headers/A.h:2:10: warning: public framework header includes 
private framework header 'A/APriv.h' [-Wframework-include-private-from-public]

  ^

This only works for the layering violation within a framework.

rdar://problem/38712182

Depends on https://reviews.llvm.org/D47157


Repository:
  rC Clang

https://reviews.llvm.org/D47301

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
  test/Modules/Inputs/framework-public-includes-private/a.hmap.json
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
  test/Modules/Inputs/framework-public-includes-private/z.hmap.json
  test/Modules/Inputs/framework-public-includes-private/z.yaml
  test/Modules/framework-public-includes-private.m

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'}}
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"