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