erik.pilkington updated this revision to Diff 223136.
erik.pilkington marked 6 inline comments as done.
erik.pilkington added a comment.
Address review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67775/new/
https://reviews.llvm.org/D67775
Files:
clang/include/clang/AST/FormatString.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/FormatString.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/format-bool.c
clang/test/Sema/format-strings-pedantic.c
clang/test/Sema/format-type-confusion.c
Index: clang/test/Sema/format-type-confusion.c
===================================================================
--- /dev/null
+++ clang/test/Sema/format-type-confusion.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9.0 -fsyntax-only -verify -Wno-format -Wformat-type-confusion %s
+
+__attribute__((format(__printf__, 1, 2)))
+int printf(const char *msg, ...);
+
+#define FMT "%hd %hu %d %u %hhd %hhu %c"
+
+int main() {
+ _Bool b = 0;
+ printf(FMT,
+ b, // expected-warning {{format specifies type 'short' but the argument has type '_Bool'}}
+ b, // expected-warning {{format specifies type 'unsigned short' but the argument has type '_Bool'}}
+ b, b, b, b, b);
+
+ unsigned char uc = 0;
+ printf(FMT,
+ uc, // expected-warning {{format specifies type 'short' but the argument has type 'unsigned char'}}
+ uc, // expected-warning {{format specifies type 'unsigned short' but the argument has type 'unsigned char'}}
+ uc, uc, uc, uc, uc);
+
+ signed char sc = 0;
+ printf(FMT,
+ sc, // expected-warning {{format specifies type 'short' but the argument has type 'signed char'}}
+ sc, // expected-warning {{format specifies type 'unsigned short' but the argument has type 'signed char'}}
+ sc, sc, sc, sc, sc);
+}
Index: clang/test/Sema/format-strings-pedantic.c
===================================================================
--- clang/test/Sema/format-strings-pedantic.c
+++ clang/test/Sema/format-strings-pedantic.c
@@ -1,10 +1,20 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-pedantic -isystem %S/Inputs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
+// RUN: %clang_cc1 -xobjective-c -fblocks -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
+// RUN: %clang_cc1 -xc++ -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
+__attribute__((format(printf, 1, 2)))
int printf(const char *restrict, ...);
-typedef unsigned char uint8_t;
+int main() {
+ printf("%p", (int *)0); // expected-warning {{format specifies type 'void *' but the argument has type 'int *'}}
+ printf("%p", (void *)0);
-void print_char_as_short() {
- printf("%hu\n", (unsigned char)1); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'unsigned char'}}
- printf("%hu\n", (uint8_t)1); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'uint8_t' (aka 'unsigned char')}}
+#ifdef __OBJC__
+ printf("%p", ^{}); // expected-warning {{format specifies type 'void *' but the argument has type 'void (^)(void)'}}
+ printf("%p", (id)0); // expected-warning {{format specifies type 'void *' but the argument has type 'id'}}
+#endif
+
+#ifdef __cplusplus
+ printf("%p", nullptr); // expected-warning {{format specifies type 'void *' but the argument has type 'nullptr_t'}}
+#endif
}
Index: clang/test/Sema/format-bool.c
===================================================================
--- clang/test/Sema/format-bool.c
+++ clang/test/Sema/format-bool.c
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool
// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool
// RUN: %clang_cc1 -xobjective-c %s -verify -DBOOL=_Bool
-// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool -Wformat-pedantic -DPEDANTIC
-// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool -Wformat-pedantic -DPEDANTIC
+// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool -Wformat-type-confusion -DTYPE_CONF
+// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool -Wformat-type-confusion -DTYPE_CONF
__attribute__((format(__printf__, 1, 2)))
int p(const char *fmt, ...);
@@ -22,13 +22,13 @@
int main() {
p("%d", b);
p("%hd", b);
-#ifdef PEDANTIC
+#ifdef TYPE_CONF
// expected-warning@-2 {{format specifies type 'short' but the argument has type}}
#endif
p("%hhd", b);
p("%u", b);
p("%hu", b);
-#ifdef PEDANTIC
+#ifdef TYPE_CONF
// expected-warning@-2 {{format specifies type 'unsigned short' but the argument has type}}
#endif
p("%hhu", b);
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8172,9 +8172,7 @@
return true;
}
- const analyze_printf::ArgType::MatchKind Match =
- AT.matchesType(S.Context, ExprTy);
- bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
+ analyze_printf::ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
if (Match == analyze_printf::ArgType::Match)
return true;
@@ -8198,8 +8196,9 @@
AT.matchesType(S.Context, ExprTy);
if (ImplicitMatch == analyze_printf::ArgType::Match)
return true;
- if (ImplicitMatch == analyze_printf::ArgType::NoMatchPedantic)
- Pedantic = true;
+ if (ImplicitMatch == ArgType::NoMatchPedantic ||
+ ImplicitMatch == ArgType::NoMatchTypeConfusion)
+ Match = ImplicitMatch;
}
}
} else if (const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E)) {
@@ -8261,7 +8260,7 @@
if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
(AT.isSizeT() || AT.isPtrdiffT()) &&
AT.matchesType(S.Context, CastTy))
- Pedantic = true;
+ Match = ArgType::NoMatchPedantic;
IntendedTy = CastTy;
ShouldNotPrintDirectly = true;
}
@@ -8281,10 +8280,20 @@
CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
- unsigned Diag =
- Pedantic
- ? diag::warn_format_conversion_argument_type_mismatch_pedantic
- : diag::warn_format_conversion_argument_type_mismatch;
+ unsigned Diag;
+ switch (Match) {
+ case ArgType::Match: llvm_unreachable("expected non-matching");
+ case ArgType::NoMatchPedantic:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
+ break;
+ case ArgType::NoMatchTypeConfusion:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
+ break;
+ case ArgType::NoMatch:
+ Diag = diag::warn_format_conversion_argument_type_mismatch;
+ break;
+ }
+
// In this case, the specifier is wrong and should be changed to match
// the argument.
EmitFormatDiagnostic(S.PDiag(Diag)
@@ -8340,7 +8349,7 @@
Name = TypedefTy->getDecl()->getName();
else
Name = CastTyName;
- unsigned Diag = Pedantic
+ unsigned Diag = Match == ArgType::NoMatchPedantic
? diag::warn_format_argument_needs_cast_pedantic
: diag::warn_format_argument_needs_cast;
EmitFormatDiagnostic(S.PDiag(Diag) << Name << IntendedTy << IsEnum
@@ -8367,10 +8376,19 @@
switch (S.isValidVarArgType(ExprTy)) {
case Sema::VAK_Valid:
case Sema::VAK_ValidInCXX11: {
- unsigned Diag =
- Pedantic
- ? diag::warn_format_conversion_argument_type_mismatch_pedantic
- : diag::warn_format_conversion_argument_type_mismatch;
+ unsigned Diag;
+ switch (Match) {
+ case ArgType::Match: llvm_unreachable("expected non-matching");
+ case ArgType::NoMatchPedantic:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
+ break;
+ case ArgType::NoMatchTypeConfusion:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
+ break;
+ case ArgType::NoMatch:
+ Diag = diag::warn_format_conversion_argument_type_mismatch;
+ break;
+ }
EmitFormatDiagnostic(
S.PDiag(Diag) << AT.getRepresentativeTypeName(S.Context) << ExprTy
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -389,7 +389,7 @@
case BuiltinType::UChar:
case BuiltinType::Bool:
if (T == C.UnsignedShortTy || T == C.ShortTy)
- return NoMatchPedantic;
+ return NoMatchTypeConfusion;
return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
: NoMatch;
case BuiltinType::Short:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8087,16 +8087,17 @@
"%select{type|underlying type}2 %1">,
InGroup<Format>;
def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
- "format specifies type %0 but the argument has "
- "%select{type|underlying type}2 %1">,
+ warn_format_conversion_argument_type_mismatch.Text>,
InGroup<FormatPedantic>;
+def warn_format_conversion_argument_type_mismatch_confusion : Warning<
+ warn_format_conversion_argument_type_mismatch.Text>,
+ InGroup<FormatTypeConfusion>, DefaultIgnore;
def warn_format_argument_needs_cast : 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<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">,
+ warn_format_argument_needs_cast.Text>,
InGroup<FormatPedantic>, DefaultIgnore;
def warn_printf_positional_arg_exceeds_data_args : Warning <
"data argument position '%0' exceeds the number of data arguments (%1)">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -780,6 +780,7 @@
def FormatNonStandard : DiagGroup<"format-non-iso">;
def FormatY2K : DiagGroup<"format-y2k">;
def FormatPedantic : DiagGroup<"format-pedantic">;
+def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
def Format : DiagGroup<"format",
[FormatExtraArgs, FormatZeroLength, NonNull,
FormatSecurity, FormatY2K, FormatInvalidSpecifier]>,
Index: clang/include/clang/AST/FormatString.h
===================================================================
--- clang/include/clang/AST/FormatString.h
+++ clang/include/clang/AST/FormatString.h
@@ -251,7 +251,21 @@
enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
AnyCharTy, CStrTy, WCStrTy, WIntTy };
- enum MatchKind { NoMatch = 0, Match = 1, NoMatchPedantic };
+ /// How well a given conversion specifier matches its argument.
+ enum MatchKind {
+ /// The conversion specifier and the argument types are incompatible. For
+ /// instance, "%d" and float.
+ NoMatch = 0,
+ /// The conversion specifier and the argument type are compatible. For
+ /// instance, "%d" and _Bool.
+ Match = 1,
+ /// The conversion specifier and the argument type are disallowed by the C
+ /// standard, but are in practice harmless. For instance, "%p" and int*.
+ NoMatchPedantic,
+ /// The conversion specifier and the argument type are compatible, but still
+ /// seems likely to be an error. For instance, "%hd" and _Bool.
+ NoMatchTypeConfusion,
+ };
private:
const Kind K;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
