[PATCH] D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 219815.
erik.pilkington marked 6 inline comments as done.
erik.pilkington added a comment.

Update the diagnostic text.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66856/new/

https://reviews.llvm.org/D66856

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Expr.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/format-bool.c


Index: clang/test/Sema/format-bool.c
===================================================================
--- /dev/null
+++ clang/test/Sema/format-bool.c
@@ -0,0 +1,46 @@
+// 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
+
+__attribute__((format(__printf__, 1, 2)))
+int p(const char *fmt, ...);
+
+BOOL b;
+
+#ifdef __OBJC__
+@interface NSString
++(NSString *)stringWithFormat:(NSString *)fmt, ...
+    __attribute__((format(__NSString__, 1, 2)));
+@end
+
+#define YES __objc_yes
+#define NO __objc_no
+#endif
+
+int main() {
+  p("%d", b);
+  p("%hd", b);
+#ifdef PEDANTIC
+  // expected-warning@-2 {{format specifies type 'short' but the argument has type}}
+#endif
+  p("%hhd", b);
+  p("%u", b);
+  p("%hu", b);
+#ifdef PEDANTIC
+  // expected-warning@-2 {{format specifies type 'unsigned short' but the argument has type}}
+#endif
+  p("%hhu", b);
+  p("%c", b); // expected-warning {{using '%c' format specifier, but argument has boolean value}}
+  p("%lc", b); // expected-warning {{using '%lc' format specifier, but argument has boolean value}}
+  p("%c", 1 == 1); // expected-warning {{using '%c' format specifier, but argument has boolean value}}
+  p("%f", b); // expected-warning{{format specifies type 'double' but the argument has type}}
+  p("%ld", b); // expected-warning{{format specifies type 'long' but the argument has type}}
+  p("%lld", b); // expected-warning{{format specifies type 'long long' but the argument has type}}
+
+#ifdef __OBJC__
+  [NSString stringWithFormat: @"%c", 0]; // probably fine?
+  [NSString stringWithFormat: @"%c", NO]; // expected-warning {{using '%c' format specifier, but argument has boolean value}}
+#endif
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8109,6 +8109,22 @@
     ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
+  // Diagnose attempts to print a boolean value as a character. Unlike other
+  // -Wformat diagnostics, this is fine from a type perspective, but it still
+  // doesn't make sense.
+  if (FS.getConversionSpecifier().getKind() == ConversionSpecifier::cArg &&
+      E->isKnownToHaveBooleanValue()) {
+    const CharSourceRange &CSR =
+        getSpecifierRange(StartSpecifier, SpecifierLen);
+    SmallString<4> FSString;
+    llvm::raw_svector_ostream os(FSString);
+    FS.toString(os);
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_bool_as_character)
+                             << FSString,
+                         E->getExprLoc(), false, CSR);
+    return true;
+  }
+
   const analyze_printf::ArgType::MatchKind Match =
       AT.matchesType(S.Context, ExprTy);
   bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -359,6 +359,7 @@
           case BuiltinType::SChar:
           case BuiltinType::UChar:
           case BuiltinType::Char_U:
+          case BuiltinType::Bool:
             return Match;
         }
       return NoMatch;
@@ -386,6 +387,7 @@
           case BuiltinType::SChar:
           case BuiltinType::Char_U:
           case BuiltinType::UChar:
+          case BuiltinType::Bool:
             if (T == C.UnsignedShortTy || T == C.ShortTy)
               return NoMatchPedantic;
             return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -185,6 +185,9 @@
     return CO->getTrueExpr()->isKnownToHaveBooleanValue() &&
            CO->getFalseExpr()->isKnownToHaveBooleanValue();
 
+  if (isa<ObjCBoolLiteralExpr>(E))
+    return true;
+
   return false;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8119,6 +8119,9 @@
 def warn_scanf_scanlist_incomplete : Warning<
   "no closing ']' for '%%[' in scanf format string">,
   InGroup<Format>;
+def warn_format_bool_as_character : Warning<
+  "using '%0' format specifier, but argument has boolean value">,
+  InGroup<Format>;
 def note_format_string_defined : Note<"format string is defined here">;
 def note_format_fix_specifier : Note<"did you mean to use '%0'?">;
 def note_printf_c_str: Note<"did you mean to call the %0 method?">;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits