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

[Jason Merrill <jason@redhat.com>]

On Mon, Aug 13, 2018 at 11:24 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Mon, Aug 13, 2018 at 10:14:21PM +1200, Jason Merrill wrote:
>> >> >> > --- gcc/cp/decl.c
>> >> >> > +++ gcc/cp/decl.c
>> >> >> > @@ -9581,7 +9581,7 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain)
>> >> >> >      {
>> >> >> >        tree folded = cp_fully_fold (size);
>> >> >> >        if (TREE_CODE (folded) == INTEGER_CST)
>> >> >> > -       pedwarn (location_of (size), OPT_Wpedantic,
>> >> >> > +       pedwarn (input_location, OPT_Wpedantic,
>> >> >>
>> >> >> It should work to use location_of (osize) here.
>> >> >
>> >> > I dropped this hunk altogether.  Because location_of will use
>> >> > DECL_SOURCE_LOCATION for DECLs, the error message will point to the declaration
>> >> > itself, not the use.  I don't really care either way.
>> >>
>> >> We want the message to point to the use, which location_of (osize)
>> >> will provide, since it should still have a location wrapper around a
>> >> DECL.
>> >
>> > location_of (osize) is actually the same as location_of (size) so that didn't
>> > change anything.
>>
>> Hunh, that's strange.  Why isn't osize the unfolded expression?  Where
>> is the location wrapper getting stripped?
>
> I actually see that it didn't have the location wrapper at the start.
> The array bound is parsed in cp_parser_direct_new_declarator, and we
> never called maybe_wrap_with_location to add the wrapper.  I don't know
> where that's supposed to happen.
>
> This quick hack works
>
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -8681,6 +8681,7 @@ cp_parser_direct_new_declarator (cp_parser* parser)
>        cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
>
>        /* Add this bound to the declarator.  */
> +      expression = maybe_wrap_with_location (expression, token->location);
>        declarator = make_array_declarator (declarator, expression);
>
>        /* If the next token is not a `[', then there are no more
>
> but that feels too ad-hoc and beyond the scope of this fix.
>
>> > The code below uses input_location which is why I went with
>> > it in the first place.  So, should I change this to input_location?
>>
>> I suppose so.
>
> Here's the version with input_location.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-08-13  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/57891
>         * call.c (struct conversion): Add check_narrowing_const_only.
>         (build_converted_constant_expr): Set check_narrowing and
>         check_narrowing_const_only.  Give error if expr is error node.
>         (convert_like_real): Pass it to check_narrowing.
>         * cp-tree.h (check_narrowing): Add a default parameter.
>         * decl.c (compute_array_index_type): Use input_location instead of
>         location_of.
>         * pt.c (convert_nontype_argument): Return NULL_TREE if tf_error.
>         * typeck2.c (check_narrowing): Don't warn for instantiation-dependent
>         expressions.  Call maybe_constant_value instead of
>         fold_non_dependent_expr.  Don't mention { } in diagnostic.  Only check
>         narrowing for constants if CONST_ONLY.

This seems to have broken cpp1z/direct-enum-init1.C with -std=c++17
and above; please use make check-c++-all (or set
GXX_TESTSUITE_STDS=98,11,14,17,2a) to test C++ patches in all
conformance modes.

Jason