<div dir="ltr">Another +1 to that.<div><br>For NSInteger, one thing that projects can do until the SDK either makes NSInteger and ssize_t consistent or offers format strings for NSInteger instead of recommending casting is to add something like this and use it:</div><div><br></div><div><div>// The size of NSInteger and NSUInteger varies between 32-bit and 64-bit</div><div>// architectures and Apple does not provides standard format macros and</div><div>// recommends casting. This has many drawbacks, so instead define macros</div><div>// for formatting those types.</div><div>#if defined(OS_MACOSX)</div><div>#if defined(ARCH_CPU_64_BITS)</div><div>#if !defined(PRIdNS)</div><div>#define PRIdNS "ld"</div><div>#endif</div><div>#if !defined(PRIuNS)</div><div>#define PRIuNS "lu"</div><div>#endif</div><div>#if !defined(PRIxNS)</div><div>#define PRIxNS "lx"</div><div>#endif</div><div>#else  // defined(ARCH_CPU_64_BITS)</div><div>#if !defined(PRIdNS)</div><div>#define PRIdNS "d"</div><div>#endif</div><div>#if !defined(PRIuNS)</div><div>#define PRIuNS "u"</div><div>#endif</div><div>#if !defined(PRIxNS)</div><div>#define PRIxNS "x"</div><div>#endif</div><div>#endif</div><div>#endif  // defined(OS_MACOSX)</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 16, 2018 at 7:49 AM, Aaron Ballman via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm in the same camp as Hans and Joerg -- I would prefer to keep the<br>
existing check functionality and provide a more relaxed mode as an<br>
alternative if one is necessary (for the same reasons they bring up).<br>
<br>
~Aaron<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, May 16, 2018 at 7:28 AM, Hans Wennborg via cfe-dev<br>
<<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br>
> Sorry for the late reply. I didn't see the thread until now.<br>
><br>
> On Sat, May 12, 2018 at 1:26 AM, Shoaib Meenai via cfe-dev<br>
> <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br>
>> Hi all,<br>
>><br>
>><br>
>><br>
>> Currently, clang's format specifier check warnings (-Wformat) check for<br>
>> exact type matches between the specifier and the corresponding argument. I'm<br>
>> proposing adding an option to relax these warnings such that it doesn't warn<br>
>> when there's a type mismatch but the types of the specifier and the argument<br>
>> have the same size and alignment (and are both integral). For an example, on<br>
>> an LLP64 system, attempting to print a long with "%d" would not warn, but<br>
>> attempting to print a long long with "%d" would still warn. We could either<br>
>> do this relaxation for -Wformat and add a new option for the stricter<br>
>> warnings, or keep -Wformat as is and add a new option for the relaxed<br>
>> warnings.<br>
>><br>
>><br>
>><br>
>> The motivation is that during the 6.0 timeframe, clang gained format<br>
>> specifier checking for %zd (r308067) and %tu (r314470), which made -Wformat<br>
>> fire in a lot of additional cases. A particularly common case is programs<br>
>> using %zd to print NSInteger. This will be warning-free on 64-bit targets,<br>
>> where both ssize_t and NSInteger are long, but warn on 32-bit targets, where<br>
>> ssize_t is still long but NSInteger is int. This additional warning<br>
>> motivated <a href="https://reviews.llvm.org/D42933" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D42933</a>, which attempted to special case<br>
>> %z and NSInteger; the diff's comments have a bunch of additional discussion<br>
>> about this issue. The conclusion there was that, although having mismatched<br>
>> specifiers of this nature is definitely technically incorrect, there<br>
>> shouldn't actually be any issues in practice when the size and alignment of<br>
>> the specifier and the argument match. Ideally people would update their<br>
>> codebases to use proper specifiers, but that can be impractical for large<br>
>> codebases, and it would be really nice to have a way to restrict the warning<br>
>> to the cases which will actually cause problems in practice.<br>
><br>
> It sounds like the problem is really that NSInteger and size_t are<br>
> typedeffed to different types on 32-bit. Would it be possible to get<br>
> that fixed instead?<br>
><br>
>> Note that -Wformat isn't meant to be a general portability checker, since it<br>
>> only ever raises warnings when there's a type mismatch *on the current<br>
>> target*, not if there's any possibility of a mismatch.<br>
><br>
> It's (mostly) not trying to be clever; it's just checking what the<br>
> standard requires. Printing an int with %ld is wrong according to the<br>
> standard even if int and long are the same size on the target.<br>
><br>
>> For example, you<br>
>> could be printing out a size_t using %lu, and you wouldn't get warnings if<br>
>> your compilation target used unsigned long as the underlying type for<br>
>> size_t, even though that underlying type could differ across targets.<br>
><br>
> Right, because doing this kind of portability checks would be hard:<br>
> the compiler can't know if the source used unsigned long and was lucky<br>
> that it matched size_t, or if unsigned long had been carefully<br>
> selected to match.<br>
><br>
>> This<br>
>> proposal does mean we would lose out on some additional portability<br>
>> warnings; continuing the previous example, we would now not get warnings for<br>
>> printing out a size_t using %lu even for a compilation target where size_t<br>
>> was actually an unsigned int, assuming that the target had the same size and<br>
>> alignment for int and long (as most 32-bit platforms do). However, given<br>
>> that -Wformat was never meant to be a general portability checker, and that<br>
>> there would still be an option to get the fully strict warnings, I don't<br>
>> think this is too much of an issue in practice.<br>
><br>
> I prefer to think of them as correctness warnings rather than<br>
> portability warnings. I'm not buying the argument that because it<br>
> doesn't catch all portability issues, it shouldn't warn about<br>
> incorrect code.<br>
><br>
>> What are people's thoughts here? Are there preferences for relaxing -Wformat<br>
>> and adding a new option for strict format warnings vs. keeping -Wformat<br>
>> as-is and adding a new option for relaxed format warnings? Of course, we<br>
>> should also ensure that the optimizer doesn't do anything surprising when<br>
>> there's a type mismatch between the specifier and the argument but both<br>
>> types have the same size and alignment (i.e., the case where the relaxed<br>
>> format warning won't fire), both now and in the future.<br>
><br>
> I would prefer not to relax -Wformat by default. I think what it's<br>
> doing is simple and correct, and a large amount of code compiles with<br>
> it.<br>
><br>
> I'm not opposed to adding a -Wformat-relaxed option if there's a lot<br>
> of demand for it.<br>
><br>
> TL;DR: What Joerg said.<br>
><br>
> Thanks,<br>
> Hans<br>
> ______________________________<wbr>_________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
</div></div></blockquote></div><br></div>