r335393 - [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin
JF Bastien via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 22 14:54:40 PDT 2018
Author: jfb
Date: Fri Jun 22 14:54:40 2018
New Revision: 335393
URL: http://llvm.org/viewvc/llvm-project?rev=335393&view=rev
Log:
[Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin
Summary:
Pick D42933 back up, and make NSInteger/NSUInteger with %zu/%zi specifiers on Darwin warn only in pedantic mode. The default -Wformat recently started warning for the following code because of the added support for analysis for the '%zi' specifier.
NSInteger i = NSIntegerMax;
NSLog(@"max NSInteger = %zi", i);
The problem is that on armv7 %zi is 'long', and NSInteger is typedefed to 'int' in Foundation. We should avoid this warning as it's inconvenient to our users: it's target specific (happens only on armv7 and not arm64), and breaks their existing code. We should also silence the warning for the '%zu' specifier to ensure consistency. This is acceptable because Darwin guarantees that, despite the unfortunate choice of typedef, sizeof(size_t) == sizeof(NS[U]Integer), the warning is therefore noisy for pedantic reasons. Once this is in I'll update public documentation.
Related discussion on cfe-dev:
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058050.html
<rdar://36874921&40501559>
Reviewers: ahatanak, vsapsai, alexshap, aaron.ballman, javed.absar, jfb, rjmccall
Subscribers: kristof.beyls, aheejin, cfe-commits
Differential Revision: https://reviews.llvm.org/D47290
Added:
cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m
cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m
Modified:
cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Analysis/PrintfFormatString.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/FixIt/fixit-format-ios.m
Modified: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=335393&r1=335392&r2=335393&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Fri Jun 22 14:54:40 2018
@@ -256,18 +256,19 @@ public:
private:
const Kind K;
QualType T;
- const char *Name;
- bool Ptr;
+ const char *Name = nullptr;
+ bool Ptr = false, IsSizeT = false;
+
public:
- ArgType(Kind k = UnknownTy, const char *n = nullptr)
- : K(k), Name(n), Ptr(false) {}
- ArgType(QualType t, const char *n = nullptr)
- : K(SpecificTy), T(t), Name(n), Ptr(false) {}
- ArgType(CanQualType t) : K(SpecificTy), T(t), Name(nullptr), Ptr(false) {}
+ ArgType(Kind K = UnknownTy, const char *N = nullptr) : K(K), Name(N) {}
+ ArgType(QualType T, const char *N = nullptr) : K(SpecificTy), T(T), Name(N) {}
+ ArgType(CanQualType T) : K(SpecificTy), T(T) {}
static ArgType Invalid() { return ArgType(InvalidTy); }
bool isValid() const { return K != InvalidTy; }
+ bool isSizeT() const { return IsSizeT; }
+
/// Create an ArgType which corresponds to the type pointer to A.
static ArgType PtrTo(const ArgType& A) {
assert(A.K >= InvalidTy && "ArgType cannot be pointer to invalid/unknown");
@@ -276,6 +277,13 @@ public:
return Res;
}
+ /// Create an ArgType which corresponds to the size_t/ssize_t type.
+ static ArgType makeSizeT(const ArgType &A) {
+ ArgType Res = A;
+ Res.IsSizeT = true;
+ return Res;
+ }
+
MatchKind matchesType(ASTContext &C, QualType argTy) const;
QualType getRepresentativeType(ASTContext &C) const;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=335393&r1=335392&r2=335393&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 22 14:54:40 2018
@@ -7719,6 +7719,10 @@ def warn_format_argument_needs_cast : Wa
"%select{values of type|enum values with underlying type}2 '%0' should not "
"be used as format arguments; add an explicit cast to %1 instead">,
InGroup<Format>;
+def warn_format_argument_needs_cast_pedantic : Warning<
+ "%select{values of type|enum values with underlying type}2 '%0' should not "
+ "be used as format arguments; add an explicit cast to %1 instead">,
+ InGroup<FormatPedantic>, DefaultIgnore;
def warn_printf_positional_arg_exceeds_data_args : Warning <
"data argument position '%0' exceeds the number of data arguments (%1)">,
InGroup<Format>;
Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=335393&r1=335392&r2=335393&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Fri Jun 22 14:54:40 2018
@@ -466,7 +466,7 @@ ArgType PrintfSpecifier::getArgType(ASTC
case LengthModifier::AsIntMax:
return ArgType(Ctx.getIntMaxType(), "intmax_t");
case LengthModifier::AsSizeT:
- return ArgType(Ctx.getSignedSizeType(), "ssize_t");
+ return ArgType::makeSizeT(ArgType(Ctx.getSignedSizeType(), "ssize_t"));
case LengthModifier::AsInt3264:
return Ctx.getTargetInfo().getTriple().isArch64Bit()
? ArgType(Ctx.LongLongTy, "__int64")
@@ -499,7 +499,7 @@ ArgType PrintfSpecifier::getArgType(ASTC
case LengthModifier::AsIntMax:
return ArgType(Ctx.getUIntMaxType(), "uintmax_t");
case LengthModifier::AsSizeT:
- return ArgType(Ctx.getSizeType(), "size_t");
+ return ArgType::makeSizeT(ArgType(Ctx.getSizeType(), "size_t"));
case LengthModifier::AsInt3264:
return Ctx.getTargetInfo().getTriple().isArch64Bit()
? ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64")
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=335393&r1=335392&r2=335393&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jun 22 14:54:40 2018
@@ -6805,11 +6805,11 @@ CheckPrintfHandler::checkFormatExpr(cons
ExprTy = TET->getUnderlyingExpr()->getType();
}
- analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
- if (match == analyze_printf::ArgType::Match) {
+ const analyze_printf::ArgType::MatchKind Match =
+ AT.matchesType(S.Context, ExprTy);
+ bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
+ if (Match == analyze_printf::ArgType::Match)
return true;
- }
// Look through argument promotions for our error message's reported type.
// This includes the integral and floating promotions, but excludes array
@@ -6885,6 +6885,11 @@ CheckPrintfHandler::checkFormatExpr(cons
QualType CastTy;
std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E);
if (!CastTy.isNull()) {
+ // %zi/%zu are OK to use for NSInteger/NSUInteger of type int
+ // (long in ASTContext). Only complain to pedants.
+ if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+ AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+ Pedantic = true;
IntendedTy = CastTy;
ShouldNotPrintDirectly = true;
}
@@ -6892,10 +6897,10 @@ CheckPrintfHandler::checkFormatExpr(cons
// We may be able to offer a FixItHint if it is a supported type.
PrintfSpecifier fixedFS = FS;
- bool success =
+ bool Success =
fixedFS.fixType(IntendedTy, S.getLangOpts(), S.Context, isObjCContext());
- if (success) {
+ if (Success) {
// Get the fix string from the fixed format specifier
SmallString<16> buf;
llvm::raw_svector_ostream os(buf);
@@ -6904,13 +6909,13 @@ CheckPrintfHandler::checkFormatExpr(cons
CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
- unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
- if (match == analyze_format_string::ArgType::NoMatchPedantic) {
- diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
- }
+ unsigned Diag =
+ Pedantic
+ ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+ : diag::warn_format_conversion_argument_type_mismatch;
// In this case, the specifier is wrong and should be changed to match
// the argument.
- EmitFormatDiagnostic(S.PDiag(diag)
+ EmitFormatDiagnostic(S.PDiag(Diag)
<< AT.getRepresentativeTypeName(S.Context)
<< IntendedTy << IsEnum << E->getSourceRange(),
E->getLocStart(),
@@ -6963,9 +6968,11 @@ CheckPrintfHandler::checkFormatExpr(cons
Name = TypedefTy->getDecl()->getName();
else
Name = CastTyName;
- EmitFormatDiagnostic(S.PDiag(diag::warn_format_argument_needs_cast)
- << Name << IntendedTy << IsEnum
- << E->getSourceRange(),
+ unsigned Diag = Pedantic
+ ? diag::warn_format_argument_needs_cast_pedantic
+ : diag::warn_format_argument_needs_cast;
+ EmitFormatDiagnostic(S.PDiag(Diag) << Name << IntendedTy << IsEnum
+ << E->getSourceRange(),
E->getLocStart(), /*IsStringLocation=*/false,
SpecRange, Hints);
} else {
@@ -6989,13 +6996,13 @@ CheckPrintfHandler::checkFormatExpr(cons
switch (S.isValidVarArgType(ExprTy)) {
case Sema::VAK_Valid:
case Sema::VAK_ValidInCXX11: {
- unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
- if (match == analyze_printf::ArgType::NoMatchPedantic) {
- diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
- }
+ unsigned Diag =
+ Pedantic
+ ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+ : diag::warn_format_conversion_argument_type_mismatch;
EmitFormatDiagnostic(
- S.PDiag(diag) << AT.getRepresentativeTypeName(S.Context) << ExprTy
+ S.PDiag(Diag) << AT.getRepresentativeTypeName(S.Context) << ExprTy
<< IsEnum << CSR << E->getSourceRange(),
E->getLocStart(), /*IsStringLocation*/ false, CSR);
break;
@@ -7178,29 +7185,28 @@ bool CheckScanfHandler::HandleScanfSpeci
return true;
}
- analyze_format_string::ArgType::MatchKind match =
+ analyze_format_string::ArgType::MatchKind Match =
AT.matchesType(S.Context, Ex->getType());
- if (match == analyze_format_string::ArgType::Match) {
+ bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
+ if (Match == analyze_format_string::ArgType::Match)
return true;
- }
ScanfSpecifier fixedFS = FS;
- bool success = fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
+ bool Success = fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
S.getLangOpts(), S.Context);
- unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
- if (match == analyze_format_string::ArgType::NoMatchPedantic) {
- diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
- }
+ unsigned Diag =
+ Pedantic ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+ : diag::warn_format_conversion_argument_type_mismatch;
- if (success) {
+ if (Success) {
// Get the fix string from the fixed format specifier.
SmallString<128> buf;
llvm::raw_svector_ostream os(buf);
fixedFS.toString(os);
EmitFormatDiagnostic(
- S.PDiag(diag) << AT.getRepresentativeTypeName(S.Context)
+ S.PDiag(Diag) << AT.getRepresentativeTypeName(S.Context)
<< Ex->getType() << false << Ex->getSourceRange(),
Ex->getLocStart(),
/*IsStringLocation*/ false,
@@ -7208,7 +7214,7 @@ bool CheckScanfHandler::HandleScanfSpeci
FixItHint::CreateReplacement(
getSpecifierRange(startSpecifier, specifierLen), os.str()));
} else {
- EmitFormatDiagnostic(S.PDiag(diag)
+ EmitFormatDiagnostic(S.PDiag(Diag)
<< AT.getRepresentativeTypeName(S.Context)
<< Ex->getType() << false << Ex->getSourceRange(),
Ex->getLocStart(),
Added: cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m?rev=335393&view=auto
==============================================================================
--- cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m (added)
+++ cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m Fri Jun 22 14:54:40 2018
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+ // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+ // and the underlying type of size_t is unsigned long.
+
+ printf("test 1: %zu", getNSUInteger());
+
+ printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+ printf("test 3: %zd", getNSInteger());
+
+ printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Modified: cfe/trunk/test/FixIt/fixit-format-ios.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-format-ios.m?rev=335393&r1=335392&r2=335393&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/fixit-format-ios.m (original)
+++ cfe/trunk/test/FixIt/fixit-format-ios.m Fri Jun 22 14:54:40 2018
@@ -1,5 +1,5 @@
// RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
// RUN: grep -v CHECK %t | FileCheck %s
int printf(const char * restrict, ...);
Added: cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m?rev=335393&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m (added)
+++ cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m Fri Jun 22 14:54:40 2018
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if !defined(PEDANTIC)
+// expected-no-diagnostics
+#endif
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+ at class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+ NSInteger i = 0;
+ NSUInteger j = 0;
+ NSLog(@"max NSInteger = %zi", i);
+ NSLog(@"max NSUinteger = %zu", j);
+
+#if defined(PEDANTIC)
+ // expected-warning at -4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ // expected-warning at -4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+#endif
+}
More information about the cfe-commits
mailing list