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

[arphaman]

Summary

The -Wformat recently started warning for the following code because of the added support for analysis for the '%zi' specifier.

NSInteger i = NSIntegerMax;
NSLog(@"max NSInteger = %zi", i);

The problem is that on armv7 %zi is 'long', and NSInteger is typedefed to 'int' in Foundation. We should avoid this warning as it's inconvenient to our users: it's target specific (happens only on armv7 and not arm64), and breaks their existing code. We should also silence the warning for the '%zu' specifier to ensure consistency.

rdar://36874921

[smeenai]

This seems ... suboptimal. It breaks existing code in the sense that their code was already broken, and the compiler is just diagnosing it correctly now. Note that Apple themselves recommend casting and using a proper printf specifier: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html (under Platform Dependencies).

[aaron.ballman]

It sounds to me like the check is working as designed and that the user code was already broken, but it happened to work at runtime because the stars aligned properly for the user. Am I misunderstanding something?

[compnerd]

@aaron.ballman, yeah, I believe that the warning is working as intended, it just so happens that at runtime things just happened to work out.

[arphaman]

Thanks for your input. You're right, this warning is quite correct (even though it seems like too much).

I discussed this with @dexonsmith and @dcoughlin and we decided to keep the warning as is and to avoid establishing new guidelines. I'm closing this one.

[jfb]

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)). I agree that, if we're playing C++ pedant and look at the typedefs, then it's undefined behavior and the code is broken. 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.

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.

[smeenai]

Note that the alignment matters in addition to the size.

The pattern I've seen internally is people using %zd for NSInteger and %tu for NSUInteger, since until clang 6 neither of those were format-checked at all.

I'd be fine with adding an option to relax the printf checking if the size and alignment of the specifier and the actual type match, even if the types themselves differ (-Wformat-relaxed or something similar), so that you'd still get warnings on cases where the specifier mismatch could cause runtime issues. I think that would be preferable to special-casing the Apple types.

[jfb]

In D42933#1090286, @smeenai wrote:

Note that the alignment matters in addition to the size.

Sure, but AFAICT from ./lib/Basic/Targets/* the alignment is also specified properly, is it not?

The pattern I've seen internally is people using %zd for NSInteger and %tu for NSUInteger, since until clang 6 neither of those were format-checked at all.

I'd be fine with adding an option to relax the printf checking if the size and alignment of the specifier and the actual type match, even if the types themselves differ (-Wformat-relaxed or something similar), so that you'd still get warnings on cases where the specifier mismatch could cause runtime issues.

What are the cases that you're worried about? The only ones I'm trying to capture here are NSInteger with %zd and NSUInteger with %zu, are there others?

I think that would be preferable to special-casing the Apple types.

If there are more that should be captured and a similar point solution doesn't apply, agreed. However I'd like to understand if we agree on the guarantees that the platform offers for the two specific cases I'm targeting.

[aaron.ballman]

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

[smeenai]

In D42933#1091234, @aaron.ballman wrote:

In 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 (D46052), and WinObjC is also a thing. Those platforms might not necessarily provide the same guarantees or consistency that Apple's do.

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.

[jfb]

In D42933#1091234, @aaron.ballman wrote:

In 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 D42933#1091411, @smeenai wrote:

In D42933#1091234, @aaron.ballman wrote:

In 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 (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 :-)

[jyknight]

In D42933#1090384, @jfb wrote:

In D42933#1090286, @smeenai wrote:

I'd be fine with adding an option to relax the printf checking if the size and alignment of the specifier and the actual type match, even if the types themselves differ (-Wformat-relaxed or something similar), so that you'd still get warnings on cases where the specifier mismatch could cause runtime issues.

What are the cases that you're worried about? The only ones I'm trying to capture here are NSInteger with %zd and NSUInteger with %zu, are there others?

I think that would be preferable to special-casing the Apple types.

If there are more that should be captured and a similar point solution doesn't apply, agreed. However I'd like to understand if we agree on the guarantees that the platform offers for the two specific cases I'm targeting.

I also think that special casing these two specifiers doesn't make sense. The problem is a general issue -- and one I've often found irritating. This exact same situation comes up all the time in non-Darwin contexts too.

E.g. one I find particularly annoying is "%lld" vs "%ld" in printf.

Some 64-bit platforms define e.g. int64_t as long long, and others as long. Although both types are size 8, align 8, and mean exactly the same thing, they are still distinct. And so, users write code like int64_t x = 0; printf("%ld", x);...which then emits warnings on the platform that defines int64_t as a long long. Obviously, the code _could_ be using PRId64 instead...but it often doesn't. So it'd sure be nice if you could restrict the warning to only warn about actual problems, and suppress these superficially-incompatible-but-not-really warnings.

So, +1 from me for the general-purpose -Wformat-relaxed flag (or whatever other flag name is appropriate).

[jfb]

In D42933#1091809, @jyknight wrote:

I also think that special casing these two specifiers doesn't make sense. The problem is a general issue -- and one I've often found irritating. This exact same situation comes up all the time in non-Darwin contexts too.

I don't think that's true. In this very specific case the platform guarantees that (sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger)) for all architectures this platform supports. This exact same situation does not come up all the time in other contexts because the int / long / long long distinction isn't backed by a portability guarantee. The warning is there to say "this code isn't portable!", but in the very specific case of NSInteger and NSUInteger it is and users rely on it so it cannot be broken. The warning is therefore spurious, users therefore rightly complain.

[aaron.ballman]

In D42933#1091502, @jfb wrote:

In D42933#1091234, @aaron.ballman wrote:

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

Ah, I missed the fact that you were proposing this only for Darwin.

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

People used to think the same thing about many other optimizations that had non-local effects, but we've come to learn that these are not uncommon and incredibly hard for users to track down when it happens. Some enterprising soul could lower the call with an undef on the wrongly-typed argument and then who knows what happens. However, it's also reasonable for us to define undefined behavior for a given platform and that sounds like exactly this case.

If we're looking at only a specific change for Darwin, I think it's reasonable to make it to -Wformat rather than require -Wformat-relaxed (though we may still want to assert that the size and alignment of the underlying type match expectations).

I've added @rjmccall as a reviewer to see what his opinions are on this, but I'm inclined to say this is a reasonable change to make for Darwin targets.

[jyknight]

In D42933#1091817, @jfb wrote:

In D42933#1091809, @jyknight wrote:

I also think that special casing these two specifiers doesn't make sense. The problem is a general issue -- and one I've often found irritating. This exact same situation comes up all the time in non-Darwin contexts too.

I don't think that's true. In this very specific case the platform guarantees that (sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger)) for all architectures this platform supports. This exact same situation does not come up all the time in other contexts because the int / long / long long distinction isn't backed by a portability guarantee. The warning is there to say "this code isn't portable!", but in the very specific case of NSInteger and NSUInteger it is and users rely on it so it cannot be broken. The warning is therefore spurious, users therefore rightly complain.

The printf format specifier warning is not primarily a cross-platform portability checker. And, although in a few limited cases it can act somewhat like one, in general it does not. Take my previous example -- you get no warning on a platform that has int64_t as a typedef for long -- if this feature is to be useful as a portability checker, it should require that you used the PRId64 macro. Or, given ssize_t x = 0; printf("%ld", x);, it doesn't tell you to use "%zd" instead if ssize_t is a typedef for long -- although to be portable you ought to.

No, the major usefulness of the printf warning is to tell you that your code is incorrect for the _current_ platform. And most importantly when you chose the wrong size for your argument.

Those types which have matched size and alignment are still different types, and so it's technically appropriate to warn about using the wrong specifier there, too. But it's also entirely reasonable to not want to be bothered by such useless trivia, so skipping the warning when there's only a technical and not actual mismatch seems entirely sensible. (I might suggest that it should even be the default behavior for the warning, and if you want the stricter checks you'd ask for them...but I bet I'll get more pushback on that...)

[smeenai]

In D42933#1091943, @jyknight wrote:

In D42933#1091817, @jfb wrote:

In D42933#1091809, @jyknight wrote:

I also think that special casing these two specifiers doesn't make sense. The problem is a general issue -- and one I've often found irritating. This exact same situation comes up all the time in non-Darwin contexts too.

I don't think that's true. In this very specific case the platform guarantees that (sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == sizeof(NSUInteger)) for all architectures this platform supports. This exact same situation does not come up all the time in other contexts because the int / long / long long distinction isn't backed by a portability guarantee. The warning is there to say "this code isn't portable!", but in the very specific case of NSInteger and NSUInteger it is and users rely on it so it cannot be broken. The warning is therefore spurious, users therefore rightly complain.

The printf format specifier warning is not primarily a cross-platform portability checker. And, although in a few limited cases it can act somewhat like one, in general it does not. Take my previous example -- you get no warning on a platform that has int64_t as a typedef for long -- if this feature is to be useful as a portability checker, it should require that you used the PRId64 macro. Or, given ssize_t x = 0; printf("%ld", x);, it doesn't tell you to use "%zd" instead if ssize_t is a typedef for long -- although to be portable you ought to.

No, the major usefulness of the printf warning is to tell you that your code is incorrect for the _current_ platform. And most importantly when you chose the wrong size for your argument.

Those types which have matched size and alignment are still different types, and so it's technically appropriate to warn about using the wrong specifier there, too. But it's also entirely reasonable to not want to be bothered by such useless trivia, so skipping the warning when there's only a technical and not actual mismatch seems entirely sensible. (I might suggest that it should even be the default behavior for the warning, and if you want the stricter checks you'd ask for them...but I bet I'll get more pushback on that...)

+1 to everything you said; what are other people's opinions on this? @rsmith perhaps?

[rjmccall]

I agree that the format-specifier checker is not intended to be a portability checker.

Any attempt to check portability problems has to account for two things:

• Not all code is intended to be portable. If you're going to warn about portability problems, you need some way to not annoy programmers who might have good reason to say that they only care about their code running on Linux / macOS / 64-bit / 32-bit / whatever. Generally this means splitting the portability warning out as a separate warning group.
• There are always established idioms for solving portability problems. In this case, a certain set of important typedefs from the C standard have been blessed with dedicated format modifiers like "z", but every other typedef in the world has to find some other solution, either by asserting that some existing format is good enough (as NSInteger does) or by introducing a macro that expands to an appropriate format (as some things in POSIX do). A proper format-portability warning would have to know about all of these conventions (in order to check that e.g. part of the format string came from the right macro), which just seems wildly out-of-scope for a compiler warning.

[smeenai]

In D42933#1092048, @rjmccall wrote:

I agree that the format-specifier checker is not intended to be a portability checker.

Any attempt to check portability problems has to account for two things:

• Not all code is intended to be portable. If you're going to warn about portability problems, you need some way to not annoy programmers who might have good reason to say that they only care about their code running on Linux / macOS / 64-bit / 32-bit / whatever. Generally this means splitting the portability warning out as a separate warning group.
• There are always established idioms for solving portability problems. In this case, a certain set of important typedefs from the C standard have been blessed with dedicated format modifiers like "z", but every other typedef in the world has to find some other solution, either by asserting that some existing format is good enough (as NSInteger does) or by introducing a macro that expands to an appropriate format (as some things in POSIX do). A proper format-portability warning would have to know about all of these conventions (in order to check that e.g. part of the format string came from the right macro), which just seems wildly out-of-scope for a compiler warning.

Apple's current recommendation for using printf with the NSInteger types is casting to a long, per https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html. Are you suggesting that recommendation would change to using %zd instead?

[jfb]

In D42933#1092077, @smeenai wrote:

In D42933#1092048, @rjmccall wrote:

I agree that the format-specifier checker is not intended to be a portability checker.

I don't disagree with the original intent, but AFAICT that's exactly the intent of the warning I'd like to get rid of. I'm making a very narrow point: there is no portability problem with the warning I'm specifically trying to silence (using %z with NS[U]Integer on Darwin). If we decide that -Wformat shouldn't emit portability warnings then my point is moot, so let's see if people agree to that. If not, then my point still stands.

Any attempt to check portability problems has to account for two things:

• Not all code is intended to be portable. If you're going to warn about portability problems, you need some way to not annoy programmers who might have good reason to say that they only care about their code running on Linux / macOS / 64-bit / 32-bit / whatever. Generally this means splitting the portability warning out as a separate warning group.

Right, it seems like recently -Wformat went against this, and users aren't thrilled.

• There are always established idioms for solving portability problems. In this case, a certain set of important typedefs from the C standard have been blessed with dedicated format modifiers like "z", but every other typedef in the world has to find some other solution, either by asserting that some existing format is good enough (as NSInteger does) or by introducing a macro that expands to an appropriate format (as some things in POSIX do). A proper format-portability warning would have to know about all of these conventions (in order to check that e.g. part of the format string came from the right macro), which just seems wildly out-of-scope for a compiler warning.

We could provide a macro for NS[U]Integer, but it's been long enough and there's enough existing code with %z. Not sure the code churn on user is worth it, if we can instead say "%z works".

Apple's current recommendation for using printf with the NSInteger types is casting to a long, per https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html. Are you suggesting that recommendation would change to using %zd instead?

Indeed, I believe that came from https://github.com/llvm-mirror/clang/commit/ec08735e1b6a51c11533311b774bf6336c0a5d63 and I intend to update documentation once the compiler is updated. It's not *wrong*, it's kinda more portable in that it's what you probably want to do if it's not NS[U]Integer, but it's more typing and users clearly haven't been following the recommendation (because let's be honest, %z is fine).

[rjmccall]

In D42933#1092077, @smeenai wrote:

Apple's current recommendation for using printf with the NSInteger types is casting to a long, per https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html. Are you suggesting that recommendation would change to using %zd instead?

I'm not saying that the recommendation would change, but long being pointer-sized is a pretty universal assumption on Darwin, I'm afraid.

In D42933#1092154, @jfb wrote:

I don't disagree with the original intent, but AFAICT that's exactly the intent of the warning I'd like to get rid of. I'm making a very narrow point: there is no portability problem with the warning I'm specifically trying to silence (using %z with NS[U]Integer on Darwin). If we decide that -Wformat shouldn't emit portability warnings then my point is moot, so let's see if people agree to that. If not, then my point still stands.

Agreed.

• There are always established idioms for solving portability problems. In this case, a certain set of important typedefs from the C standard have been blessed with dedicated format modifiers like "z", but every other typedef in the world has to find some other solution, either by asserting that some existing format is good enough (as NSInteger does) or by introducing a macro that expands to an appropriate format (as some things in POSIX do). A proper format-portability warning would have to know about all of these conventions (in order to check that e.g. part of the format string came from the right macro), which just seems wildly out-of-scope for a compiler warning.

We could provide a macro for NS[U]Integer, but it's been long enough and there's enough existing code with %z. Not sure the code churn on user is worth it, if we can instead say "%z works".

Right. I'm not trying to argue that we should add a macro for NSInteger, I'm saying that that's the sort of thing that a portability warning would have to consider. Portability warnings are... always tempting, but they're very, very difficult in C. Since we're not going to do that sort of work, we should back off from doing a portability warning.

John.

[smeenai]

Yeah, I think we all agree now that a portability warning isn't really tractable. Note that even for the warnings that motivated this diff, they should have only fired if size_t and NSInteger had separate types, so it wasn't a portability warning in that sense to begin with (as in, it would only warn if there was a mismatch for your current target, not if there was a potential mismatch for any target).

We still have two options:

1. Special-case NSInteger/NSUInteger on Apple platforms so that they can always be printed using %z without any issues.
2. Relax the format specifier warnings (either under a new warning option, e.g. -Wformat-relaxed, or by relaxing -Wformat itself and adding something like -Wformat-pedantic) so that you don't warn when the specifier and the actual type have the same size and alignment, even when the actual type is different (which would also cover the case in 1).

I'm personally in favor of 2, and I can start a discussion on cfe-dev if you think we should try to achieve a broader consensus. Whichever option we went with, we would also have to ensure that the optimizer didn't do anything bad (as @aaron.ballman pointed out), both now and in the future.

[jfb]

In D42933#1093503, @smeenai wrote:

Yeah, I think we all agree now that a portability warning isn't really tractable. Note that even for the warnings that motivated this diff, they should have only fired if size_t and NSInteger had separate types, so it wasn't a portability warning in that sense to begin with (as in, it would only warn if there was a mismatch for your current target, not if there was a potential mismatch for any target).

We still have two options:

1. Special-case NSInteger/NSUInteger on Apple platforms so that they can always be printed using %z without any issues.
2. Relax the format specifier warnings (either under a new warning option, e.g. -Wformat-relaxed, or by relaxing -Wformat itself and adding something like -Wformat-pedantic) so that you don't warn when the specifier and the actual type have the same size and alignment, even when the actual type is different (which would also cover the case in 1).

I'm personally in favor of 2, and I can start a discussion on cfe-dev if you think we should try to achieve a broader consensus. Whichever option we went with, we would also have to ensure that the optimizer didn't do anything bad (as @aaron.ballman pointed out), both now and in the future.

2 sounds better overall. However I don’t like a relaxed flag because it’s weird when you have relaxed/normal/pedantic. How do they mix? What if I want format warnings, but not “portability” ones. I think we either want to move this to pedantic and (as you propose) only warm on -Wformat if size or alignment don’t match. We can also have a separate -Wformat-portability warning where we try (and fail) to be helpful.

Please do start a thread on cfe-dev, much appreciated!

[smeenai]

Emailed cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2018-May/057934.html

[jfb]

Based on the cfe-dev discussion we'll want to handle the case of NSInteger with %z format on Darwin separately from other attempts at portability warnings in printf formats. I'll therefore re-post this patch as-is and CC all of you.

include/clang/Analysis/Analyses/FormatString.h

enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
               AnyCharTy, CStrTy, WCStrTy, WIntTy };
 
   enum MatchKind { NoMatch = 0, Match = 1, NoMatchPedantic };
 
 private:
   const Kind K;
   QualType T;
   const char *Name;
-  bool Ptr;
+  bool Ptr, IsSizeT;
+
 public:
   ArgType(Kind k = UnknownTy, const char *n = nullptr)
-      : K(k), Name(n), Ptr(false) {}
+      : K(k), Name(n), Ptr(false), IsSizeT(false) {}
   ArgType(QualType t, const char *n = nullptr)
-      : K(SpecificTy), T(t), Name(n), Ptr(false) {}
-  ArgType(CanQualType t) : K(SpecificTy), T(t), Name(nullptr), Ptr(false) {}
+      : K(SpecificTy), T(t), Name(n), Ptr(false), IsSizeT(false) {}
+  ArgType(CanQualType t)
+      : K(SpecificTy), T(t), Name(nullptr), Ptr(false), IsSizeT(false) {}
 
   static ArgType Invalid() { return ArgType(InvalidTy); }
   bool isValid() const { return K != InvalidTy; }
 
+  bool isSizeT() const { return IsSizeT; }
+
   /// Create an ArgType which corresponds to the type pointer to A.
   static ArgType PtrTo(const ArgType& A) {
     assert(A.K >= InvalidTy && "ArgType cannot be pointer to invalid/unknown");
     ArgType Res = A;
     Res.Ptr = true;
     return Res;
   }
 
+  /// Create an ArgType which corresponds to the size_t/ssize_t type.
+  static ArgType makeSizeT(const ArgType &A) {
+    ArgType Res = A;
+    Res.IsSizeT = true;
+    return Res;
+  }
+
   MatchKind matchesType(ASTContext &C, QualType argTy) const;
 
   QualType getRepresentativeType(ASTContext &C) const;
 
   std::string getRepresentativeTypeName(ASTContext &C) const;
 };
 
 class OptionalAmount {

lib/Analysis/PrintfFormatString.cpp

switch (LM.getKind()) {
       case LengthModifier::AsLongLong:
       case LengthModifier::AsQuad:
         return Ctx.LongLongTy;
       case LengthModifier::AsInt64:
         return ArgType(Ctx.LongLongTy, "__int64");
       case LengthModifier::AsIntMax:
         return ArgType(Ctx.getIntMaxType(), "intmax_t");
       case LengthModifier::AsSizeT:
-        return ArgType(Ctx.getSignedSizeType(), "ssize_t");
+        return ArgType::makeSizeT(ArgType(Ctx.getSignedSizeType(), "ssize_t"));
       case LengthModifier::AsInt3264:
         return Ctx.getTargetInfo().getTriple().isArch64Bit()
                    ? ArgType(Ctx.LongLongTy, "__int64")
                    : ArgType(Ctx.IntTy, "__int32");
       case LengthModifier::AsPtrDiff:
         return ArgType(Ctx.getPointerDiffType(), "ptrdiff_t");
       case LengthModifier::AsAllocate:
       case LengthModifier::AsMAllocate:
switch (LM.getKind()) {
       case LengthModifier::AsLongLong:
       case LengthModifier::AsQuad:
         return Ctx.UnsignedLongLongTy;
       case LengthModifier::AsInt64:
         return ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64");
       case LengthModifier::AsIntMax:
         return ArgType(Ctx.getUIntMaxType(), "uintmax_t");
       case LengthModifier::AsSizeT:
-        return ArgType(Ctx.getSizeType(), "size_t");
+        return ArgType::makeSizeT(ArgType(Ctx.getSizeType(), "size_t"));
       case LengthModifier::AsInt3264:
         return Ctx.getTargetInfo().getTriple().isArch64Bit()
                    ? ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64")
                    : ArgType(Ctx.UnsignedIntTy, "unsigned __int32");
       case LengthModifier::AsPtrDiff:
         return ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t");
       case LengthModifier::AsAllocate:
       case LengthModifier::AsMAllocate:

lib/Sema/SemaChecking.cpp

CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
 
   // Special-case some of Darwin's platform-independence types by suggesting
   // casts to primitive types that are known to be large enough.
   bool ShouldNotPrintDirectly = false; StringRef CastTyName;
   if (S.Context.getTargetInfo().getTriple().isOSDarwin()) {
     QualType CastTy;
     std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E);
     if (!CastTy.isNull()) {
+      // %zi/%zu are ok to use for NSInteger/NSUInteger of type int
+      // (long in ASTContext).
+      if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+          AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+        return true;
       IntendedTy = CastTy;
       ShouldNotPrintDirectly = true;
     }
   }
 
   // We may be able to offer a FixItHint if it is a supported type.
   PrintfSpecifier fixedFS = FS;
   bool success =

test/SemaObjC/format-size-spec-nsinteger.m

+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wformat -fsyntax-only -fblocks -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple arm64-apple-ios -Wformat -fsyntax-only -fblocks -verify -Wno-objc-root-class %s
+// expected-no-diagnostics
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+@class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+  NSInteger i = 0;
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);
+  NSLog(@"max NSUinteger = %zu", j);
+}