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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66856#1650803 <https://reviews.llvm.org/D66856#1650803>, @aaron.ballman wrote:

> To me, actual problems at runtime belong in -Wformat and logical inconsistencies that don't cause runtime problems belong in -Wformat-pedantic. However, I think it's a defect that the C standard has no clearly UB-free way to print a _Bool value. I will bring this up on the reflectors to ask if we can clarify the standard here.


The reflector discussion is still happening and there are issues with ambiguities that we are pretty sure we want to correct. I've got a paper out that touches on some of this: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2420.pdf



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8123
+def warn_format_bool_as_character : Warning<
+  "format specifies a character but argument has boolean value">,
+  InGroup<Format>;
----------------
How about: `using '%0' format specifier, but argument has boolean value` and then pass in the character specifier used?


================
Comment at: clang/test/Sema/format-bool.c:26
+#ifdef PEDANTIC
+  // expected-warning at -2 {{format specifies type 'short' but the argument has type}}
+#endif
----------------
Just an FYI (not related to your patch): it seems that at least some people think this should be diagnosed as something other than by `-Wformat-pedantic`. Their thinking is that `-Wformat-pedantic` is for things that are required to have a diagnostic according to the standard but are not sufficiently interesting to warn about by default. This particular case is not required to be warned on by the standard, so it's not really a "pedantic" warning. It sounds like there may be interest in having `-Wformat-pedantic` for that understanding of pedantic, and introduce something like `-Wformat-type-mismatch` for these other cases where there is type confusion but not sufficiently dangerous to warrant warning by default?


================
Comment at: clang/test/Sema/format-bool.c:37
+  p("%lc", b); // expected-warning {{format specifies a character but argument has boolean value}}
+  p("%c", 1 == 1); // expected-warning {{format specifies a character but argument has boolean value}}
+  p("%f", b); // expected-warning{{format specifies type 'double' but the argument has type}}
----------------
The diagnostic is both correct and incorrect. Maybe that's fine. However, `==` returns an `int` in C and `int` is reasonable to pass in to `%c`, so the diagnostic seems a bit strange when it claims the argument has a boolean value. Then again, the results really are boolean despite the type being an int.

I think I've convinced myself this is fine as-is. :-)


================
Comment at: clang/test/Sema/format-bool.c:43
+#ifdef __OBJC__
+  [NSString stringWithFormat: @"%c", 0]; // probably fine?
+  [NSString stringWithFormat: @"%c", NO]; // expected-warning {{format specifies a character but argument has boolean value}}
----------------
Seems correct to me.


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

https://reviews.llvm.org/D66856