Message ID | 20220601224536.1553412-1-polacek@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: ICE with template NEW_EXPR [PR105803] | expand |
On Wed, 1 Jun 2022, Marek Polacek via Gcc-patches wrote: > Here we ICE because value_dependent_expression_p gets a NEW_EXPR > whose operand is a type, and we go to the default case which just > calls v_d_e_p on each operand of the NEW_EXPR. Since one of them > is a type, we crash on the new assert in t_d_e_p. Looks like NEW_EXPR is considered to be not potentially constant according to potential_constant_expression. I thought we shouldn't be calling value_dependent_expression_p on such exprs? > > t_d_e_p has code to handle {,VEC_}NEW_EXPR, which at this point > was already performed, so I think we can handle these two codes > specifically and skip the second operand, which is always going > to be a type. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR c++/105803 > > gcc/cp/ChangeLog: > > * pt.cc (value_dependent_expression_p): Handle {,VEC_}NEW_EXPR > in the switch. > > gcc/testsuite/ChangeLog: > > * g++.dg/template/new13.C: New test. > --- > gcc/cp/pt.cc | 8 ++++++++ > gcc/testsuite/g++.dg/template/new13.C | 11 +++++++++++ > 2 files changed, 19 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/template/new13.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 6de8e496859..836861e1039 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -27643,6 +27643,14 @@ value_dependent_expression_p (tree expression) > under instantiate_non_dependent_expr; it can't be constant. */ > return true; > > + case NEW_EXPR: > + case VEC_NEW_EXPR: > + /* The second operand is a type, which type_dependent_expression_p > + (and therefore value_dependent_expression_p) doesn't want to see. */ > + return (value_dependent_expression_p (TREE_OPERAND (expression, 0)) > + || value_dependent_expression_p (TREE_OPERAND (expression, 2)) > + || value_dependent_expression_p (TREE_OPERAND (expression, 3))); > + > default: > /* A constant expression is value-dependent if any subexpression is > value-dependent. */ > diff --git a/gcc/testsuite/g++.dg/template/new13.C b/gcc/testsuite/g++.dg/template/new13.C > new file mode 100644 > index 00000000000..3168374b26d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/new13.C > @@ -0,0 +1,11 @@ > +// PR c++/105803 > +// { dg-do compile } > +// { dg-additional-options "-fchecking=2" } > + > +namespace std { > +template <typename> class shared_ptr; > +} > +struct S {}; > +template <int> void build_matrices() { > + std::shared_ptr<S>(new S); > +} I think this testcase might be IFNDR since shared_ptr<S> is incomplete at the point of its non-dependent use. > > base-commit: 2d546ff69455f7deadab65309de89d19380a8864 > -- > 2.36.1 > >
On Thu, Jun 02, 2022 at 08:42:36AM -0400, Patrick Palka wrote: > On Wed, 1 Jun 2022, Marek Polacek via Gcc-patches wrote: > > > Here we ICE because value_dependent_expression_p gets a NEW_EXPR > > whose operand is a type, and we go to the default case which just > > calls v_d_e_p on each operand of the NEW_EXPR. Since one of them > > is a type, we crash on the new assert in t_d_e_p. > > Looks like NEW_EXPR is considered to be not potentially constant > according to potential_constant_expression. I thought we shouldn't > be calling value_dependent_expression_p on such exprs? You're correct. This is non-obvious: instantiation_dependent_expression_p calls p_c_e before v_d_e_p, but the expression is CAST_EXPR<[NEW_EXPR]>, where the [] denotes a TREE_LIST, created in cp_parser_functional_cast. This TREE_LIST has no type. So p_c_e_1/CAST_EXPR goes to 9183 /* If this is a dependent type, it could end up being a class 9184 with conversions. */ 9185 if (type == NULL_TREE || WILDCARD_TYPE_P (type)) 9186 return true; and returns true. So we call v_d_e_p, which looks at the CAST_EXPR's op and sees a TREE_LIST, so it calls any_value_dependent_elements_p, and we end up with a NEW_EXPR. An alternative/more proper fix would be to fix p_c_e_1/CAST_EXPR. Maybe by calling any_type_dependent_elements_p (which currently has no uses). Thoughts? > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/template/new13.C > > @@ -0,0 +1,11 @@ > > +// PR c++/105803 > > +// { dg-do compile } > > +// { dg-additional-options "-fchecking=2" } > > + > > +namespace std { > > +template <typename> class shared_ptr; > > +} > > +struct S {}; > > +template <int> void build_matrices() { > > + std::shared_ptr<S>(new S); > > +} > > I think this testcase might be IFNDR since shared_ptr<S> is incomplete > at the point of its non-dependent use. Ah, overreduced. I've made shared_ptr complete. Marek
On 6/2/22 10:03, Marek Polacek wrote: > On Thu, Jun 02, 2022 at 08:42:36AM -0400, Patrick Palka wrote: >> On Wed, 1 Jun 2022, Marek Polacek via Gcc-patches wrote: >> >>> Here we ICE because value_dependent_expression_p gets a NEW_EXPR >>> whose operand is a type, and we go to the default case which just >>> calls v_d_e_p on each operand of the NEW_EXPR. Since one of them >>> is a type, we crash on the new assert in t_d_e_p. >> >> Looks like NEW_EXPR is considered to be not potentially constant >> according to potential_constant_expression. I thought we shouldn't >> be calling value_dependent_expression_p on such exprs? Except in C++20 new-expressions are potentially constant, so the patch is OK, and we should adjust pce1 accordingly. I notice we currently fail to handle struct A { int i; constexpr A(int *p): i(*p) { delete p; } }; constexpr int i = A(new int(42)).i; though it does work inside a function. > You're correct. This is non-obvious: instantiation_dependent_expression_p > calls p_c_e before v_d_e_p, but the expression is CAST_EXPR<[NEW_EXPR]>, > where the [] denotes a TREE_LIST, created in cp_parser_functional_cast. > This TREE_LIST has no type. So p_c_e_1/CAST_EXPR goes to > 9183 /* If this is a dependent type, it could end up being a class > 9184 with conversions. */ > 9185 if (type == NULL_TREE || WILDCARD_TYPE_P (type)) > 9186 return true; > and returns true. > > So we call v_d_e_p, which looks at the CAST_EXPR's op and sees a TREE_LIST, > so it calls any_value_dependent_elements_p, and we end up with a NEW_EXPR. > > An alternative/more proper fix would be to fix p_c_e_1/CAST_EXPR. Maybe > by calling any_type_dependent_elements_p (which currently has no uses). > Thoughts? > >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/template/new13.C >>> @@ -0,0 +1,11 @@ >>> +// PR c++/105803 >>> +// { dg-do compile } >>> +// { dg-additional-options "-fchecking=2" } >>> + >>> +namespace std { >>> +template <typename> class shared_ptr; >>> +} >>> +struct S {}; >>> +template <int> void build_matrices() { >>> + std::shared_ptr<S>(new S); >>> +} >> >> I think this testcase might be IFNDR since shared_ptr<S> is incomplete >> at the point of its non-dependent use. > > Ah, overreduced. I've made shared_ptr complete. > > Marek >
On Thu, Jun 02, 2022 at 03:42:15PM -0400, Jason Merrill wrote: > On 6/2/22 10:03, Marek Polacek wrote: > > On Thu, Jun 02, 2022 at 08:42:36AM -0400, Patrick Palka wrote: > > > On Wed, 1 Jun 2022, Marek Polacek via Gcc-patches wrote: > > > > > > > Here we ICE because value_dependent_expression_p gets a NEW_EXPR > > > > whose operand is a type, and we go to the default case which just > > > > calls v_d_e_p on each operand of the NEW_EXPR. Since one of them > > > > is a type, we crash on the new assert in t_d_e_p. > > > > > > Looks like NEW_EXPR is considered to be not potentially constant > > > according to potential_constant_expression. I thought we shouldn't > > > be calling value_dependent_expression_p on such exprs? > > Except in C++20 new-expressions are potentially constant, so the patch is Thanks, pushed. > OK, and we should adjust pce1 accordingly. Is the attached patch OK then? So far dg.exp passed. Though it won't help with... > I notice we currently fail to handle > > struct A > { > int i; > constexpr A(int *p): i(*p) { delete p; } > }; > > constexpr int i = A(new int(42)).i; > > though it does work inside a function. ...this test (it complains about a TARGET_EXPR's slot variable not being declared constexpr), so I'm going to open a PR. From cf70354894bc31cc542ed8df40633bea2427fee7 Mon Sep 17 00:00:00 2001 From: Marek Polacek <polacek@redhat.com> Date: Thu, 2 Jun 2022 15:56:18 -0400 Subject: [PATCH] c++: new-expression is potentially constant in C++20 ... so adjust p_c_e accordingly. gcc/cp/ChangeLog: * constexpr.cc (potential_constant_expression_1): Treat {,VEC_}NEW_EXPR as potentially constant in C++20. --- gcc/cp/constexpr.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 1346a1d4c10..2bbd8785627 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -9039,10 +9039,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, "before C++17"); return false; - case DYNAMIC_CAST_EXPR: - case PSEUDO_DTOR_EXPR: case NEW_EXPR: case VEC_NEW_EXPR: + if (cxx_dialect >= cxx20) + /* In C++20, new-expressions are potentially constant. */ + return true; + else if (flags & tf_error) + error_at (loc, "new-expression is not a constant expression " + "before C++20"); + return false; + + case DYNAMIC_CAST_EXPR: + case PSEUDO_DTOR_EXPR: case DELETE_EXPR: case VEC_DELETE_EXPR: case THROW_EXPR: base-commit: 7b98910406b5000a6429c188b0c6cc14e3140637
On 6/2/22 16:10, Marek Polacek wrote: > On Thu, Jun 02, 2022 at 03:42:15PM -0400, Jason Merrill wrote: >> On 6/2/22 10:03, Marek Polacek wrote: >>> On Thu, Jun 02, 2022 at 08:42:36AM -0400, Patrick Palka wrote: >>>> On Wed, 1 Jun 2022, Marek Polacek via Gcc-patches wrote: >>>> >>>>> Here we ICE because value_dependent_expression_p gets a NEW_EXPR >>>>> whose operand is a type, and we go to the default case which just >>>>> calls v_d_e_p on each operand of the NEW_EXPR. Since one of them >>>>> is a type, we crash on the new assert in t_d_e_p. >>>> >>>> Looks like NEW_EXPR is considered to be not potentially constant >>>> according to potential_constant_expression. I thought we shouldn't >>>> be calling value_dependent_expression_p on such exprs? >> >> Except in C++20 new-expressions are potentially constant, so the patch is > > Thanks, pushed. > >> OK, and we should adjust pce1 accordingly. > > Is the attached patch OK then? So far dg.exp passed. Though it won't help > with... > >> I notice we currently fail to handle >> >> struct A >> { >> int i; >> constexpr A(int *p): i(*p) { delete p; } >> }; >> >> constexpr int i = A(new int(42)).i; >> >> though it does work inside a function. > > ...this test (it complains about a TARGET_EXPR's slot variable not being > declared constexpr), so I'm going to open a PR. > > From cf70354894bc31cc542ed8df40633bea2427fee7 Mon Sep 17 00:00:00 2001 > From: Marek Polacek <polacek@redhat.com> > Date: Thu, 2 Jun 2022 15:56:18 -0400 > Subject: [PATCH] c++: new-expression is potentially constant in C++20 > > ... so adjust p_c_e accordingly. > > gcc/cp/ChangeLog: > > * constexpr.cc (potential_constant_expression_1): Treat > {,VEC_}NEW_EXPR as potentially constant in C++20. > --- > gcc/cp/constexpr.cc | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 1346a1d4c10..2bbd8785627 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -9039,10 +9039,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > "before C++17"); > return false; > > - case DYNAMIC_CAST_EXPR: > - case PSEUDO_DTOR_EXPR: > case NEW_EXPR: > case VEC_NEW_EXPR: > + if (cxx_dialect >= cxx20) > + /* In C++20, new-expressions are potentially constant. */ > + return true; > + else if (flags & tf_error) > + error_at (loc, "new-expression is not a constant expression " > + "before C++20"); > + return false; > + > + case DYNAMIC_CAST_EXPR: > + case PSEUDO_DTOR_EXPR: > case DELETE_EXPR: > case VEC_DELETE_EXPR: Delete, too. > case THROW_EXPR: > > base-commit: 7b98910406b5000a6429c188b0c6cc14e3140637
On Thu, Jun 02, 2022 at 04:26:27PM -0400, Jason Merrill wrote: > On 6/2/22 16:10, Marek Polacek wrote: > > index 1346a1d4c10..2bbd8785627 100644 > > --- a/gcc/cp/constexpr.cc > > +++ b/gcc/cp/constexpr.cc > > @@ -9039,10 +9039,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > > "before C++17"); > > return false; > > - case DYNAMIC_CAST_EXPR: > > - case PSEUDO_DTOR_EXPR: > > case NEW_EXPR: > > case VEC_NEW_EXPR: > > + if (cxx_dialect >= cxx20) > > + /* In C++20, new-expressions are potentially constant. */ > > + return true; > > + else if (flags & tf_error) > > + error_at (loc, "new-expression is not a constant expression " > > + "before C++20"); > > + return false; > > + > > + case DYNAMIC_CAST_EXPR: > > + case PSEUDO_DTOR_EXPR: > > case DELETE_EXPR: > > case VEC_DELETE_EXPR: > > Delete, too. Duh. Fixed: From 2423f6548405185e256036df3d0ef3c13fd996c5 Mon Sep 17 00:00:00 2001 From: Marek Polacek <polacek@redhat.com> Date: Thu, 2 Jun 2022 15:56:18 -0400 Subject: [PATCH] c++: new-expression is potentially constant in C++20 ... so adjust p_c_e accordingly. gcc/cp/ChangeLog: * constexpr.cc (potential_constant_expression_1): Treat {,VEC_}NEW_EXPR and {,VEC_}DELETE_EXPRas potentially constant in C++20. --- gcc/cp/constexpr.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 1346a1d4c10..684238883dc 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -9039,12 +9039,20 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, "before C++17"); return false; - case DYNAMIC_CAST_EXPR: - case PSEUDO_DTOR_EXPR: case NEW_EXPR: case VEC_NEW_EXPR: case DELETE_EXPR: case VEC_DELETE_EXPR: + if (cxx_dialect >= cxx20) + /* In C++20, new-expressions are potentially constant. */ + return true; + else if (flags & tf_error) + error_at (loc, "new-expression is not a constant expression " + "before C++20"); + return false; + + case DYNAMIC_CAST_EXPR: + case PSEUDO_DTOR_EXPR: case THROW_EXPR: case OMP_PARALLEL: case OMP_TASK: base-commit: 7b98910406b5000a6429c188b0c6cc14e3140637
On 6/2/22 16:33, Marek Polacek wrote: > On Thu, Jun 02, 2022 at 04:26:27PM -0400, Jason Merrill wrote: >> On 6/2/22 16:10, Marek Polacek wrote: >>> index 1346a1d4c10..2bbd8785627 100644 >>> --- a/gcc/cp/constexpr.cc >>> +++ b/gcc/cp/constexpr.cc >>> @@ -9039,10 +9039,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, >>> "before C++17"); >>> return false; >>> - case DYNAMIC_CAST_EXPR: >>> - case PSEUDO_DTOR_EXPR: >>> case NEW_EXPR: >>> case VEC_NEW_EXPR: >>> + if (cxx_dialect >= cxx20) >>> + /* In C++20, new-expressions are potentially constant. */ >>> + return true; >>> + else if (flags & tf_error) >>> + error_at (loc, "new-expression is not a constant expression " >>> + "before C++20"); >>> + return false; >>> + >>> + case DYNAMIC_CAST_EXPR: >>> + case PSEUDO_DTOR_EXPR: >>> case DELETE_EXPR: >>> case VEC_DELETE_EXPR: >> >> Delete, too. > > Duh. Fixed: OK, thanks. > From 2423f6548405185e256036df3d0ef3c13fd996c5 Mon Sep 17 00:00:00 2001 > From: Marek Polacek <polacek@redhat.com> > Date: Thu, 2 Jun 2022 15:56:18 -0400 > Subject: [PATCH] c++: new-expression is potentially constant in C++20 > > ... so adjust p_c_e accordingly. > > gcc/cp/ChangeLog: > > * constexpr.cc (potential_constant_expression_1): Treat > {,VEC_}NEW_EXPR and {,VEC_}DELETE_EXPRas potentially constant in C++20. > --- > gcc/cp/constexpr.cc | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 1346a1d4c10..684238883dc 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -9039,12 +9039,20 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > "before C++17"); > return false; > > - case DYNAMIC_CAST_EXPR: > - case PSEUDO_DTOR_EXPR: > case NEW_EXPR: > case VEC_NEW_EXPR: > case DELETE_EXPR: > case VEC_DELETE_EXPR: > + if (cxx_dialect >= cxx20) > + /* In C++20, new-expressions are potentially constant. */ > + return true; > + else if (flags & tf_error) > + error_at (loc, "new-expression is not a constant expression " > + "before C++20"); > + return false; > + > + case DYNAMIC_CAST_EXPR: > + case PSEUDO_DTOR_EXPR: > case THROW_EXPR: > case OMP_PARALLEL: > case OMP_TASK: > > base-commit: 7b98910406b5000a6429c188b0c6cc14e3140637
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 6de8e496859..836861e1039 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -27643,6 +27643,14 @@ value_dependent_expression_p (tree expression) under instantiate_non_dependent_expr; it can't be constant. */ return true; + case NEW_EXPR: + case VEC_NEW_EXPR: + /* The second operand is a type, which type_dependent_expression_p + (and therefore value_dependent_expression_p) doesn't want to see. */ + return (value_dependent_expression_p (TREE_OPERAND (expression, 0)) + || value_dependent_expression_p (TREE_OPERAND (expression, 2)) + || value_dependent_expression_p (TREE_OPERAND (expression, 3))); + default: /* A constant expression is value-dependent if any subexpression is value-dependent. */ diff --git a/gcc/testsuite/g++.dg/template/new13.C b/gcc/testsuite/g++.dg/template/new13.C new file mode 100644 index 00000000000..3168374b26d --- /dev/null +++ b/gcc/testsuite/g++.dg/template/new13.C @@ -0,0 +1,11 @@ +// PR c++/105803 +// { dg-do compile } +// { dg-additional-options "-fchecking=2" } + +namespace std { +template <typename> class shared_ptr; +} +struct S {}; +template <int> void build_matrices() { + std::shared_ptr<S>(new S); +}