[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D42933#1090268, @jfb wrote:

> I was just looking at this, and I think @arphaman's patch is pretty much the right approach (with 2 suggested fixes below).
>
> I don't think the code we're currently warning on is broken: a user code has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all platforms which support those types the implementor has guaranteed that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger))`.


Yes, but is this guaranteed to be the case or does it happen to be the case? I'm worried about the less mainstream uses where you might find ObjC code where this does not hold but -Wformat points out a portability concern.

> I agree that, if we're playing C++ pedant and look at the typedefs, then it's undefined behavior and the code is broken.

This is reason alone to diagnose the code, because the optimizer is welcome to use that UB to perform transformations the programmer did not expect. However, I'm not certain our optimizer is currently paying attention to that UB directly, so this may not be an immediate issue (but could be a pitfall for the future) but I'm not certain about other optimizers for which portability would still be a concern.

> However the implementation provided more guarantees than C++ does through its platform typedefs so pendantry need not apply (or rather: clang should look no further than the typedef, and trust the platform in this particular case).
> 
> We of course should still warn on sign mismatch.
> 
> I don't think this should even be a warning with pedantic on: there's no portability issue at all because all OSes and architectures where this could ever fire are guaranteed to do the right thing.

Can you point me to this guarantee, as I was unable to find one (but that doesn't mean it doesn't exist)?

> Suggested fixes:
> 
> 1. Don't compare `CastTyName` directly, instead recurse as `shouldNotPrintDirectly` does so `typedef` nesting is also handled.
> 2. Add a test that sign mismatched is still diagnosed.
> 
>   If you all agree I'll post an updated patch.


Repository:
  rC Clang

https://reviews.llvm.org/D42933