[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL335393: [Sema] -Wformat-pedantic only for 
NSInteger/NSUInteger %zu/%zi on Darwin (authored by jfb, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47290

Files:
  cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Analysis/PrintfFormatString.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m
  cfe/trunk/test/FixIt/fixit-format-ios.m
  cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m

Index: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
===
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
@@ -256,26 +256,34 @@
 private:
   const Kind K;
   QualType T;
-  const char *Name;
-  bool Ptr;
+  const char *Name = nullptr;
+  bool Ptr = false, IsSizeT = false;
+
 public:
-  ArgType(Kind k = UnknownTy, const char *n = nullptr)
-  : K(k), Name(n), Ptr(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) {}
+  ArgType(Kind K = UnknownTy, const char *N = nullptr) : K(K), Name(N) {}
+  ArgType(QualType T, const char *N = nullptr) : K(SpecificTy), T(T), Name(N) {}
+  ArgType(CanQualType T) : K(SpecificTy), T(T) {}
 
   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 ) {
+ArgType Res = A;
+Res.IsSizeT = true;
+return Res;
+  }
+
   MatchKind matchesType(ASTContext , QualType argTy) const;
 
   QualType getRepresentativeType(ASTContext ) const;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7719,6 +7719,10 @@
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,
   InGroup;
+def warn_format_argument_needs_cast_pedantic : Warning<
+  "%select{values of type|enum values with underlying type}2 '%0' should not "
+  "be used as format arguments; add an explicit cast to %1 instead">,
+  InGroup, DefaultIgnore;
 def warn_printf_positional_arg_exceeds_data_args : Warning <
   "data argument position '%0' exceeds the number of data arguments (%1)">,
   InGroup;
Index: cfe/trunk/test/FixIt/fixit-format-ios.m
===
--- cfe/trunk/test/FixIt/fixit-format-ios.m
+++ cfe/trunk/test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m
===
--- cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m
+++ cfe/trunk/test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m
===
--- cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m
+++ cfe/trunk/test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if 

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Alex Lorenz via Phabricator via cfe-commits]

arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for working on this!


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1127190, @jfb wrote:

> In https://reviews.llvm.org/D47290#1126443, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote:
> >
> > > Okay, that's fair, but the vendor-specific type for my Windows example is 
> > > spelled `DWORD`. I'm really worried that this special case will become a 
> > > precedent and we'll wind up with -Wformat being relaxed for everything 
> > > based on the same rationale. If that's how the community wants -Wformat 
> > > to work, cool, but I'd like to know if we're intending to change (what I 
> > > see as) the design of this warning.
> >
> >
> > I spoke with @jfb offline and am less concerned about this patch now. He's 
> > welcome to correct me if I misrepresent anything, but the precedent this 
> > sets is that a platform "owner" (someone with authority, not Joe Q 
> > Random-User) can relax -Wformat as in this patch, but this is not a 
> > precedent for a blanket change to -Wformat just because the UB happens to 
> > work and we "know" it.
>
>
> Thanks for asking these questions Aaron, it's helped answer everyone's 
> concerns and explain our respective positions. You've certainly summarized 
> what I was thinking.


Excellent!

> It sounds like you're both OK moving forward with this patch?

I am okay moving forward.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In https://reviews.llvm.org/D47290#1126443, @aaron.ballman wrote:

> In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote:
>
> > Okay, that's fair, but the vendor-specific type for my Windows example is 
> > spelled `DWORD`. I'm really worried that this special case will become a 
> > precedent and we'll wind up with -Wformat being relaxed for everything 
> > based on the same rationale. If that's how the community wants -Wformat to 
> > work, cool, but I'd like to know if we're intending to change (what I see 
> > as) the design of this warning.
>
>
> I spoke with @jfb offline and am less concerned about this patch now. He's 
> welcome to correct me if I misrepresent anything, but the precedent this sets 
> is that a platform "owner" (someone with authority, not Joe Q Random-User) 
> can relax -Wformat as in this patch, but this is not a precedent for a 
> blanket change to -Wformat just because the UB happens to work and we "know" 
> it.


Thanks for asking these questions Aaron, it's helped answer everyone's concerns 
and explain our respective positions. You've certainly summarized what I was 
thinking.

It sounds like you're both OK moving forward with this patch?


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote:

> Okay, that's fair, but the vendor-specific type for my Windows example is 
> spelled `DWORD`. I'm really worried that this special case will become a 
> precedent and we'll wind up with -Wformat being relaxed for everything based 
> on the same rationale. If that's how the community wants -Wformat to work, 
> cool, but I'd like to know if we're intending to change (what I see as) the 
> design of this warning.


I spoke with @jfb offline and am less concerned about this patch now. He's 
welcome to correct me if I misrepresent anything, but the precedent this sets 
is that a platform "owner" (someone with authority, not Joe Q Random-User) can 
relax -Wformat as in this patch, but this is not a precedent for a blanket 
change to -Wformat just because the UB happens to work and we "know" it.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1124991, @hans wrote:

> In https://reviews.llvm.org/D47290#1124964, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D47290#1124956, @hans wrote:
> >
> > > In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote:
> > >
> > > > In https://reviews.llvm.org/D47290#1124866, @hans wrote:
> > > >
> > > > > If we really want to special-case NSInteger, and given that you're 
> > > > > targeting a specific wide-spread pattern maybe that's the right thing 
> > > > > to do, I think we should make -Wformat accept (move the warning 
> > > > > behind -Wformat-pedantic I suppose) printing NSInteger with *any* 
> > > > > integral type of the right size, not just size_t.
> > > >
> > > >
> > > > Would you be similarly okay with %ld and %d on Windows platforms when 
> > > > mixing up int and long?
> > >
> > >
> > > No, I'm against a general relaxation of -Wformat, but to solve JF's 
> > > problem I think special-casing NSInteger might be reasonable.
> >
> >
> > How is JF's problem different?
>
>
> It concerns a vendor-specific type. Of course I personally think it would be 
> better if the code could be fixed, but it doesn't sound like that's an option 
> so then I think special-casing for NSInteger is an acceptable solution.


Okay, that's fair, but the vendor-specific type for my Windows example is 
spelled `DWORD`. I'm really worried that this special case will become a 
precedent and we'll wind up with -Wformat being relaxed for everything based on 
the same rationale. If that's how the community wants -Wformat to work, cool, 
but I'd like to know if we're intending to change (what I see as) the design of 
this warning.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D47290#1124964, @aaron.ballman wrote:

> In https://reviews.llvm.org/D47290#1124956, @hans wrote:
>
> > In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D47290#1124866, @hans wrote:
> > >
> > > > If we really want to special-case NSInteger, and given that you're 
> > > > targeting a specific wide-spread pattern maybe that's the right thing 
> > > > to do, I think we should make -Wformat accept (move the warning behind 
> > > > -Wformat-pedantic I suppose) printing NSInteger with *any* integral 
> > > > type of the right size, not just size_t.
> > >
> > >
> > > Would you be similarly okay with %ld and %d on Windows platforms when 
> > > mixing up int and long?
> >
> >
> > No, I'm against a general relaxation of -Wformat, but to solve JF's problem 
> > I think special-casing NSInteger might be reasonable.
>
>
> How is JF's problem different?


It concerns a vendor-specific type. Of course I personally think it would be 
better if the code could be fixed, but it doesn't sound like that's an option 
so then I think special-casing for NSInteger is an acceptable solution.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1124956, @hans wrote:

> In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D47290#1124866, @hans wrote:
> >
> > > If we really want to special-case NSInteger, and given that you're 
> > > targeting a specific wide-spread pattern maybe that's the right thing to 
> > > do, I think we should make -Wformat accept (move the warning behind 
> > > -Wformat-pedantic I suppose) printing NSInteger with *any* integral type 
> > > of the right size, not just size_t.
> >
> >
> > Would you be similarly okay with %ld and %d on Windows platforms when 
> > mixing up int and long?
>
>
> No, I'm against a general relaxation of -Wformat, but to solve JF's problem I 
> think special-casing NSInteger might be reasonable.


How is JF's problem different?


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote:

> In https://reviews.llvm.org/D47290#1124866, @hans wrote:
>
> > If we really want to special-case NSInteger, and given that you're 
> > targeting a specific wide-spread pattern maybe that's the right thing to 
> > do, I think we should make -Wformat accept (move the warning behind 
> > -Wformat-pedantic I suppose) printing NSInteger with *any* integral type of 
> > the right size, not just size_t.
>
>
> Would you be similarly okay with %ld and %d on Windows platforms when mixing 
> up int and long?


No, I'm against a general relaxation of -Wformat, but to solve JF's problem I 
think special-casing NSInteger might be reasonable.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1124866, @hans wrote:

> If we really want to special-case NSInteger, and given that you're targeting 
> a specific wide-spread pattern maybe that's the right thing to do, I think we 
> should make -Wformat accept (move the warning behind -Wformat-pedantic I 
> suppose) printing NSInteger with *any* integral type of the right size, not 
> just size_t.


Would you be similarly okay with %ld and %d on Windows platforms when mixing up 
int and long?


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

What's special about size_t though? If I understand your patch correctly, it 
would suppress warning about printing NSInteger with %zd, but still warn about 
%ld even though ssize_t=long on the target? As a user I'd find this confusing.

If we really want to special-case NSInteger, and given that you're targeting a 
specific wide-spread pattern maybe that's the right thing to do, I think we 
should make -Wformat accept (move the warning behind -Wformat-pedantic I 
suppose) printing NSInteger with *any* integral type of the right size, not 
just size_t.

Also, I haven't looked at what happens on the scanf side. Does that need an 
equivalent change?


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: hans, joerg.
aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1115352, @jfb wrote:

> Hopefully this makes sense? I'm not sure I summarize my context very well. 
> I'm happy to talk about it in-person next week too.


I appreciate the in-person conversation, it helped me to understand your goals 
better.

That said, I still see this as changing the portability aspects of -Wformat 
(because we'd be changing the behavior based on the target, but this is perhaps 
different from the portability concerns @rjmccall raised) and I don't see where 
to get off that train. For instance, do we also silence -Wformat when targeting 
Windows with %ld and %d because long and int have the same size and alignment 
requirements for x86 and x86-64 (at least, I don't know about other Windows 
targets) and that is effectively baked into the platform requirements? If we 
don't, how is that situation different than the one motivating your changes?

As a user, I very much appreciate that -Wformat gives me the same diagnostics 
no matter what platform I target, so these changes make me a bit uncomfortable 
-- those warnings are not useless to me. You (rightly) asked for a concrete 
explanation of why. My reason is: the user's code here is UB and there is a 
long history demonstrating that "it's UB, but it works and does what I want!" 
suddenly changing into "it's not what I want" silently, perhaps many years down 
the line, because the optimizer suddenly got smarter and it results in 
exploitable security vulnerabilities. However, I'm no longer as scared that the 
optimizer is going to start making type-based decisions based on this format 
specifier string. I can imagine the optimizer making value-based decisions 
based on a format specifier string (like seeing a value matches a %s specifier 
and assuming the value cannot be null), but that's not this. However, I have 
had users for which UB is verboten. Period. Full stop. I doubt these users care 
at all about Apple's platform, so *this* change isn't likely to impact them, 
but the *precedent* from this change certainly could. However, perhaps making 
this pedantic as in this patch will be reasonable (I believe these folks 
already pass -pedantic when they realize that's a thing). I have to do my 
homework on that (and can't do that homework this week).


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In https://reviews.llvm.org/D47290#1113422, @aaron.ballman wrote:

> In https://reviews.llvm.org/D47290#1113412, @jfb wrote:
>
> > In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote:
> >
> > > This is relaxing `-Wformat` and making users instead write 
> > > `-Wformat-pedantic` to get the strong guarantees, which is the opposite 
> > > of what I thought the consensus was. Have I misunderstood something?
> >
> >
> > From Shoaib's email:
> >
> > > Based on all the discussion so far, I think the consensus is that we 
> > > don't want to relax -Wformat by default, but an additional relaxed option 
> > > would be fine. That works for me (where I can just switch my codebases to 
> > > use the new warning option), but it wouldn't handle JF Bastien's use 
> > > case, so he would still want to pursue the relaxations specific to Apple 
> > > platforms.
> >
> > This patch is specific to the Darwin platform, as proposed in Alex' 
> > original patch and as I promised to do in the cfe-dev thread. Shoaib's 
> > follow-up would be different and do what I think you're referring to.
>
>
> There were several people on the thread asking to not relax -Wformat for any 
> target, but instead wanted a separate warning flag to perform a more relaxed 
> check.
>
> http://lists.llvm.org/pipermail/cfe-dev/2018-May/057938.html


I don't think this one is talking about platform-specific warnings, I therefore 
don't think it applies to this patch.

> http://lists.llvm.org/pipermail/cfe-dev/2018-May/058030.html
>  http://lists.llvm.org/pipermail/cfe-dev/2018-May/058039.html

Hans' comment and your +1 are on-topic here, and are the only ones AFAIK 
(others don't care or explicitly agree with this patch). What I'd like to 
emphasize is that the new stricter warnings are causing issues for developers 
on our platform. The warnings are new, and they don't help developers because 
the platform guarantees that their code is fine. That frustrates developers 
because they *know* that we know better (they might even find this discussion 
when searching! ), yet we chose to come in and warn them anyways. I have yet 
to hear a reason why we're doing them a service by warning in this case. It 
really has the feel of a pedantic warning, and that's what this patch makes it.

I'll quote James  
slightly out of context:

> No, this actually has been a thorn in the side of some people at google,
>  causing a good deal of angst (only for a very small number of people,
>  granted). I'd not truly put the blame on the warning, but rather on printf
>  itself -- and that we're still USING printf (and other functions that
>  ultimately wrap printf) all over the place. The long-term plan is certainly
>  to switch to better APIs. But, it'd be nice to be able to reduce the issue
>  in the meantime.
> 
> The problem we have is that Google has an internal "int64" typedef, which
>  predates the availability of C99 "int64_t". We'd like to eliminate this,
>  and switch everything over to using the standard int64_t. (Well, really
>  we'd've liked to have done that years ago...) However, "int64" is defined
>  as "long long", while "int64_t" is defined as "long" on 64-bit linux. A
>  bunch of printf specifiers all throughout the codebase use %lld. This
>  mismatching type-specifier makes it quite difficult to change the
>  type. Automatically updating all the format strings has not been found to
>  be easy.
> 
> If we could eliminate the warning on mismatch of "%lld" vs "long", on a
>  platform where they're the same, that could make things a whole lot easier.

It seems Hans' comment came from the sentiment "warning is the right thing to 
do, and from Google's vast experience it's not a problem". James provides a 
counter-point in line with ours: we receive exactly that feedback for NSInteger 
and `%z`. The code exists, it works fine, the platform guarantees it'll keep 
working fine, and now a tonne of "useless" warnings come in. That's frustrating 
because it means developers have these options:

- Useless code churn in code that otherwise won't change, ever
- Silence the warning, potentially silencing useful warnings later (because 
-Wformat-relaxed could start doing more things)

That's frustrating when you update compilers, especially if your update is 
incremental because only some developers see the warning. It means people are 
reluctant to update, because the first thing they see is us being pedants, not 
the cool new features we bring in. If they saw a flood of really useful 
warnings instead they'd be happy,  but even there they get drowned by these 
format warnings... Kind of a shit sandwich if you'll pardon the graphic 
description.

Hopefully this makes sense? I'm not sure I summarize my context very well. I'm 
happy to talk about it in-person next week too.


Repository:
  rC Clang

https://reviews.llvm.org/D47290

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1113412, @jfb wrote:

> In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote:
>
> > This is relaxing `-Wformat` and making users instead write 
> > `-Wformat-pedantic` to get the strong guarantees, which is the opposite of 
> > what I thought the consensus was. Have I misunderstood something?
>
>
> From Shoaib's email:
>
> > Based on all the discussion so far, I think the consensus is that we don't 
> > want to relax -Wformat by default, but an additional relaxed option would 
> > be fine. That works for me (where I can just switch my codebases to use the 
> > new warning option), but it wouldn't handle JF Bastien's use case, so he 
> > would still want to pursue the relaxations specific to Apple platforms.
>
> This patch is specific to the Darwin platform, as proposed in Alex' original 
> patch and as I promised to do in the cfe-dev thread. Shoaib's follow-up would 
> be different and do what I think you're referring to.


There were several people on the thread asking to not relax -Wformat for any 
target, but instead wanted a separate warning flag to perform a more relaxed 
check.

http://lists.llvm.org/pipermail/cfe-dev/2018-May/057938.html
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058030.html
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058039.html


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Addressed comments.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 148737.
jfb marked 2 inline comments as done.
jfb added a comment.

- Fix variable capitalization.


Repository:
  rC Clang

https://reviews.llvm.org/D47290

Files:
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/PrintfFormatString.cpp
  lib/Sema/SemaChecking.cpp
  test/FixIt/fixit-format-ios-nopedantic.m
  test/FixIt/fixit-format-ios.m
  test/SemaObjC/format-size-spec-nsinteger.m

Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if !defined(PEDANTIC)
+// expected-no-diagnostics
+#endif
+
+#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);
+
+#if defined(PEDANTIC)
+  // expected-warning@-4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+  // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+#endif
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6610,11 +6610,11 @@
 ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
-  if (match == analyze_printf::ArgType::Match) {
+  const analyze_printf::ArgType::MatchKind Match =
+  AT.matchesType(S.Context, ExprTy);
+  bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
+  if (Match == analyze_printf::ArgType::Match)
 return true;
-  }
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
@@ -6690,32 +6690,37 @@
 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). Only complain to pedants.
+  if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+  AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+Pedantic = true;
   IntendedTy = CastTy;
   ShouldNotPrintDirectly = true;
 }
   }
 
   // We may be able to offer a FixItHint if it is a supported type.
   PrintfSpecifier fixedFS = FS;
-  bool success =
+  bool Success =
   fixedFS.fixType(IntendedTy, S.getLangOpts(), S.Context, isObjCContext());
 
-  if (success) {
+  if (Success) {
 // Get the fix string from the fixed format specifier
 SmallString<16> buf;
 llvm::raw_svector_ostream os(buf);
 fixedFS.toString(os);
 
 CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
 
 if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
-  unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
-  if (match == analyze_format_string::ArgType::NoMatchPedantic) {
-diag = 

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added a comment.

In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote:

> This is relaxing `-Wformat` and making users instead write 
> `-Wformat-pedantic` to get the strong guarantees, which is the opposite of 
> what I thought the consensus was. Have I misunderstood something?


From Shoaib's email:

> Based on all the discussion so far, I think the consensus is that we don't 
> want to relax -Wformat by default, but an additional relaxed option would be 
> fine. That works for me (where I can just switch my codebases to use the new 
> warning option), but it wouldn't handle JF Bastien's use case, so he would 
> still want to pursue the relaxations specific to Apple platforms.

This patch is specific to the Darwin platform, as proposed in Alex' original 
patch and as I promised to do in the cfe-dev thread. Shoaib's follow-up would 
be different and do what I think you're referring to.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is relaxing `-Wformat` and making users instead write `-Wformat-pedantic` 
to get the strong guarantees, which is the opposite of what I thought the 
consensus was. Have I misunderstood something?




Comment at: include/clang/Analysis/Analyses/FormatString.h:263-265
+  ArgType(Kind k = UnknownTy, const char *n = nullptr) : K(k), Name(n) {}
+  ArgType(QualType t, const char *n = nullptr) : K(SpecificTy), T(t), Name(n) 
{}
+  ArgType(CanQualType t) : K(SpecificTy), T(t) {}

If we're going to update this, can you fix the parameter names to be in line 
with the coding conventions?



Comment at: lib/Sema/SemaChecking.cpp:6597-6599
+  const analyze_printf::ArgType::MatchKind match =
+  AT.matchesType(S.Context, ExprTy);
+  bool pedantic = match == analyze_printf::ArgType::NoMatchPedantic;

These identifiers also need some love to match the coding conventions. Happens 
elsewhere in this patch as well.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);  // CHECK: values of type 'NSInteger' 
should not be used as format arguments; add an explicit cast to 'long' instead
+  NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' 
should not be used as format arguments; add an explicit cast to 'unsigned long' 
instead

ahatanak wrote:
> This test looks identical to 
> test/SemaObjC/format-size-spec-nsinteger-nopedantic.m except for the CHECK 
> lines. You can probably merge the two files if you separate the CHECK lines 
> from the lines that call NSLog (see test/Sema/format-strings-darwin.c for 
> example).
Cute, I didn't know about this pattern.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 148467.
jfb marked an inline comment as done.
jfb added a comment.

- Merge format-size-spec-nsinteger


Repository:
  rC Clang

https://reviews.llvm.org/D47290

Files:
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/PrintfFormatString.cpp
  lib/Sema/SemaChecking.cpp
  test/FixIt/fixit-format-ios-nopedantic.m
  test/FixIt/fixit-format-ios.m
  test/SemaObjC/format-size-spec-nsinteger.m

Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if !defined(PEDANTIC)
+// expected-no-diagnostics
+#endif
+
+#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);
+
+#if defined(PEDANTIC)
+  // expected-warning@-4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+  // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+#endif
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6594,11 +6594,11 @@
 ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
-  if (match == analyze_printf::ArgType::Match) {
+  const analyze_printf::ArgType::MatchKind match =
+  AT.matchesType(S.Context, ExprTy);
+  bool pedantic = match == analyze_printf::ArgType::NoMatchPedantic;
+  if (match == analyze_printf::ArgType::Match)
 return true;
-  }
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
@@ -6674,6 +6674,11 @@
 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). Only complain to pedants.
+  if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+  AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+pedantic = true;
   IntendedTy = CastTy;
   ShouldNotPrintDirectly = true;
 }
@@ -6693,10 +6698,10 @@
 CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
 
 if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
-  unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
-  if (match == analyze_format_string::ArgType::NoMatchPedantic) {
-diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
-  }
+  unsigned diag =
+  pedantic
+  ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+  : diag::warn_format_conversion_argument_type_mismatch;
   // In this case, the specifier is wrong and should be changed to match
   // the argument.
   EmitFormatDiagnostic(S.PDiag(diag)
@@ -6752,9 +6757,11 @@
   

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);  // CHECK: values of type 'NSInteger' 
should not be used as format arguments; add an explicit cast to 'long' instead
+  NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' 
should not be used as format arguments; add an explicit cast to 'unsigned long' 
instead

This test looks identical to 
test/SemaObjC/format-size-spec-nsinteger-nopedantic.m except for the CHECK 
lines. You can probably merge the two files if you separate the CHECK lines 
from the lines that call NSLog (see test/Sema/format-strings-darwin.c for 
example).


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: test/FixIt/fixit-format-ios-nopedantic.m:21
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}

alexshap wrote:
> maybe i'm missing smth, but i don't see any verification in this test.
`-Werror` makes the test fail if something is reported.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.



Comment at: test/FixIt/fixit-format-ios-nopedantic.m:21
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}

maybe i'm missing smth, but i don't see any verification in this test.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: ahatanak, vsapsai, alexshap, aaron.ballman, javed.absar, 
jfb, rjmccall.
Herald added subscribers: cfe-commits, aheejin, kristof.beyls.

Pick https://reviews.llvm.org/D42933 back up, and make NSInteger/NSUInteger 
with %zu/%zi specifiers on Darwin warn only in pedantic mode. The default 
-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. This is acceptable because Darwin guarantees that, despite 
the unfortunate choice of typedef, sizeof(size_t) == sizeof(NS[U]Integer), the 
warning is therefore noisy for pedantic reasons. Once this is in I'll update 
public documentation.

Related discussion on cfe-dev:
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058050.html

rdar://36874921


Repository:
  rC Clang

https://reviews.llvm.org/D47290

Files:
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/PrintfFormatString.cpp
  lib/Sema/SemaChecking.cpp
  test/FixIt/fixit-format-ios-nopedantic.m
  test/FixIt/fixit-format-ios.m
  test/SemaObjC/format-size-spec-nsinteger-nopedantic.m
  test/SemaObjC/format-size-spec-nsinteger.m

Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wformat-pedantic -fsyntax-only -Wno-objc-root-class %s 2>&1 | FileCheck %s
+
+#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);  // CHECK: values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead
+  NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead
+}
Index: test/SemaObjC/format-size-spec-nsinteger-nopedantic.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger-nopedantic.m
@@ -0,0 +1,22 @@
+// 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);
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6594,11 +6594,11 @@
 ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
-  if