Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

[Jason Merrill <jason at redhat dot com>]

On Fri, Jun 29, 2018 at 3:58 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Jun 27, 2018 at 07:35:15PM -0400, Jason Merrill wrote:
>> On Wed, Jun 27, 2018 at 12:53 PM, Marek Polacek <polacek@redhat.com> wrote:
>> > This PR complains about us accepting invalid code like
>> >
>> >   template<unsigned int> struct A {};
>> >   A<-1> a;
>> >
>> > Where we should detect the narrowing: [temp.arg.nontype] says
>> > "A template-argument for a non-type template-parameter shall be a converted
>> > constant expression ([expr.const]) of the type of the template-parameter."
>> > and a converted constant expression can contain only
>> > - integral conversions other than narrowing conversions,
>> > - [...]."
>> > It spurred e.g.
>> > <https://stackoverflow.com/questions/28184888/how-implicit-conversion-works-for-non-type-template-parameters>
>> > and has >=3 dups so it has some visibility.
>> >
>> > I think build_converted_constant_expr needs to set check_narrowing.
>> > check_narrowing also always mentions that it's in { } but that is no longer
>> > true; in the future it will also apply to <=>.  We'd probably have to add a new
>> > flag to struct conversion if wanted to distinguish between these.
>> >
>> > This does not yet fix detecting narrowing in function templates (78244).
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >
>> > 2018-06-27  Marek Polacek  <polacek@redhat.com>
>> >
>> >         PR c++/57891
>> >         * call.c (build_converted_constant_expr): Set check_narrowing.
>> >         * decl.c (compute_array_index_type): Add warning sentinel.  Use
>> >         input_location.
>> >         * pt.c (convert_nontype_argument): Return NULL_TREE if any errors
>> >         were reported.
>> >         * typeck2.c (check_narrowing): Don't mention { } in diagnostic.
>> >
>> >         * g++.dg/cpp0x/Wnarrowing6.C: New test.
>> >         * g++.dg/cpp0x/Wnarrowing7.C: New test.
>> >         * g++.dg/cpp0x/Wnarrowing8.C: New test.
>> >         * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
>> >         * g++.dg/init/new43.C: Adjust dg-error.
>> >         * g++.dg/other/fold1.C: Likewise.
>> >         * g++.dg/parse/array-size2.C: Likewise.
>> >         * g++.dg/other/vrp1.C: Add dg-error.
>> >         * g++.dg/template/char1.C: Likewise.
>> >         * g++.dg/ext/builtin12.C: Likewise.
>> >         * g++.dg/template/dependent-name3.C: Adjust dg-error.
>> >
>> > diff --git gcc/cp/call.c gcc/cp/call.c
>> > index 209c1fd2f0e..956c7b149dc 100644
>> > --- gcc/cp/call.c
>> > +++ gcc/cp/call.c
>> > @@ -4152,7 +4152,10 @@ build_converted_constant_expr (tree type, tree expr, tsubst_flags_t complain)
>> >      }
>> >
>> >    if (conv)
>> > -    expr = convert_like (conv, expr, complain);
>> > +    {
>> > +      conv->check_narrowing = !processing_template_decl;
>>
>> Why !processing_template_decl?  This needs a comment.
>
> Otherwise we'd warn for e.g.
>
> template<int N> struct S { char a[N]; };
> S<1> s;
>
> where compute_array_index_type will try to convert the size of the array (which
> is a template_parm_index of type int when parsing the template) to size_type.
> So I guess I can say that we need to wait for instantiation?

We certainly shouldn't give a narrowing diagnostic about a
value-dependent expression.  It probably makes sense to check that at
the top of check_narrowing, with all the other early exit conditions.
But if we do know the constant value in the template, it's good to
complain then rather than wait for instantiation.

Jason