On 02/13/2018 02:05 PM, Joseph Myers wrote:
On Mon, 12 Feb 2018, Martin Sebor wrote:
Bug 84207 - Hard coded plural in gimple-fold.c points out one
of a number of warning_at() calls where warning_n() should have
been used. The attached patch both replaces the calls and also
changes the signatures of the warning_n(), error_n(), and
inform_n() functions to take an unsigned HOST_WIDE_INT argument
instead of int. I also changed the implementation of
diagnostic_n_impl() to deal with unsigned HOST_WIDE_INT values
in excess of ULONG_MAX (the maximum value ngettext handles) so
callers don't need to.
Saturating to ULONG_MAX is not correct for languages where the plural form
depends on n%10 or n%100 (see the various Plural-Forms entries in the .po
files). If n is too large you want something like n % 1000000 + 1000000
instead to get the correct plural form in all cases.
Thanks. I've made that change in the attached patch.
I was also hoping to test it, either now if it's easy, or if
it's complicated, sometime in the future but I couldn't find
a .po file where it would make a difference. I could have
easily missed one but none of those I've looked seems to do
much with the plural forms where such large numbers could
come up. The strings are either all empty or all look
the same. Do you happen to know of one where it matters
and a suggestion for how to test it? I suppose I could
create a dummy .po file with a non-trivial Plural-Forms but
then how would I plug it into GCC to verify (in an automated
test) that the right form is used?
Martin
PR translation/84207 - Hard coded plural in gimple-fold.c
gcc/ChangeLog:
PR translation/84207
* diagnostic-core.h (warning_n, error_n, inform_n): Change
n argument to unsigned HOST_WIDE_INT.
* diagnostic.c (warning_n, error_n, inform_n): Ditto.
(diagnostic_n_impl): Ditto. Handle arguments in excess of LONG_MAX.
* gimple-fold.c (gimple_fold_builtin_strncpy): Use warning_n.
* gimple-ssa-sprintf.c (format_directive): Simplify inform_n call.
Index: gcc/diagnostic-core.h
===================================================================
--- gcc/diagnostic-core.h (revision 257665)
+++ gcc/diagnostic-core.h (working copy)
@@ -59,10 +59,11 @@ extern void internal_error_no_backtrace (const cha
ATTRIBUTE_GCC_DIAG(1,2) ATTRIBUTE_NORETURN;
/* Pass one of the OPT_W* from options.h as the first parameter. */
extern bool warning (int, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
-extern bool warning_n (location_t, int, int, const char *, const char *, ...)
+extern bool warning_n (location_t, int, unsigned HOST_WIDE_INT,
+ const char *, const char *, ...)
ATTRIBUTE_GCC_DIAG(4,6) ATTRIBUTE_GCC_DIAG(5,6);
-extern bool warning_n (rich_location *, int, int, const char *,
- const char *, ...)
+extern bool warning_n (rich_location *, int, unsigned HOST_WIDE_INT,
+ const char *, const char *, ...)
ATTRIBUTE_GCC_DIAG(4, 6) ATTRIBUTE_GCC_DIAG(5, 6);
extern bool warning_at (location_t, int, const char *, ...)
ATTRIBUTE_GCC_DIAG(3,4);
@@ -69,7 +70,8 @@ extern bool warning_at (location_t, int, const cha
extern bool warning_at (rich_location *, int, const char *, ...)
ATTRIBUTE_GCC_DIAG(3,4);
extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
-extern void error_n (location_t, int, const char *, const char *, ...)
+extern void error_n (location_t, unsigned HOST_WIDE_INT, const char *,
+ const char *, ...)
ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
extern void error_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
extern void error_at (rich_location *, const char *, ...)
@@ -87,7 +89,8 @@ extern bool permerror (rich_location *, const char
extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
extern void inform (rich_location *, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
-extern void inform_n (location_t, int, const char *, const char *, ...)
+extern void inform_n (location_t, unsigned HOST_WIDE_INT, const char *,
+ const char *, ...)
ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
extern void verbatim (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
extern bool emit_diagnostic (diagnostic_t, location_t, int,
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c (revision 257665)
+++ gcc/diagnostic.c (working copy)
@@ -51,8 +51,8 @@ along with GCC; see the file COPYING3. If not see
/* Prototypes. */
static bool diagnostic_impl (rich_location *, int, const char *,
va_list *, diagnostic_t) ATTRIBUTE_GCC_DIAG(3,0);
-static bool diagnostic_n_impl (rich_location *, int, int, const char *,
- const char *, va_list *,
+static bool diagnostic_n_impl (rich_location *, int, unsigned HOST_WIDE_INT,
+ const char *, const char *, va_list *,
diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
static void error_recursion (diagnostic_context *) ATTRIBUTE_NORETURN;
@@ -1111,15 +1111,24 @@ diagnostic_impl (rich_location *richloc, int opt,
/* Implement inform_n, warning_n, and error_n, as documented and
defined below. */
static bool
-diagnostic_n_impl (rich_location *richloc, int opt, int n,
+diagnostic_n_impl (rich_location *richloc, int opt, unsigned HOST_WIDE_INT n,
const char *singular_gmsgid,
const char *plural_gmsgid,
va_list *ap, diagnostic_t kind)
{
diagnostic_info diagnostic;
- diagnostic_set_info_translated (&diagnostic,
- ngettext (singular_gmsgid, plural_gmsgid, n),
- ap, richloc, kind);
+ unsigned long gtn;
+
+ if (sizeof n <= sizeof gtn)
+ gtn = n;
+ else
+ /* Use the largest number ngettext() can handle, otherwise
+ preserve the six least significant decimal digits for
+ languages where the plural form depends on them. */
+ gtn = n <= ULONG_MAX ? n : n % 1000000LU + 1000000LU;
+
+ const char *text = ngettext (singular_gmsgid, plural_gmsgid, gtn);
+ diagnostic_set_info_translated (&diagnostic, text, ap, richloc, kind);
if (kind == DK_WARNING)
diagnostic.option_index = opt;
return diagnostic_report_diagnostic (global_dc, &diagnostic);
@@ -1176,8 +1185,8 @@ inform (rich_location *richloc, const char *gmsgid
/* An informative note at LOCATION. Use this for additional details on an
error message. */
void
-inform_n (location_t location, int n, const char *singular_gmsgid,
- const char *plural_gmsgid, ...)
+inform_n (location_t location, unsigned HOST_WIDE_INT n,
+ const char *singular_gmsgid, const char *plural_gmsgid, ...)
{
va_list ap;
va_start (ap, plural_gmsgid);
@@ -1233,7 +1242,7 @@ warning_at (rich_location *richloc, int opt, const
/* Same as warning_n plural variant below, but using RICHLOC. */
bool
-warning_n (rich_location *richloc, int opt, int n,
+warning_n (rich_location *richloc, int opt, unsigned HOST_WIDE_INT n,
const char *singular_gmsgid, const char *plural_gmsgid, ...)
{
gcc_assert (richloc);
@@ -1252,8 +1261,8 @@ bool
Returns true if the warning was printed, false if it was inhibited. */
bool
-warning_n (location_t location, int opt, int n, const char *singular_gmsgid,
- const char *plural_gmsgid, ...)
+warning_n (location_t location, int opt, unsigned HOST_WIDE_INT n,
+ const char *singular_gmsgid, const char *plural_gmsgid, ...)
{
va_list ap;
va_start (ap, plural_gmsgid);
@@ -1350,8 +1359,8 @@ error (const char *gmsgid, ...)
/* A hard error: the code is definitely ill-formed, and an object file
will not be produced. */
void
-error_n (location_t location, int n, const char *singular_gmsgid,
- const char *plural_gmsgid, ...)
+error_n (location_t location, unsigned HOST_WIDE_INT n,
+ const char *singular_gmsgid, const char *plural_gmsgid, ...)
{
va_list ap;
va_start (ap, plural_gmsgid);
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c (revision 257665)
+++ gcc/gimple-fold.c (working copy)
@@ -1700,13 +1700,13 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator
tree fndecl = gimple_call_fndecl (stmt);
gcall *call = as_a <gcall *> (stmt);
- warning_at (loc, OPT_Wstringop_truncation,
- (tree_int_cst_equal (size_one_node, len)
- ? G_("%G%qD output truncated copying %E byte "
- "from a string of length %E")
- : G_("%G%qD output truncated copying %E bytes "
- "from a string of length %E")),
- call, fndecl, len, slen);
+ warning_n (loc, OPT_Wstringop_truncation,
+ tree_to_uhwi (len),
+ "%G%qD output truncated copying %E byte "
+ "from a string of length %E",
+ "%G%qD output truncated copying %E bytes "
+ "from a string of length %E",
+ call, fndecl, len, slen);
}
else if (tree_int_cst_equal (len, slen))
{
@@ -1713,15 +1713,15 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator
tree fndecl = gimple_call_fndecl (stmt);
gcall *call = as_a <gcall *> (stmt);
- warning_at (loc, OPT_Wstringop_truncation,
- (tree_int_cst_equal (size_one_node, len)
- ? G_("%G%qD output truncated before terminating nul "
- "copying %E byte from a string of the same "
- "length")
- : G_("%G%qD output truncated before terminating nul "
- "copying %E bytes from a string of the same "
- "length")),
- call, fndecl, len);
+ warning_n (loc, OPT_Wstringop_truncation,
+ tree_to_uhwi (len),
+ "%G%qD output truncated before terminating nul "
+ "copying %E byte from a string of the same "
+ "length",
+ "%G%qD output truncated before terminating nul "
+ "copying %E bytes from a string of the same "
+ "length",
+ call, fndecl, len);
}
}
Index: gcc/gimple-ssa-sprintf.c
===================================================================
--- gcc/gimple-ssa-sprintf.c (revision 257665)
+++ gcc/gimple-ssa-sprintf.c (working copy)
@@ -2937,10 +2937,7 @@ format_directive (const sprintf_dom_walker::call_i
&& fmtres.range.likely < fmtres.range.max)
/* 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,
+ inform_n (info.fmtloc, fmtres.range.likely,
"assuming directive output of %wu byte",
"assuming directive output of %wu bytes",
fmtres.range.likely);
