On Wed, May 23, 2018 at 2:50 PM, Marek Polacek <pola...@redhat.com> wrote:
> On Wed, May 23, 2018 at 12:45:11PM -0400, Jason Merrill wrote:
>> On Wed, May 23, 2018 at 9:46 AM, Marek Polacek <pola...@redhat.com> wrote:
>> > The diagnostic code in build_new{,_1} was using maybe_constant_value to
>> > fold
>> > the array length, but that breaks while parsing a template, because we
>> > might
>> > then leak template codes to the constexpr machinery.
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
>> >
>> > 2018-05-23 Marek Polacek <pola...@redhat.com>
>> >
>> > PR c++/85847
>> > * init.c (build_new_1): Use fold_non_dependent_expr.
>> > (build_new): Likewise.
>> >
>> > * g++.dg/cpp0x/new3.C: New test.
>> >
>> > @@ -2860,7 +2860,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree
>> > type, tree nelts,
>> > /* Lots of logic below. depends on whether we have a constant number of
>> > elements, so go ahead and fold it now. */
>> > if (outer_nelts)
>> > - outer_nelts = maybe_constant_value (outer_nelts);
>> > + outer_nelts = fold_non_dependent_expr (outer_nelts);
>>
>> If outer_nelts is non-constant, this will mean that it ends up
>> instantiated but still non-constant, which can lead to problems when
>> the result is used in building up other expressions.
>>
>> I think we want to put the result of folding in a separate variable
>> for use with things that want to know about a constant size, and keep
>> the original outer_nelts for use in building outer_nelts_check.
>>
>> > /* Try to determine the constant value only for the purposes
>> > of the diagnostic below but continue to use the original
>> > value and handle const folding later. */
>> > - const_tree cst_nelts = maybe_constant_value (nelts);
>> > + const_tree cst_nelts = fold_non_dependent_expr (nelts);
>>
>> ...like we do here.
>
> Like this?
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-05-23 Marek Polacek <pola...@redhat.com>
>
> PR c++/85847
> * init.c (build_new_1): Use fold_non_dependent_expr. Use a dedicated
> variable for its result. Fix a condition.
> (build_new): Use fold_non_dependent_expr. Tweak a condition.
>
> * g++.dg/cpp0x/new3.C: New test.
>
> diff --git gcc/cp/init.c gcc/cp/init.c
> index b558742abf6..cd0110a1e19 100644
> --- gcc/cp/init.c
> +++ gcc/cp/init.c
> @@ -2857,10 +2857,9 @@ build_new_1 (vec<tree, va_gc> **placement, tree type,
> tree nelts,
> outer_nelts_from_type = true;
> }
>
> - /* Lots of logic below. depends on whether we have a constant number of
> + /* Lots of logic below depends on whether we have a constant number of
> elements, so go ahead and fold it now. */
> - if (outer_nelts)
> - outer_nelts = maybe_constant_value (outer_nelts);
> + const_tree cst_outer_nelts = fold_non_dependent_expr (outer_nelts);
>
> /* If our base type is an array, then make sure we know how many elements
> it has. */
> @@ -2912,11 +2911,12 @@ build_new_1 (vec<tree, va_gc> **placement, tree type,
> tree nelts,
> /* Warn if we performed the (T[N]) to T[N] transformation and N is
> variable. */
> if (outer_nelts_from_type
> - && !TREE_CONSTANT (outer_nelts))
> + && cst_outer_nelts != NULL_TREE
> + && !TREE_CONSTANT (cst_outer_nelts))
Why add the comparisons with NULL_TREE? fold_non_dependent_expr only
returns null if its argument is null.
> - pedwarn (EXPR_LOC_OR_LOC (outer_nelts, input_location), OPT_Wvla,
> + pedwarn (EXPR_LOC_OR_LOC (cst_outer_nelts, input_location),
> OPT_Wvla,
Let's drop this change, the original expression has the location we want.
Jason
