* C++ PATCH for c++/85847, ICE with template_id_expr in new()
@ 2018-05-23 13:57 Marek Polacek
2018-05-23 16:45 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2018-05-23 13:57 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
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 <polacek@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.
diff --git gcc/cp/init.c gcc/cp/init.c
index b558742abf6..d96fec46f65 100644
--- gcc/cp/init.c
+++ gcc/cp/init.c
@@ -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 our base type is an array, then make sure we know how many elements
it has. */
@@ -3639,7 +3639,7 @@ build_new (vec<tree, va_gc> **placement, tree type, tree nelts,
/* 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);
/* The expression in a noptr-new-declarator is erroneous if it's of
non-class type and its value before converting to std::size_t is
diff --git gcc/testsuite/g++.dg/cpp0x/new3.C gcc/testsuite/g++.dg/cpp0x/new3.C
index e69de29bb2d..c388acf552e 100644
--- gcc/testsuite/g++.dg/cpp0x/new3.C
+++ gcc/testsuite/g++.dg/cpp0x/new3.C
@@ -0,0 +1,11 @@
+// PR c++/85847
+// { dg-do compile { target c++11 } }
+
+template <class>
+int f(int b) { return b; }
+
+template <class>
+void g()
+{
+ auto a = new int[f<int>(2), 2];
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: C++ PATCH for c++/85847, ICE with template_id_expr in new()
2018-05-23 13:57 C++ PATCH for c++/85847, ICE with template_id_expr in new() Marek Polacek
@ 2018-05-23 16:45 ` Jason Merrill
2018-05-23 18:51 ` Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2018-05-23 16:45 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Wed, May 23, 2018 at 9:46 AM, Marek Polacek <polacek@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 <polacek@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.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: C++ PATCH for c++/85847, ICE with template_id_expr in new()
2018-05-23 16:45 ` Jason Merrill
@ 2018-05-23 18:51 ` Marek Polacek
2018-05-23 19:28 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2018-05-23 18:51 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Wed, May 23, 2018 at 12:45:11PM -0400, Jason Merrill wrote:
> On Wed, May 23, 2018 at 9:46 AM, Marek Polacek <polacek@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 <polacek@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 <polacek@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))
{
if (complain & tf_warning_or_error)
{
- pedwarn (EXPR_LOC_OR_LOC (outer_nelts, input_location), OPT_Wvla,
+ pedwarn (EXPR_LOC_OR_LOC (cst_outer_nelts, input_location), OPT_Wvla,
typedef_variant_p (orig_type)
? G_("non-constant array new length must be specified "
"directly, not by typedef")
@@ -3011,9 +3011,10 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
size = size_binop (MULT_EXPR, size, fold_convert (sizetype, nelts));
- if (INTEGER_CST == TREE_CODE (outer_nelts))
+ if (cst_outer_nelts != NULL_TREE
+ && TREE_CODE (cst_outer_nelts) == INTEGER_CST)
{
- if (tree_int_cst_lt (max_outer_nelts_tree, outer_nelts))
+ if (tree_int_cst_lt (max_outer_nelts_tree, cst_outer_nelts))
{
/* When the array size is constant, check it at compile time
to make sure it doesn't exceed the implementation-defined
@@ -3639,13 +3640,13 @@ build_new (vec<tree, va_gc> **placement, tree type, tree nelts,
/* 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);
/* The expression in a noptr-new-declarator is erroneous if it's of
non-class type and its value before converting to std::size_t is
less than zero. ... If the expression is a constant expression,
the program is ill-fomed. */
- if (INTEGER_CST == TREE_CODE (cst_nelts)
+ if (TREE_CODE (cst_nelts) == INTEGER_CST
&& tree_int_cst_sgn (cst_nelts) == -1)
{
if (complain & tf_error)
diff --git gcc/testsuite/g++.dg/cpp0x/new3.C gcc/testsuite/g++.dg/cpp0x/new3.C
index e69de29bb2d..c388acf552e 100644
--- gcc/testsuite/g++.dg/cpp0x/new3.C
+++ gcc/testsuite/g++.dg/cpp0x/new3.C
@@ -0,0 +1,11 @@
+// PR c++/85847
+// { dg-do compile { target c++11 } }
+
+template <class>
+int f(int b) { return b; }
+
+template <class>
+void g()
+{
+ auto a = new int[f<int>(2), 2];
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: C++ PATCH for c++/85847, ICE with template_id_expr in new()
2018-05-23 18:51 ` Marek Polacek
@ 2018-05-23 19:28 ` Jason Merrill
2018-05-23 20:36 ` Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2018-05-23 19:28 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Wed, May 23, 2018 at 2:50 PM, Marek Polacek <polacek@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 <polacek@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 <polacek@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 <polacek@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: C++ PATCH for c++/85847, ICE with template_id_expr in new()
2018-05-23 19:28 ` Jason Merrill
@ 2018-05-23 20:36 ` Marek Polacek
2018-05-24 13:31 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2018-05-23 20:36 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Wed, May 23, 2018 at 03:24:20PM -0400, Jason Merrill wrote:
> On Wed, May 23, 2018 at 2:50 PM, Marek Polacek <polacek@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 <polacek@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 <polacek@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 <polacek@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.
True, and it seemed to me that the argument can be null when NELTS is null,
which, according to the comment for build_new_1 could happen. So I was just
being cautious. But I dropped the checks and nothing in the testsuite broke.
> > - 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.
Okay.
Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
2018-05-23 Marek Polacek <polacek@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..5bfd0848fc4 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,7 +2911,7 @@ 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))
+ && !TREE_CONSTANT (cst_outer_nelts))
{
if (complain & tf_warning_or_error)
{
@@ -3011,9 +3010,9 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
size = size_binop (MULT_EXPR, size, fold_convert (sizetype, nelts));
- if (INTEGER_CST == TREE_CODE (outer_nelts))
+ if (TREE_CODE (cst_outer_nelts) == INTEGER_CST)
{
- if (tree_int_cst_lt (max_outer_nelts_tree, outer_nelts))
+ if (tree_int_cst_lt (max_outer_nelts_tree, cst_outer_nelts))
{
/* When the array size is constant, check it at compile time
to make sure it doesn't exceed the implementation-defined
@@ -3639,13 +3638,13 @@ build_new (vec<tree, va_gc> **placement, tree type, tree nelts,
/* 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);
/* The expression in a noptr-new-declarator is erroneous if it's of
non-class type and its value before converting to std::size_t is
less than zero. ... If the expression is a constant expression,
the program is ill-fomed. */
- if (INTEGER_CST == TREE_CODE (cst_nelts)
+ if (TREE_CODE (cst_nelts) == INTEGER_CST
&& tree_int_cst_sgn (cst_nelts) == -1)
{
if (complain & tf_error)
diff --git gcc/testsuite/g++.dg/cpp0x/new3.C gcc/testsuite/g++.dg/cpp0x/new3.C
index e69de29bb2d..c388acf552e 100644
--- gcc/testsuite/g++.dg/cpp0x/new3.C
+++ gcc/testsuite/g++.dg/cpp0x/new3.C
@@ -0,0 +1,11 @@
+// PR c++/85847
+// { dg-do compile { target c++11 } }
+
+template <class>
+int f(int b) { return b; }
+
+template <class>
+void g()
+{
+ auto a = new int[f<int>(2), 2];
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: C++ PATCH for c++/85847, ICE with template_id_expr in new()
2018-05-23 20:36 ` Marek Polacek
@ 2018-05-24 13:31 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2018-05-24 13:31 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
OK.
On Wed, May 23, 2018 at 4:27 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, May 23, 2018 at 03:24:20PM -0400, Jason Merrill wrote:
>> On Wed, May 23, 2018 at 2:50 PM, Marek Polacek <polacek@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 <polacek@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 <polacek@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 <polacek@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.
>
> True, and it seemed to me that the argument can be null when NELTS is null,
> which, according to the comment for build_new_1 could happen. So I was just
> being cautious. But I dropped the checks and nothing in the testsuite broke.
>
>> > - 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.
>
> Okay.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
>
> 2018-05-23 Marek Polacek <polacek@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..5bfd0848fc4 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,7 +2911,7 @@ 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))
> + && !TREE_CONSTANT (cst_outer_nelts))
> {
> if (complain & tf_warning_or_error)
> {
> @@ -3011,9 +3010,9 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
>
> size = size_binop (MULT_EXPR, size, fold_convert (sizetype, nelts));
>
> - if (INTEGER_CST == TREE_CODE (outer_nelts))
> + if (TREE_CODE (cst_outer_nelts) == INTEGER_CST)
> {
> - if (tree_int_cst_lt (max_outer_nelts_tree, outer_nelts))
> + if (tree_int_cst_lt (max_outer_nelts_tree, cst_outer_nelts))
> {
> /* When the array size is constant, check it at compile time
> to make sure it doesn't exceed the implementation-defined
> @@ -3639,13 +3638,13 @@ build_new (vec<tree, va_gc> **placement, tree type, tree nelts,
> /* 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);
>
> /* The expression in a noptr-new-declarator is erroneous if it's of
> non-class type and its value before converting to std::size_t is
> less than zero. ... If the expression is a constant expression,
> the program is ill-fomed. */
> - if (INTEGER_CST == TREE_CODE (cst_nelts)
> + if (TREE_CODE (cst_nelts) == INTEGER_CST
> && tree_int_cst_sgn (cst_nelts) == -1)
> {
> if (complain & tf_error)
> diff --git gcc/testsuite/g++.dg/cpp0x/new3.C gcc/testsuite/g++.dg/cpp0x/new3.C
> index e69de29bb2d..c388acf552e 100644
> --- gcc/testsuite/g++.dg/cpp0x/new3.C
> +++ gcc/testsuite/g++.dg/cpp0x/new3.C
> @@ -0,0 +1,11 @@
> +// PR c++/85847
> +// { dg-do compile { target c++11 } }
> +
> +template <class>
> +int f(int b) { return b; }
> +
> +template <class>
> +void g()
> +{
> + auto a = new int[f<int>(2), 2];
> +}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-24 13:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23 13:57 C++ PATCH for c++/85847, ICE with template_id_expr in new() Marek Polacek
2018-05-23 16:45 ` Jason Merrill
2018-05-23 18:51 ` Marek Polacek
2018-05-23 19:28 ` Jason Merrill
2018-05-23 20:36 ` Marek Polacek
2018-05-24 13:31 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).