r335375 - Re-apply: Warning for framework headers using double quote includes
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 22 11:58:26 PDT 2018
Why not enable it by default, or put it in -Wall at least? We don't like
off-by-default warnings :-)
On Fri, Jun 22, 2018 at 2:09 PM Bruno Cardoso Lopes via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: bruno
> Date: Fri Jun 22 11:05:17 2018
> New Revision: 335375
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335375&view=rev
> Log:
> Re-apply: Warning for framework headers using double quote includes
>
> Introduce -Wquoted-include-in-framework-header, which should fire a warning
> whenever a quote include appears in a framework header and suggest a
> fix-it.
> For instance, for header A.h added in the tests, this is how the warning
> looks
> like:
>
> ./A.framework/Headers/A.h:2:10: warning: double-quoted include "A0.h" in
> framework header, expected angle-bracketed instead
> [-Wquoted-include-in-framework-header]
> #include "A0.h"
> ^~~~~~
> <A/A0.h>
> ./A.framework/Headers/A.h:3:10: warning: double-quoted include "B.h" in
> framework header, expected angle-bracketed instead
> [-Wquoted-include-in-framework-header]
> #include "B.h"
> ^~~~~
> <B.h>
>
> This helps users to prevent frameworks from using local headers when in
> fact
> they should be targetting system level ones.
>
> The warning is off by default.
>
> Differential Revision: https://reviews.llvm.org/D47157
>
> rdar://problem/37077034
>
> Added:
> cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
> cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
>
> cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
> cfe/trunk/test/Modules/Inputs/double-quotes/B.h
> cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
>
> cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
> cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
> cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
>
> cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
> cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
> cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
> cfe/trunk/test/Modules/double-quotes.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=335375&r1=335374&r2=335375&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Jun 22 11:05:17
> 2018
> @@ -31,6 +31,7 @@ def AutoDisableVptrSanitizer : DiagGroup
> def Availability : DiagGroup<"availability">;
> def Section : DiagGroup<"section">;
> def AutoImport : DiagGroup<"auto-import">;
> +def FrameworkHdrQuotedInclude :
> DiagGroup<"quoted-include-in-framework-header">;
> 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=335375&r1=335374&r2=335375&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Fri Jun 22
> 11:05:17 2018
> @@ -714,6 +714,11 @@ def warn_mmap_redundant_export_as : Warn
> def err_mmap_submodule_export_as : Error<
> "only top-level modules can be re-exported as public">;
>
> +def warn_quoted_include_in_framework_header : Warning<
> + "double-quoted include \"%0\" in framework header, "
> + "expected angle-bracketed instead"
> + >, InGroup<FrameworkHdrQuotedInclude>, DefaultIgnore;
> +
> def warn_auto_module_import : Warning<
> "treating #%select{include|import|include_next|__include_macros}0 as an
> "
> "import of module '%1'">, InGroup<AutoImport>, DefaultIgnore;
>
> Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=335375&r1=335374&r2=335375&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
> +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Jun 22 11:05:17 2018
> @@ -621,6 +621,59 @@ static const char *copyString(StringRef
> return CopyStr;
> }
>
> +static bool isFrameworkStylePath(StringRef Path,
> + SmallVectorImpl<char> &FrameworkName) {
> + using namespace llvm::sys;
> + path::const_iterator I = path::begin(Path);
> + path::const_iterator E = path::end(Path);
> +
> + // Detect different types of framework style paths:
> + //
> + // ...Foo.framework/{Headers,PrivateHeaders}
> + // ...Foo.framework/Versions/{A,Current}/{Headers,PrivateHeaders}
> + //
> ...Foo.framework/Frameworks/Nested.framework/{Headers,PrivateHeaders}
> + // ...<other variations with 'Versions' like in the above path>
> + //
> + // and some other variations among these lines.
> + int FoundComp = 0;
> + while (I != E) {
> + if (I->endswith(".framework")) {
> + FrameworkName.append(I->begin(), I->end());
> + ++FoundComp;
> + }
> + if (*I == "Headers" || *I == "PrivateHeaders")
> + ++FoundComp;
> + ++I;
> + }
> +
> + return FoundComp >= 2;
> +}
> +
> +static void
> +diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation
> IncludeLoc,
> + StringRef Includer, StringRef IncludeFilename,
> + const FileEntry *IncludeFE, bool isAngled =
> false,
> + bool FoundByHeaderMap = false) {
> + SmallString<128> FromFramework, ToFramework;
> + if (!isFrameworkStylePath(Includer, FromFramework))
> + return;
> + bool IsIncludeeInFramework =
> + isFrameworkStylePath(IncludeFE->getName(), ToFramework);
> +
> + if (!isAngled && !FoundByHeaderMap) {
> + SmallString<128> NewInclude("<");
> + if (IsIncludeeInFramework) {
> + NewInclude += StringRef(ToFramework).drop_back(10); // drop
> .framework
> + NewInclude += "/";
> + }
> + NewInclude += IncludeFilename;
> + NewInclude += ">";
> + Diags.Report(IncludeLoc,
> diag::warn_quoted_include_in_framework_header)
> + << IncludeFilename
> + << FixItHint::CreateReplacement(IncludeLoc, NewInclude);
> + }
> +}
> +
> /// LookupFile - Given a "foo" or \<foo> 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
> @@ -722,8 +775,12 @@ const FileEntry *HeaderSearch::LookupFil
> RelativePath->clear();
> RelativePath->append(Filename.begin(), Filename.end());
> }
> - if (First)
> + if (First) {
> + diagnoseFrameworkInclude(Diags, IncludeLoc,
> + IncluderAndDir.second->getName(),
> Filename,
> + FE);
> return FE;
> + }
>
> // Otherwise, we found the path via MSVC header search rules. If
> // -Wmsvc-include is enabled, we have to keep searching to see if
> we
> @@ -834,6 +891,12 @@ const FileEntry *HeaderSearch::LookupFil
> return MSFE;
> }
>
> + bool FoundByHeaderMap = !IsMapped ? false : *IsMapped;
> + if (!Includers.empty())
> + diagnoseFrameworkInclude(Diags, IncludeLoc,
> + Includers.front().second->getName(),
> Filename,
> + FE, isAngled, FoundByHeaderMap);
> +
> // Remember this location for the next lookup we do.
> CacheLookup.HitIdx = i;
> return FE;
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
> (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
> Fri Jun 22 11:05:17 2018
> @@ -0,0 +1,6 @@
> +#include "A0.h"
> +#include "B.h"
> +
> +#include "X.h"
> +
> +int foo();
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
> (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h
> Fri Jun 22 11:05:17 2018
> @@ -0,0 +1 @@
> +// double-quotes/A.framework/Headers/A0.h
>
> Added:
> cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap?rev=335375&view=auto
>
> ==============================================================================
> ---
> cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
> (added)
> +++
> cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
> Fri Jun 22 11:05:17 2018
> @@ -0,0 +1,5 @@
> +// double-quotes/A.framework/Modules/module.modulemap
> +framework module A {
> + header "A.h"
> + header "A0.h"
> +}
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/B.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/B.h?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/B.h (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/B.h Fri Jun 22 11:05:17
> 2018
> @@ -0,0 +1 @@
> +// double-quotes/B.h
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
> (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h
> Fri Jun 22 11:05:17 2018
> @@ -0,0 +1 @@
> +// double-quotes/X.framework/Headers/X.h
>
> Added:
> cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap?rev=335375&view=auto
>
> ==============================================================================
> ---
> cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
> (added)
> +++
> cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
> Fri Jun 22 11:05:17 2018
> @@ -0,0 +1,4 @@
> +// double-quotes/X.framework/Modules/module.modulemap
> +framework module X {
> + header "X.h"
> +}
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json Fri Jun 22
> 11:05:17 2018
> @@ -0,0 +1,6 @@
> +{
> + "mappings" :
> + {
> + "A.h" : "A/A.h"
> + }
> +}
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
> (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h Fri
> Jun 22 11:05:17 2018
> @@ -0,0 +1 @@
> +#import "B.h" // Included from Z.h & A.h
>
> Added:
> cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap?rev=335375&view=auto
>
> ==============================================================================
> ---
> cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
> (added)
> +++
> cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
> Fri Jun 22 11:05:17 2018
> @@ -0,0 +1,4 @@
> +// double-quotes/flat-header-path/Z.modulemap
> +framework module Z {
> + header "Z.h"
> +}
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json Fri Jun 22
> 11:05:17 2018
> @@ -0,0 +1,7 @@
> +
> +{
> + "mappings" :
> + {
> + "X.h" : "X/X.h"
> + }
> +}
>
> Added: cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml (added)
> +++ cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml Fri Jun 22 11:05:17
> 2018
> @@ -0,0 +1,28 @@
> +{
> + 'version': 0,
> + 'case-sensitive': '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/Modules",
> + 'contents': [
> + {
> + 'type': 'file',
> + 'name': "module.modulemap",
> + 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
> + }
> + ]
> + }
> + ]
> +}
>
> Added: cfe/trunk/test/Modules/double-quotes.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/double-quotes.m?rev=335375&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/double-quotes.m (added)
> +++ cfe/trunk/test/Modules/double-quotes.m Fri Jun 22 11:05:17 2018
> @@ -0,0 +1,39 @@
> +// REQUIRES: shell
> +
> +// RUN: rm -rf %t
> +// RUN: mkdir %t
> +
> +// RUN: %hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap
> +// RUN: %hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap
> +
> +// RUN: sed -e "s:TEST_DIR:%S/Inputs/double-quotes:g" \
> +// RUN: %S/Inputs/double-quotes/z.yaml > %t/z.yaml
> +
> +// The output with and without modules should be the same
> +
> +// RUN: %clang_cc1 \
> +// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
> +// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
> +// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s -verify
> +
> +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps
> -fmodules-cache-path=%t/cache \
> +// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
> +// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \
> +// RUN: -Wquoted-include-in-framework-header -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: double-quoted include "A0.h" in framework header, expected
> angle-bracketed instead
> +// CHECK: double-quoted include "B.h" in framework header, expected
> angle-bracketed instead
> +// CHECK: double-quoted include "B.h" in framework header, expected
> angle-bracketed instead
> +
> +#import "A.h"
> +#import <Z/Z.h>
> +
> +int bar() { return foo(); }
> +
> +// expected-warning at Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted
> include "A0.h" in framework header, expected angle-bracketed instead}}
> +// expected-warning at Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted
> include "B.h" in framework header, expected angle-bracketed instead}}
> +// expected-warning at Inputs/double-quotes/flat-header-path/Z.h:1{{double-quoted
> include "B.h" in framework header, expected angle-bracketed instead}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180622/83273113/attachment-0001.html>
More information about the cfe-commits
mailing list