From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 95089 invoked by alias); 14 Feb 2018 18:02:19 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 95074 invoked by uid 89); 14 Feb 2018 18:02:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-15.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-ot0-f179.google.com Received: from mail-ot0-f179.google.com (HELO mail-ot0-f179.google.com) (74.125.82.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Feb 2018 18:02:16 +0000 Received: by mail-ot0-f179.google.com with SMTP id 79so963961oth.11 for ; Wed, 14 Feb 2018 10:02:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=mV3esZCGngWk8jvEvwRCRqiHovePgV/6gdEZd+ayEQ0=; b=VnApdJOIiGy46oSqdQSpvj7EO07ZrbdfxrwNcuA0Q+EXl095psr2dEwOK6pv8lBVf/ GcFLYru5cdnn1S1AYEgTE+dVd9iJWzFj9NyJkCPMCCeOmqxVPQAzLyrUgJ3GEtQ+KyxB gEx1h6L/8hlIyKk2zABPID5m24DUKQgwaT6eSlPJPO/CrsqqpRoHlssefx6hMEw2EVWA CXwXFiDOv60xN15O02hN6L77h/of797+JJXvHM0/x/P2HVAzl76aT0IUvzjOc7S9uoed UXNWEm8xlL4miVU8PftrkuTF/ezr3uXNerqv81sO68owEY5mSK5qpVkobFP+7QX5F2lS +pYQ== X-Gm-Message-State: APf1xPCdTOBgf+AK8zQVDfnIJUkjcZ8hKHMTz2SzvVyEUnEODdGjx1N/ 0b8Numb+q2ptHRnSNVdbuEf/gA== X-Google-Smtp-Source: AH8x22582c5DXi8tog5HY2UztI9KR9BMvDUNNSrujxLzuC5e8fxuJt11wmSczF88A/T1U3SbUIaIKg== X-Received: by 10.157.14.12 with SMTP id c12mr33152otc.43.1518631333866; Wed, 14 Feb 2018 10:02:13 -0800 (PST) Received: from localhost.localdomain (75-171-228-29.hlrn.qwest.net. [75.171.228.29]) by smtp.gmail.com with ESMTPSA id r11sm6249215oih.50.2018.02.14.10.02.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Feb 2018 10:02:12 -0800 (PST) Subject: Re: [PATCH] adjust warning_n() to take uhwi (PR 84207) To: Joseph Myers References: <7c200219-2d6e-02ca-e720-9d3a8616a1e6@gmail.com> Cc: Gcc Patch List From: Martin Sebor Message-ID: <6f9fceb8-2f63-3753-7e82-9977630f373f@gmail.com> Date: Wed, 14 Feb 2018 18:02:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------132EE132013C3D691A5B2672" X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00858.txt.bz2 This is a multi-part message in MIME format. --------------132EE132013C3D691A5B2672 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1603 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 --------------132EE132013C3D691A5B2672 Content-Type: text/x-patch; name="gcc-84207.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-84207.diff" Content-length: 9102 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 (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 (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); --------------132EE132013C3D691A5B2672--