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 for c++/57891, narrowing conversions in non-type template arguments


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


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