[cfe-commits] [Patch] Make -Wformat accept printf("%hhx", c); with -funsigned-char (PR12761)

[Hans Wennborg]

On Tue, May 8, 2012 at 4:26 PM, David Blaikie <dblaikie at gmail.com> wrote:
> There's a few unrelated whitespace changes here - those are usually
> avoided (changing only whitespace on lines your patch is touching
> anyway) or at least committed separately to keep the revision history
> more clear.

OK, I'll take those out of the patch.

> Also, your test case only tests one case, but the code change modifies
> at least a couple fo the switch cases - it'd be nice to test all your
> changes. If possible, please add the test cases to an existing test
> file (I assume there are existing test cases for these warnings)
> rather than adding a new one. Though I realize you need to pass extra
> flags - they may not conflict with the existing test cases (or perhaps
> they do - I'm not sure)

That's a good idea. Moving it to test/Sema/format-strings.c.

> & one naive question: I assume there's no way to detect that the user
> wrote the type 'char', not 'unsigned char' or 'signed char'?

Yes, those are different builtin types, if that's what you mean.
'signed char' and 'unsigned char' are BuiltinType::SChar and UChar,
respectively. Just 'char' is BuiltinType::Char_S or Char_U depending
whether its signed or unsigned. Is that what you mean?

I'll address Matt's comments below as well and land.

Thanks,
Hans