On 12/19/2017 10:20 AM, Jakub Jelinek wrote:
Hi!
This inform is correct in english, but wrong in many other languages.
I think someone already mentioned this in another review but
the computation below is far from intuitive:
+ ? (fmtres.range.likely % 1000000) + 1000000
+ : fmtres.range.likely,
It may not be a big deal in this one isolated instance but
there are many more examples of this problem in this code
and elsewhere that should be fixed (the sprintf warnings
aren't as easy to deal with due to bug 77810).
Other examples of the same problem are going to be in some
of the other warnings I added (-Walloc-size-larger-than,
-Wstringop-overflow/truncation, -Wrestrict).
Since all these should be adjusted and ideally handled
consistently, duplicating the same computation as above
would be tedious and error-prone. I'd much rather have
a general solution that obviated having to deal with it
at every call site.
Can the math be moved into inform_n (and warning_n) itself?
If that doesn't make sense, then providing a helper for
warning_n and inform_n callers to call might be an alternative.
I CC David for his thoughts on how best to deal with it.
As a separate enhancement, it would also be nice to put in
place a checker (such a post-processing script) to help detect
this problem before it's introduced into the code base.
Martin
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-12-19 Jakub Jelinek <ja...@redhat.com>
* gimple-ssa-sprintf.c (format_directive): Use inform_n instead of
inform with hardcoded english plural handling.
--- gcc/gimple-ssa-sprintf.c.jj 2017-12-14 12:00:32.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c 2017-12-19 12:19:10.975042433 +0100
@@ -2933,13 +2933,15 @@ format_directive (const sprintf_dom_walk
if (warned && fmtres.range.min < fmtres.range.likely
&& fmtres.range.likely < fmtres.range.max)
- {
- inform (info.fmtloc,
- (1 == fmtres.range.likely
- ? G_("assuming directive output of %wu byte")
- : G_("assuming directive output of %wu bytes")),
+ /* Some languages have special plural rules even for large values,
+ but it is periodic with period of 10, 100, 1000 etc. */
+ inform_n (info.fmtloc,
+ fmtres.range.likely > INT_MAX
+ ? (fmtres.range.likely % 1000000) + 1000000
+ : fmtres.range.likely,
+ "assuming directive output of %wu byte",
+ "assuming directive output of %wu bytes",
fmtres.range.likely);
- }
if (warned && fmtres.argmin)
{
Jakub
