This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++ PATCH] Fix ICE on invalid std::tuple_size<...>::value (PR c++/83205)


OK.

On Fri, Dec 15, 2017 at 2:46 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Dec 15, 2017 at 08:09:20PM +0100, Jakub Jelinek wrote:
>> On Fri, Dec 15, 2017 at 02:01:36PM -0500, Jason Merrill wrote:
>> > On 11/29/2017 08:19 PM, Martin Sebor wrote:
>> > > On 11/29/2017 03:32 PM, Jakub Jelinek wrote:
>> > > > +      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.
>> >
>> > Agreed.
>>
>> Yeah, I've implemented it as an incremental patch.
>> So
>> http://gcc.gnu.org/ml/gcc-patches/2017-11/msg02521.html
>> for the ICE and
>> http://gcc.gnu.org/ml/gcc-patches/2017-11/msg02538.html
>> on top of it.  The latter is what Nathan approved already, the former
>> needs review.
>
> If it helps any, here are the 2 patches combined, re-tested on x86_64-linux
> with check-c++-all.
>
> 2017-12-15  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/83205
>         * decl.c (cp_finish_decomp): Handle the case when tsize is not
>         error_mark_node, but doesn't fit into uhwi.  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: New test.
>
> --- gcc/cp/decl.c.jj    2017-12-15 20:40:00.601221086 +0100
> +++ gcc/cp/decl.c       2017-12-15 20:42:25.917445216 +0100
> @@ -7427,11 +7427,20 @@ 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);
> +         /* Some languages have special plural rules even for large values,
> +            but it is periodic with period of 10, 100, 1000 etc.  */
> +         inform_n (loc, eltscnt > INT_MAX
> +                        ? (eltscnt % 1000000) + 1000000 : eltscnt,
> +                   "while %qT decomposes into %wu element",
> +                   "while %qT decomposes into %wu elements",
> +                   type, eltscnt);
>           goto error_out;
>         }
>        eltype = TREE_TYPE (type);
> @@ -7500,6 +7509,15 @@ cp_finish_decomp (tree decl, tree first,
>                          "constant expression", type);
>           goto error_out;
>         }
> +      if (!tree_fits_uhwi_p (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);
>        if (count != eltscnt)
>         goto cnt_mismatch;
> --- gcc/testsuite/g++.dg/cpp1z/decomp3.C.jj     2017-11-30 11:18:00.078805693 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp3.C        2017-12-15 20:42:25.917445216 +0100
> @@ -51,16 +51,21 @@ int arr[4];
>  void
>  test3 (A &b, 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" "" { target c++14_down } .-1 }
> +  auto [ d, e, f ] = arr;              // { dg-error "only 3 names provided" }
> +                                       // { dg-message "while 'int .4.' decomposes into 4 elements" "" { target *-*-* } .-1 }
> +                                       // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-2 }
> +  auto & [ g, h, i, j, k ] = arr;      // { dg-error "5 names provided" }
> +                                       // { dg-message "while 'int .4.' decomposes into 4 elements" "" { target *-*-* } .-1 }
> +                                       // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-2 }
> +  auto [ l, m ] = b;                   // { dg-error "only 2 names provided" }
> +                                       // { dg-message "while 'A' decomposes into 3 elements" "" { target *-*-* } .-1 }
> +                                       // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-2 }
> +  auto & [ n, o, p, q ] = b;           // { dg-error "4 names provided" }
> +                                       // { dg-message "while 'A' decomposes into 3 elements" "" { target *-*-* } .-1 }
> +                                       // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-2 }
>    auto [] { c };                       // { dg-error "empty structured binding declaration" }
>                                         // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-1 }
> -  auto [ r, s ] = c;                   // { dg-error "2 names provided while 'B' decomposes into 1 elements" }
> -                                       // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-1 }
> +  auto [ r, s ] = c;                   // { dg-error "2 names provided" }
> +                                       // { dg-message "while 'B' decomposes into 1 element" "" { target *-*-* } .-1 }
> +                                       // { dg-warning "structured bindings only available with -std=c..17 or -std=gnu..17" "" { target c++14_down } .-2 }
>  }
> --- gcc/testsuite/g++.dg/cpp1z/decomp10.C.jj    2017-11-30 11:18:00.102805393 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp10.C       2017-12-15 20:42:25.918445204 +0100
> @@ -11,7 +11,8 @@ void f1() { auto [ x ] = a1; }        // { dg-e
>
>  struct A2 { int i,j; } a2;
>  template<> struct std::tuple_size<A2> { enum { value = 5 }; };
> -void f2() { auto [ x ] = a2; } // { dg-error "decomposes into 5" }
> +void f2() { auto [ x ] = a2; } // { dg-error "only 1 name provided" }
> +                               // { dg-message "decomposes into 5" "" { target *-*-* } .-1 }
>
>  struct A3 { int i,j; } a3;
>  template<> struct std::tuple_size<A3> { enum { value = 1 }; };
> --- gcc/testsuite/g++.dg/cpp1z/decomp32.C.jj    2017-12-15 20:42:58.977041204 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp32.C       2017-12-15 20:42:25.918445204 +0100
> @@ -0,0 +1,32 @@
> +// PR c++/83205
> +// { dg-do compile { target c++11 } }
> +// { dg-options "" }
> +
> +struct A { int i; };
> +struct B { int i; };
> +namespace std {
> +  template <typename T> struct tuple_size;
> +  template <> struct tuple_size<A> {
> +    static constexpr int value = -1;
> +  };
> +#ifdef __SIZEOF_INT128__
> +  template <> struct tuple_size<B> {
> +    static constexpr unsigned __int128 value = -1;
> +  };
> +#endif
> +}
> +
> +auto [a] = A{};        // { dg-error "1 name provided" }
> +               // { dg-message "while 'A' decomposes into -1 elements" "" { target *-*-* } .-1 }
> +               // { dg-warning "structured bindings only available with" "" { target c++14_down } .-2 }
> +#ifdef __SIZEOF_INT128__
> +auto [b] = B{};        // { dg-error "1 name provided" "" { target int128 } }
> +               // { dg-message "while 'B' decomposes into \[0-9xa-fXA-F]* elements" "" { target int128 } .-1 }
> +               // { dg-warning "structured bindings only available with" "" { target { c++14_down && int128 } } .-2 }
> +auto [c, d] = B{};     // { dg-error "2 names provided" "" { target int128 } }
> +                       // { dg-message "while 'B' decomposes into \[0-9xa-fXA-F]* elements" "" { target int128 } .-1 }
> +                       // { dg-warning "structured bindings only available with" "" { target { c++14_down && int128 } } .-2 }
> +#endif
> +auto [e, f, g] = A{};  // { dg-error "3 names provided" }
> +                       // { dg-message "while 'A' decomposes into -1 elements" "" { target *-*-* } .-1 }
> +                       // { dg-warning "structured bindings only available with" "" { target c++14_down } .-2 }
>
>
>         Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]