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@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@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted > include "A0.h" in framework header, expected angle-bracketed instead}} > +// > expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted > include "B.h" in framework header, expected angle-bracketed instead}} > +// > expected-warning@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
