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

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote:

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


For Darwin platform, yes. That's why this diagnostic should only be squelched for Darwin platforms.

>> 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)?

Once this patch is committed I'll update the corresponding documentation on developer.apple.com

In https://reviews.llvm.org/D42933#1091411, @smeenai wrote:

> In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote:
>
> > 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.
>
>
> Also keep in mind that Obj-C might be used on non-Apple platforms, e.g. there's an active review going on for GNUstep ABI v2 right now (https://reviews.llvm.org/D46052), and WinObjC is also a thing. Those platforms might not necessarily provide the same guarantees or consistency that Apple's do.


Indeed, I want to make sure that this change only affects Darwin platforms.

>>> 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.
> 
> I would honestly find it a bit surprising (and scary) if the optimizer actually took advantage of UB in the case where the size and alignment of the specifier and the actual type matches.

Hear, hear! I'm happy to fix any such optimization out of their misguided optimism :-)


Repository:
  rC Clang

https://reviews.llvm.org/D42933