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@-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



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

Reply via email to