Re: [C++ PATCH] Plural forms for count != eltscnt structured binding diagnostics

[Pedro Alves]

On 11/30/2017 01:10 PM, Jakub Jelinek wrote:
> On Thu, Nov 30, 2017 at 01:33:48PM +0100, Jakub Jelinek wrote:
>> On Thu, Nov 30, 2017 at 01:01:58PM +0100, Jakub Jelinek wrote:
>>> I just followed:
>>> https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Plural-forms.html
>>>
>>> "About larger integer types, such as ‘uintmax_t’ or ‘unsigned long long’: 
>>> they can be
>>> handled by reducing the value to a range that fits in an ‘unsigned long’. 
>>> Simply
>>> casting the value to ‘unsigned long’ would not do the right thing, since it 
>>> would
>>> treat ULONG_MAX + 1 like zero, ULONG_MAX + 2 like singular, and the like. 
>>> Here you
>>> can exploit the fact that all mentioned plural form formulas eventually 
>>> become periodic,
>>> with a period that is a divisor of 100 (or 1000 or 100). So, when you 
>>> reduce a large
>>> value to another one in the range [100, 199] that ends in the same 
>>> 6 decimal
>>> digits, you can assume that it will lead to the same plural form selection. 
>>> This code
>>> does this:
>>>
>>> #include 
>>> uintmax_t nbytes = ...;
>>> printf (ngettext ("The file has %"PRIuMAX" byte.",
>>>   "The file has %"PRIuMAX" bytes.",
>>>   (nbytes > ULONG_MAX
>>>? (nbytes % 100) + 100
>>>: nbytes)),
>>> nbytes);"
>>>
>>> I can surely add a comment about that.

How about wrapping it in a function to make it self-describing?
Something around:

/* Comment/url here.  */

unsigned long
plural_form_for (unsigned HOST_WIDE_INT val)
{
  return (val > ULONG_MAX
  ? (val % 100) + 100
  : val);
}

and then:

  inform_n (loc, plural_form_for (eltscnt),
"while %qT decomposes into %wu element",
"while %qT decomposes into %wu elements",
type, eltscnt);

Pedro Alves

Re: [C++ PATCH] Plural forms for count != eltscnt structured binding diagnostics

[Jakub Jelinek]

On Thu, Nov 30, 2017 at 01:33:48PM +0100, Jakub Jelinek wrote:
> On Thu, Nov 30, 2017 at 01:01:58PM +0100, Jakub Jelinek wrote:
> > I just followed:
> > https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Plural-forms.html
> > 
> > "About larger integer types, such as ‘uintmax_t’ or ‘unsigned long long’: 
> > they can be
> > handled by reducing the value to a range that fits in an ‘unsigned long’. 
> > Simply
> > casting the value to ‘unsigned long’ would not do the right thing, since it 
> > would
> > treat ULONG_MAX + 1 like zero, ULONG_MAX + 2 like singular, and the like. 
> > Here you
> > can exploit the fact that all mentioned plural form formulas eventually 
> > become periodic,
> > with a period that is a divisor of 100 (or 1000 or 100). So, when you 
> > reduce a large
> > value to another one in the range [100, 199] that ends in the same 
> > 6 decimal
> > digits, you can assume that it will lead to the same plural form selection. 
> > This code
> > does this:
> > 
> > #include 
> > uintmax_t nbytes = ...;
> > printf (ngettext ("The file has %"PRIuMAX" byte.",
> >   "The file has %"PRIuMAX" bytes.",
> >   (nbytes > ULONG_MAX
> >? (nbytes % 100) + 100
> >: nbytes)),
> > nbytes);"
> > 
> > I can surely add a comment about that.
> > 
> > Note the patch depends on the 
> > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02521.html
> > patch.
> 
> Though, looking at our *_n diagnostic routines, the n argument is actually
> int there, not unsigned long for some reason.  So, either we should fix
> that (seems ngettext has unsigned long), or the above would need to be
> (unsigned HOST_WIDE_INT) (int) eltscnt != eltscnt || (int) eltscnt < 0 

Of course better eltscnt > INT_MAX

Jakub

Re: [C++ PATCH] Plural forms for count != eltscnt structured binding diagnostics

[Jakub Jelinek]

On Thu, Nov 30, 2017 at 01:01:58PM +0100, Jakub Jelinek wrote:
> I just followed:
> https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Plural-forms.html
> 
> "About larger integer types, such as ‘uintmax_t’ or ‘unsigned long long’: 
> they can be
> handled by reducing the value to a range that fits in an ‘unsigned long’. 
> Simply
> casting the value to ‘unsigned long’ would not do the right thing, since it 
> would
> treat ULONG_MAX + 1 like zero, ULONG_MAX + 2 like singular, and the like. 
> Here you
> can exploit the fact that all mentioned plural form formulas eventually 
> become periodic,
> with a period that is a divisor of 100 (or 1000 or 100). So, when you 
> reduce a large
> value to another one in the range [100, 199] that ends in the same 6 
> decimal
> digits, you can assume that it will lead to the same plural form selection. 
> This code
> does this:
> 
> #include 
> uintmax_t nbytes = ...;
> printf (ngettext ("The file has %"PRIuMAX" byte.",
>   "The file has %"PRIuMAX" bytes.",
>   (nbytes > ULONG_MAX
>? (nbytes % 100) + 100
>: nbytes)),
> nbytes);"
> 
> I can surely add a comment about that.
> 
> Note the patch depends on the 
> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02521.html
> patch.

Though, looking at our *_n diagnostic routines, the n argument is actually
int there, not unsigned long for some reason.  So, either we should fix
that (seems ngettext has unsigned long), or the above would need to be
(unsigned HOST_WIDE_INT) (int) eltscnt != eltscnt || (int) eltscnt < 0 

Jakub

Re: [C++ PATCH] Plural forms for count != eltscnt structured binding diagnostics

[Jakub Jelinek]

On Thu, Nov 30, 2017 at 06:55:08AM -0500, Nathan Sidwell wrote:
> > --- gcc/cp/decl.c.jj2017-11-30 09:44:19.0 +0100
> > +++ gcc/cp/decl.c   2017-11-30 09:57:44.539504854 +0100
> > @@ -7445,11 +7445,18 @@ cp_finish_decomp (tree decl, tree first,
> 
> > + inform_n (loc, eltscnt != (unsigned long) eltscnt
> > +? (eltscnt % 100) + 100 : eltscnt,
> 
> Is such elaboration with the modulo operator necessary? wouldn;t an
> arbitrary non-unity constant do.  (It took me a while to figure out what
> this was trying to do.  At least a comment?)

For english it isn't needed of course.

I just followed:
https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Plural-forms.html

"About larger integer types, such as ‘uintmax_t’ or ‘unsigned long long’: they 
can be
handled by reducing the value to a range that fits in an ‘unsigned long’. Simply
casting the value to ‘unsigned long’ would not do the right thing, since it 
would
treat ULONG_MAX + 1 like zero, ULONG_MAX + 2 like singular, and the like. Here 
you
can exploit the fact that all mentioned plural form formulas eventually become 
periodic,
with a period that is a divisor of 100 (or 1000 or 100). So, when you 
reduce a large
value to another one in the range [100, 199] that ends in the same 6 
decimal
digits, you can assume that it will lead to the same plural form selection. 
This code
does this:

#include 
uintmax_t nbytes = ...;
printf (ngettext ("The file has %"PRIuMAX" byte.",
  "The file has %"PRIuMAX" bytes.",
  (nbytes > ULONG_MAX
   ? (nbytes % 100) + 100
   : nbytes)),
nbytes);"

I can surely add a comment about that.

Note the patch depends on the 
https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02521.html
patch.

Jakub

Re: [C++ PATCH] Plural forms for count != eltscnt structured binding diagnostics

[Nathan Sidwell]

On 11/30/2017 04:18 AM, Jakub Jelinek wrote:


Here is an incremental patch to deal with that on top of the previously
posted patch, ok for trunk if it passes testing?  Note, not using inform_n
in this !tree_fits_uhwi_p case, because gettext doesn't recommend that for
negative values and the other values printed there (the very large ones) are
printed using hexadecimal where also I think human plural forms are rarely
useful.

2017-11-30  Jakub Jelinek  

* decl.c (cp_finish_decomp): Split up count != eltscnt and
!tree_fits_uhwi_p (tsize) error_at calls into error_n and inform_n
to handle plural forms properly.

* g++.dg/cpp1z/decomp3.C: Adjust for structured binding count
mismatch diagnostics split into error and warning with plural
forms.
* g++.dg/cpp1z/decomp10.C: Likewise.
* g++.dg/cpp1z/decomp32.C: Likewise.


Ok.


--- gcc/cp/decl.c.jj2017-11-30 09:44:19.0 +0100
+++ gcc/cp/decl.c   2017-11-30 09:57:44.539504854 +0100
@@ -7445,11 +7445,18 @@ cp_finish_decomp (tree decl, tree first,



+ inform_n (loc, eltscnt != (unsigned long) eltscnt
+? (eltscnt % 100) + 100 : eltscnt,


Is such elaboration with the modulo operator necessary? wouldn;t an 
arbitrary non-unity constant do.  (It took me a while to figure out what 
this was trying to do.  At least a comment?)


nathan

--
Nathan Sidwell

[C++ PATCH] Plural forms for count != eltscnt structured binding diagnostics

[Jakub Jelinek]

On Wed, Nov 29, 2017 at 06:19:04PM -0700, Martin Sebor wrote:
> > --- gcc/cp/decl.c.jj2017-11-28 22:23:34.0 +0100
> > +++ gcc/cp/decl.c   2017-11-29 15:00:24.487658715 +0100
> > @@ -7518,6 +7518,12 @@ cp_finish_decomp (tree decl, tree first,
> >  "constant expression", type);
> >   goto error_out;
> > }
> > +  if (!tree_fits_uhwi_p (tsize))
> > +   {
> > + error_at (loc, "%u names provided while %qT decomposes into "
> 
> When count is 1 as in the test below the error isn't grammatically
> correct ("1 names").  I see that the same message is already issued
> elsewhere in the function so this seems like an opportunity to use
> the right form here and also fix the other one at the same time or
> in a followup.  The error_n function exists to issue the right form
> for the language, singular or plural.  It's not as convenient when
> the sentence contains two terms that may be singular or plural,
> but that can also be dealt with.

Here is an incremental patch to deal with that on top of the previously
posted patch, ok for trunk if it passes testing?  Note, not using inform_n
in this !tree_fits_uhwi_p case, because gettext doesn't recommend that for
negative values and the other values printed there (the very large ones) are
printed using hexadecimal where also I think human plural forms are rarely
useful.

2017-11-30  Jakub Jelinek  

* decl.c (cp_finish_decomp): Split up count != eltscnt and
!tree_fits_uhwi_p (tsize) error_at calls into error_n and inform_n
to handle plural forms properly.

* g++.dg/cpp1z/decomp3.C: Adjust for structured binding count
mismatch diagnostics split into error and warning with plural
forms.
* g++.dg/cpp1z/decomp10.C: Likewise.
* g++.dg/cpp1z/decomp32.C: Likewise.

--- gcc/cp/decl.c.jj2017-11-30 09:44:19.0 +0100
+++ gcc/cp/decl.c   2017-11-30 09:57:44.539504854 +0100
@@ -7445,11 +7445,18 @@ cp_finish_decomp (tree decl, tree first,
{
cnt_mismatch:
  if (count > eltscnt)
-   error_at (loc, "%u names provided while %qT decomposes into "
-  "%wu elements", count, type, eltscnt);
+   error_n (loc, count,
+"%u name provided for structured binding",
+"%u names provided for structured binding", count);
  else
-   error_at (loc, "only %u names provided while %qT decomposes into "
-  "%wu elements", count, type, eltscnt);
+   error_n (loc, count,
+"only %u name provided for structured binding",
+"only %u names provided for structured binding", count);
+ inform_n (loc, eltscnt != (unsigned long) eltscnt
+? (eltscnt % 100) + 100 : eltscnt,
+   "while %qT decomposes into %wu element",
+   "while %qT decomposes into %wu elements",
+   type, eltscnt);
  goto error_out;
}
   eltype = TREE_TYPE (type);
@@ -7520,8 +7527,11 @@ cp_finish_decomp (tree decl, tree first,
}
   if (!tree_fits_uhwi_p (tsize))
{
- error_at (loc, "%u names provided while %qT decomposes into "
-"%E elements", count, type, tsize);
+ error_n (loc, count,
+  "%u name provided for structured binding",
+  "%u names provided for structured binding", count);
+ inform (loc, "while %qT decomposes into %E elements",
+ type, tsize);
  goto error_out;
}
   eltscnt = tree_to_uhwi (tsize);
--- gcc/testsuite/g++.dg/cpp1z/decomp3.C.jj 2017-09-15 18:11:04.0 
+0200
+++ gcc/testsuite/g++.dg/cpp1z/decomp3.C2017-11-30 10:09:42.756619330 
+0100
@@ -51,16 +51,21 @@ int arr[4];
 void
 test3 (A , B c)
 {
-  auto [ d, e, f ] = arr;  // { dg-error "only 3 names provided 
while 'int .4.' decomposes into 4 elements" }
-   // { dg-warning "structured bindings 
only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-1 }
-  auto & [ g, h, i, j, k ] = arr;  // { dg-error "5 names provided while 
'int .4.' decomposes into 4 elements" }
-   // { dg-warning "structured bindings 
only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-1 }
-  auto [ l, m ] = b;   // { dg-error "only 2 names provided 
while 'A' decomposes into 3 elements" }
-   // { dg-warning "structured bindings 
only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-1 }
-  auto & [ n, o, p, q ] = b;   // { dg-error "4 names provided while 
'A' decomposes into 3 elements" }
-   // { dg-warning "structured bindings 
only available with -std=c..17 or -std=gnu..17"