Author: bruno
Date: Mon Jun 25 15:24:17 2018
New Revision: 335542
URL: http://llvm.org/viewvc/llvm-project?rev=335542&view=rev
Log:
Warning for framework include violation from Headers to PrivateHeaders
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
<Foo/some-header.h>, 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:
./A.framework/Headers/A.h:1:10: warning: public framework header includes
private framework header 'A/APriv.h'
#include <A/APriv.h>
^
rdar://problem/38712182
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/a.hmap.json
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.hmap.json
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.yaml
cfe/trunk/test/Modules/framework-public-includes-private.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/HeaderSearch.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=335542&r1=335541&r2=335542&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jun 25 15:24:17 2018
@@ -32,6 +32,8 @@ def Availability : DiagGroup<"availabili
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">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=335542&r1=335541&r2=335542&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Mon Jun 25 15:24:17 2018
@@ -718,6 +718,9 @@ def warn_quoted_include_in_framework_hea
"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 "
Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=335542&r1=335541&r2=335542&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Mon Jun 25 15:24:17 2018
@@ -621,11 +621,12 @@ static const char *copyString(StringRef
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(StringR
// 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(DiagnosticsEngi
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(DiagnosticsEngi
<< 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,
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,4 @@
+#include <A/APriv.h>
+#include "APriv2.h"
+#include <Z/Z.h>
+int foo();
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,4 @@
+// framework-public-includes-private/A.framework/Modules/module.modulemap
+framework module A {
+ header "A.h"
+}
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,4 @@
+//
framework-public-includes-private/A.framework/Modules/module.private.modulemap
+framework module A_Private {
+ header "APriv.h"
+}
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
Mon Jun 25 15:24:17 2018
@@ -0,0 +1 @@
+// framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
Mon Jun 25 15:24:17 2018
@@ -0,0 +1 @@
+// framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/a.hmap.json
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/a.hmap.json?rev=335542&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/framework-public-includes-private/a.hmap.json
(added)
+++ cfe/trunk/test/Modules/Inputs/framework-public-includes-private/a.hmap.json
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,8 @@
+
+{
+ "mappings" :
+ {
+ "A.h" : "A/A.h",
+ "APriv2.h" : "A/APriv2.h"
+ }
+}
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,2 @@
+// framework-public-includes-private/flat-header-path/Z.h
+#import "ZPriv.h"
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,4 @@
+// framework-public-includes-private/flat-header-path/Z.modulemap
+framework module Z {
+ header "Z.h"
+}
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,4 @@
+// framework-public-includes-private/flat-header-path/Z.private.modulemap
+framework module Z_Private {
+ header "ZPriv.h"
+}
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h?rev=335542&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
(added)
+++
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
Mon Jun 25 15:24:17 2018
@@ -0,0 +1 @@
+// framework-public-includes-private/flat-header-path/ZPriv.h
Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.hmap.json
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.hmap.json?rev=335542&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.hmap.json
(added)
+++ cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.hmap.json
Mon Jun 25 15:24:17 2018
@@ -0,0 +1,7 @@
+
+{
+ "mappings" :
+ {
+ "ZPriv.h" : "Z/ZPriv.h"
+ }
+}
Added: cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.yaml
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.yaml?rev=335542&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.yaml
(added)
+++ cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.yaml Mon
Jun 25 15:24:17 2018
@@ -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"
+ }
+ ]
+ }
+ ]
+}
Added: cfe/trunk/test/Modules/framework-public-includes-private.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/framework-public-includes-private.m?rev=335542&view=auto
==============================================================================
--- cfe/trunk/test/Modules/framework-public-includes-private.m (added)
+++ cfe/trunk/test/Modules/framework-public-includes-private.m Mon Jun 25
15:24:17 2018
@@ -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'}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
