Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On 7/25/23 16:30, Marek Polacek wrote: On Tue, Jul 25, 2023 at 04:24:39PM -0400, Jason Merrill wrote: On 7/25/23 15:59, Marek Polacek wrote: Something like this, then? I see that cp_parser_initializer_clause et al offer further opportunities (because they sometimes use a dummy too) but this should be a good start. Looks good. Please do update the other callers as well, while you're looking at this. Thanks. Can I push this part first? Ah, sure. I had thought the other callers would be trivial to add. Jason
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On Tue, Jul 25, 2023 at 04:24:39PM -0400, Jason Merrill wrote: > On 7/25/23 15:59, Marek Polacek wrote: > > Something like this, then? I see that cp_parser_initializer_clause et al > > offer further opportunities (because they sometimes use a dummy too) but > > this should be a good start. > > Looks good. Please do update the other callers as well, while you're > looking at this. Thanks. Can I push this part first?
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On 7/25/23 15:59, Marek Polacek wrote:
On Fri, Jul 21, 2023 at 01:44:17PM -0400, Jason Merrill wrote:
On 7/20/23 17:58, Marek Polacek wrote:
On Thu, Jul 20, 2023 at 03:51:32PM -0400, Marek Polacek wrote:
On Thu, Jul 20, 2023 at 02:37:07PM -0400, Jason Merrill wrote:
On 7/20/23 14:13, Marek Polacek wrote:
On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote:
On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches?
Looks reasonable to me.
Thanks.
Though I wonder if we could also fix this by not checking potentiality
at all in this case? The problematic call to is_rvalue_constant_expression
happens from cp_parser_constant_expression with 'allow_non_constant' != 0
and with 'non_constant_p' being a dummy out argument that comes from
cp_parser_functional_cast, so the result of is_rvalue_constant_expression
is effectively unused in this case, and we should be able to safely elide
it when 'allow_non_constant && non_constant_p == nullptr'.
Sounds plausible. I think my patch could be applied first since it
removes a tiny bit of code, then I can hopefully remove the flag below,
then maybe go back and optimize the call to is_rvalue_constant_expression.
Does that sound sensible?
Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p
is also effectively unused and could be removed?
It looks that way. Seems it's only used in cp_parser_constant_expression:
10806 if (allow_non_constant_p)
10807 *non_constant_p = parser->non_integral_constant_expression_p;
but that could be easily replaced by a local var. I'd be happy to see if
we can actually do away with it. (I wonder why it was introduced and when
it actually stopped being useful.)
It was for the C++98 notion of constant-expression, which was more of a
parser-level notion, and has been supplanted by the C++11 version. I'm
happy to remove it, and therefore remove the is_rvalue_constant_expression
call.
Wonderful. I'll do that next.
I found a use of parser->non_integral_constant_expression_p:
finish_id_expression_1 can set it to true which then makes
a difference in cp_parser_constant_expression in C++98. In
cp_parser_constant_expression we set n_i_c_e_p to false, call
cp_parser_assignment_expression in which finish_id_expression_1
sets n_i_c_e_p to true, then back in cp_parser_constant_expression
we skip the cxx11 block, and set *non_constant_p to true. If I
remove n_i_c_e_p, we lose that. This can be seen in init/array60.C.
Sure, we would need to use the C++11 code for C++98 mode, which is likely
fine but is more uncertain.
It's probably simpler to just ignore n_i_c_e_p for C++11 and up, along with
Patrick's suggestion of allowing null non_constant_p with true
allow_non_constant_p.
Something like this, then? I see that cp_parser_initializer_clause et al
offer further opportunities (because they sometimes use a dummy too) but
this should be a good start.
Looks good. Please do update the other callers as well, while you're
looking at this.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
It's pointless to call *_rvalue_constant_expression when we're not using
the result. Also apply some drive-by cleanups.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_constant_expression): Allow non_constant_p to be
nullptr even when allow_non_constant_p is true. Don't call
_rvalue_constant_expression when not necessary. Move local variable
declarations closer to their first use.
(cp_parser_static_assert): Don't pass a dummy down to
cp_parser_constant_expression.
---
gcc/cp/parser.cc | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5e2b5cba57e..efaa806f107 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -10734,11 +10734,6 @@ cp_parser_constant_expression (cp_parser* parser,
bool *non_constant_p /* = NULL */,
bool strict_p /* = false */)
{
- bool saved_integral_constant_expression_p;
- bool saved_allow_non_integral_constant_expression_p;
- bool saved_non_integral_constant_expression_p;
- cp_expr expression;
-
/* It might seem that we could simply parse the
conditional-expression, and then check to see if it were
TREE_CONSTANT. However, an expression that is TREE_CONSTANT is
@@ -10757,10 +10752,12 @@ cp_parser_constant_expression (cp_parser* parser,
will fold this operation to an INTEGER_CST for `3'. */
/* Save the old settings. */
- saved_integral_constant_expression_p =
parser->integral_constant_expression_p;
- saved_allow_non_integral_constant_expression_p
+ bool saved_integral_constant_expression_p
+= parser->integral_constant_expression_p;
+ bool saved_allow_non_integral_constant_expression_p
= parser->allow_non_integral_constant_expression_p;
-
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On Fri, Jul 21, 2023 at 01:44:17PM -0400, Jason Merrill wrote:
> On 7/20/23 17:58, Marek Polacek wrote:
> > On Thu, Jul 20, 2023 at 03:51:32PM -0400, Marek Polacek wrote:
> > > On Thu, Jul 20, 2023 at 02:37:07PM -0400, Jason Merrill wrote:
> > > > On 7/20/23 14:13, Marek Polacek wrote:
> > > > > On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote:
> > > > > > On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
> > > > > >
> > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and
> > > > > > > branches?
> > > > > >
> > > > > > Looks reasonable to me.
> > > > >
> > > > > Thanks.
> > > > > > Though I wonder if we could also fix this by not checking
> > > > > > potentiality
> > > > > > at all in this case? The problematic call to
> > > > > > is_rvalue_constant_expression
> > > > > > happens from cp_parser_constant_expression with
> > > > > > 'allow_non_constant' != 0
> > > > > > and with 'non_constant_p' being a dummy out argument that comes from
> > > > > > cp_parser_functional_cast, so the result of
> > > > > > is_rvalue_constant_expression
> > > > > > is effectively unused in this case, and we should be able to safely
> > > > > > elide
> > > > > > it when 'allow_non_constant && non_constant_p == nullptr'.
> > > > >
> > > > > Sounds plausible. I think my patch could be applied first since it
> > > > > removes a tiny bit of code, then I can hopefully remove the flag
> > > > > below,
> > > > > then maybe go back and optimize the call to
> > > > > is_rvalue_constant_expression.
> > > > > Does that sound sensible?
> > > > >
> > > > > > Relatedly, ISTM the member
> > > > > > cp_parser::non_integral_constant_expression_p
> > > > > > is also effectively unused and could be removed?
> > > > >
> > > > > It looks that way. Seems it's only used in
> > > > > cp_parser_constant_expression:
> > > > > 10806 if (allow_non_constant_p)
> > > > > 10807 *non_constant_p =
> > > > > parser->non_integral_constant_expression_p;
> > > > > but that could be easily replaced by a local var. I'd be happy to
> > > > > see if
> > > > > we can actually do away with it. (I wonder why it was introduced and
> > > > > when
> > > > > it actually stopped being useful.)
> > > >
> > > > It was for the C++98 notion of constant-expression, which was more of a
> > > > parser-level notion, and has been supplanted by the C++11 version. I'm
> > > > happy to remove it, and therefore remove the
> > > > is_rvalue_constant_expression
> > > > call.
> > >
> > > Wonderful. I'll do that next.
> >
> > I found a use of parser->non_integral_constant_expression_p:
> > finish_id_expression_1 can set it to true which then makes
> > a difference in cp_parser_constant_expression in C++98. In
> > cp_parser_constant_expression we set n_i_c_e_p to false, call
> > cp_parser_assignment_expression in which finish_id_expression_1
> > sets n_i_c_e_p to true, then back in cp_parser_constant_expression
> > we skip the cxx11 block, and set *non_constant_p to true. If I
> > remove n_i_c_e_p, we lose that. This can be seen in init/array60.C.
>
> Sure, we would need to use the C++11 code for C++98 mode, which is likely
> fine but is more uncertain.
>
> It's probably simpler to just ignore n_i_c_e_p for C++11 and up, along with
> Patrick's suggestion of allowing null non_constant_p with true
> allow_non_constant_p.
Something like this, then? I see that cp_parser_initializer_clause et al
offer further opportunities (because they sometimes use a dummy too) but
this should be a good start.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
It's pointless to call *_rvalue_constant_expression when we're not using
the result. Also apply some drive-by cleanups.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_constant_expression): Allow non_constant_p to be
nullptr even when allow_non_constant_p is true. Don't call
_rvalue_constant_expression when not necessary. Move local variable
declarations closer to their first use.
(cp_parser_static_assert): Don't pass a dummy down to
cp_parser_constant_expression.
---
gcc/cp/parser.cc | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5e2b5cba57e..efaa806f107 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -10734,11 +10734,6 @@ cp_parser_constant_expression (cp_parser* parser,
bool *non_constant_p /* = NULL */,
bool strict_p /* = false */)
{
- bool saved_integral_constant_expression_p;
- bool saved_allow_non_integral_constant_expression_p;
- bool saved_non_integral_constant_expression_p;
- cp_expr expression;
-
/* It might seem that we could simply parse the
conditional-expression, and then check to see if it were
TREE_CONSTANT. However, an expression that is TREE_CONSTANT is
@@ -10757,10 +10752,12 @@ cp_parser_constant_expression
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On 7/20/23 15:51, Marek Polacek wrote:
On Thu, Jul 20, 2023 at 02:37:07PM -0400, Jason Merrill wrote:
On 7/20/23 14:13, Marek Polacek wrote:
On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote:
On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches?
Looks reasonable to me.
Thanks.
Though I wonder if we could also fix this by not checking potentiality
at all in this case? The problematic call to is_rvalue_constant_expression
happens from cp_parser_constant_expression with 'allow_non_constant' != 0
and with 'non_constant_p' being a dummy out argument that comes from
cp_parser_functional_cast, so the result of is_rvalue_constant_expression
is effectively unused in this case, and we should be able to safely elide
it when 'allow_non_constant && non_constant_p == nullptr'.
Sounds plausible. I think my patch could be applied first since it
removes a tiny bit of code, then I can hopefully remove the flag below,
then maybe go back and optimize the call to is_rvalue_constant_expression.
Does that sound sensible?
Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p
is also effectively unused and could be removed?
It looks that way. Seems it's only used in cp_parser_constant_expression:
10806 if (allow_non_constant_p)
10807 *non_constant_p = parser->non_integral_constant_expression_p;
but that could be easily replaced by a local var. I'd be happy to see if
we can actually do away with it. (I wonder why it was introduced and when
it actually stopped being useful.)
It was for the C++98 notion of constant-expression, which was more of a
parser-level notion, and has been supplanted by the C++11 version. I'm
happy to remove it, and therefore remove the is_rvalue_constant_expression
call.
Wonderful. I'll do that next.
-- >8 --
is_really_empty_class is liable to crash when it gets an incomplete
or dependent type. Since r11-557, we pass the yet-uninstantiated
class type S<0> of the PARM_DECL s to is_really_empty_class -- because
of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
change in cp_parser_constant_expression. Here we're not parsing
a template so we did not check COMPLETE_TYPE_P as we should.
PR c++/110106
gcc/cp/ChangeLog:
* constexpr.cc (potential_constant_expression_1): Check COMPLETE_TYPE_P
even when !processing_template_decl.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/noexcept80.C: New test.
---
gcc/cp/constexpr.cc | 2 +-
gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 6e8f1c2b61e..1f59c5472fb 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -9116,7 +9116,7 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
if (now && want_rval)
{
tree type = TREE_TYPE (t);
- if ((processing_template_decl && !COMPLETE_TYPE_P (type))
+ if (!COMPLETE_TYPE_P (type)
|| dependent_type_p (type)
There shouldn't be a problem completing the type here, so it seems to me
that we're missing a call to complete_type_p, at least when
!processing_template_decl. Probably need to move the dependent_type_p check
up as a result.
Like so?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK.
-- >8 --
is_really_empty_class is liable to crash when it gets an incomplete
or dependent type. Since r11-557, we pass the yet-uninstantiated
class type S<0> of the PARM_DECL s to is_really_empty_class -- because
of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
change in cp_parser_constant_expression. Here we're not parsing
a template so we did not check COMPLETE_TYPE_P as we should.
It should work to complete the type before checking COMPLETE_TYPE_P.
PR c++/110106
gcc/cp/ChangeLog:
* constexpr.cc (potential_constant_expression_1): Try to complete the
type when !processing_template_decl.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/noexcept80.C: New test.
---
gcc/cp/constexpr.cc | 5 +++--
gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
2 files changed, 15 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 6e8f1c2b61e..fb94f3cefcb 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -9116,8 +9116,9 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
if (now && want_rval)
{
tree type = TREE_TYPE (t);
- if ((processing_template_decl && !COMPLETE_TYPE_P (type))
- || dependent_type_p (type)
+ if (dependent_type_p (type)
+ ||
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On 7/20/23 17:58, Marek Polacek wrote: On Thu, Jul 20, 2023 at 03:51:32PM -0400, Marek Polacek wrote: On Thu, Jul 20, 2023 at 02:37:07PM -0400, Jason Merrill wrote: On 7/20/23 14:13, Marek Polacek wrote: On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote: On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote: Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches? Looks reasonable to me. Thanks. Though I wonder if we could also fix this by not checking potentiality at all in this case? The problematic call to is_rvalue_constant_expression happens from cp_parser_constant_expression with 'allow_non_constant' != 0 and with 'non_constant_p' being a dummy out argument that comes from cp_parser_functional_cast, so the result of is_rvalue_constant_expression is effectively unused in this case, and we should be able to safely elide it when 'allow_non_constant && non_constant_p == nullptr'. Sounds plausible. I think my patch could be applied first since it removes a tiny bit of code, then I can hopefully remove the flag below, then maybe go back and optimize the call to is_rvalue_constant_expression. Does that sound sensible? Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p is also effectively unused and could be removed? It looks that way. Seems it's only used in cp_parser_constant_expression: 10806 if (allow_non_constant_p) 10807 *non_constant_p = parser->non_integral_constant_expression_p; but that could be easily replaced by a local var. I'd be happy to see if we can actually do away with it. (I wonder why it was introduced and when it actually stopped being useful.) It was for the C++98 notion of constant-expression, which was more of a parser-level notion, and has been supplanted by the C++11 version. I'm happy to remove it, and therefore remove the is_rvalue_constant_expression call. Wonderful. I'll do that next. I found a use of parser->non_integral_constant_expression_p: finish_id_expression_1 can set it to true which then makes a difference in cp_parser_constant_expression in C++98. In cp_parser_constant_expression we set n_i_c_e_p to false, call cp_parser_assignment_expression in which finish_id_expression_1 sets n_i_c_e_p to true, then back in cp_parser_constant_expression we skip the cxx11 block, and set *non_constant_p to true. If I remove n_i_c_e_p, we lose that. This can be seen in init/array60.C. Sure, we would need to use the C++11 code for C++98 mode, which is likely fine but is more uncertain. It's probably simpler to just ignore n_i_c_e_p for C++11 and up, along with Patrick's suggestion of allowing null non_constant_p with true allow_non_constant_p. Jason
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On Thu, Jul 20, 2023 at 03:51:32PM -0400, Marek Polacek wrote: > On Thu, Jul 20, 2023 at 02:37:07PM -0400, Jason Merrill wrote: > > On 7/20/23 14:13, Marek Polacek wrote: > > > On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote: > > > > On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote: > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and > > > > > branches? > > > > > > > > Looks reasonable to me. > > > > > > Thanks. > > > > Though I wonder if we could also fix this by not checking potentiality > > > > at all in this case? The problematic call to > > > > is_rvalue_constant_expression > > > > happens from cp_parser_constant_expression with 'allow_non_constant' != > > > > 0 > > > > and with 'non_constant_p' being a dummy out argument that comes from > > > > cp_parser_functional_cast, so the result of > > > > is_rvalue_constant_expression > > > > is effectively unused in this case, and we should be able to safely > > > > elide > > > > it when 'allow_non_constant && non_constant_p == nullptr'. > > > > > > Sounds plausible. I think my patch could be applied first since it > > > removes a tiny bit of code, then I can hopefully remove the flag below, > > > then maybe go back and optimize the call to is_rvalue_constant_expression. > > > Does that sound sensible? > > > > > > > Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p > > > > is also effectively unused and could be removed? > > > > > > It looks that way. Seems it's only used in cp_parser_constant_expression: > > > 10806 if (allow_non_constant_p) > > > 10807 *non_constant_p = parser->non_integral_constant_expression_p; > > > but that could be easily replaced by a local var. I'd be happy to see if > > > we can actually do away with it. (I wonder why it was introduced and when > > > it actually stopped being useful.) > > > > It was for the C++98 notion of constant-expression, which was more of a > > parser-level notion, and has been supplanted by the C++11 version. I'm > > happy to remove it, and therefore remove the is_rvalue_constant_expression > > call. > > Wonderful. I'll do that next. I found a use of parser->non_integral_constant_expression_p: finish_id_expression_1 can set it to true which then makes a difference in cp_parser_constant_expression in C++98. In cp_parser_constant_expression we set n_i_c_e_p to false, call cp_parser_assignment_expression in which finish_id_expression_1 sets n_i_c_e_p to true, then back in cp_parser_constant_expression we skip the cxx11 block, and set *non_constant_p to true. If I remove n_i_c_e_p, we lose that. This can be seen in init/array60.C. Marek
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On Thu, Jul 20, 2023 at 02:37:07PM -0400, Jason Merrill wrote:
> On 7/20/23 14:13, Marek Polacek wrote:
> > On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote:
> > > On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
> > >
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and
> > > > branches?
> > >
> > > Looks reasonable to me.
> >
> > Thanks.
> > > Though I wonder if we could also fix this by not checking potentiality
> > > at all in this case? The problematic call to
> > > is_rvalue_constant_expression
> > > happens from cp_parser_constant_expression with 'allow_non_constant' != 0
> > > and with 'non_constant_p' being a dummy out argument that comes from
> > > cp_parser_functional_cast, so the result of is_rvalue_constant_expression
> > > is effectively unused in this case, and we should be able to safely elide
> > > it when 'allow_non_constant && non_constant_p == nullptr'.
> >
> > Sounds plausible. I think my patch could be applied first since it
> > removes a tiny bit of code, then I can hopefully remove the flag below,
> > then maybe go back and optimize the call to is_rvalue_constant_expression.
> > Does that sound sensible?
> >
> > > Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p
> > > is also effectively unused and could be removed?
> >
> > It looks that way. Seems it's only used in cp_parser_constant_expression:
> > 10806 if (allow_non_constant_p)
> > 10807 *non_constant_p = parser->non_integral_constant_expression_p;
> > but that could be easily replaced by a local var. I'd be happy to see if
> > we can actually do away with it. (I wonder why it was introduced and when
> > it actually stopped being useful.)
>
> It was for the C++98 notion of constant-expression, which was more of a
> parser-level notion, and has been supplanted by the C++11 version. I'm
> happy to remove it, and therefore remove the is_rvalue_constant_expression
> call.
Wonderful. I'll do that next.
> > > > -- >8 --
> > > >
> > > > is_really_empty_class is liable to crash when it gets an incomplete
> > > > or dependent type. Since r11-557, we pass the yet-uninstantiated
> > > > class type S<0> of the PARM_DECL s to is_really_empty_class -- because
> > > > of the potential_rvalue_constant_expression ->
> > > > is_rvalue_constant_expression
> > > > change in cp_parser_constant_expression. Here we're not parsing
> > > > a template so we did not check COMPLETE_TYPE_P as we should.
> > > >
> > > > PR c++/110106
> > > >
> > > > gcc/cp/ChangeLog:
> > > >
> > > > * constexpr.cc (potential_constant_expression_1): Check
> > > > COMPLETE_TYPE_P
> > > > even when !processing_template_decl.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * g++.dg/cpp0x/noexcept80.C: New test.
> > > > ---
> > > > gcc/cp/constexpr.cc | 2 +-
> > > > gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
> > > > 2 files changed, 13 insertions(+), 1 deletion(-)
> > > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> > > >
> > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > > > index 6e8f1c2b61e..1f59c5472fb 100644
> > > > --- a/gcc/cp/constexpr.cc
> > > > +++ b/gcc/cp/constexpr.cc
> > > > @@ -9116,7 +9116,7 @@ potential_constant_expression_1 (tree t, bool
> > > > want_rval, bool strict, bool now,
> > > > if (now && want_rval)
> > > > {
> > > > tree type = TREE_TYPE (t);
> > > > - if ((processing_template_decl && !COMPLETE_TYPE_P (type))
> > > > + if (!COMPLETE_TYPE_P (type)
> > > > || dependent_type_p (type)
>
> There shouldn't be a problem completing the type here, so it seems to me
> that we're missing a call to complete_type_p, at least when
> !processing_template_decl. Probably need to move the dependent_type_p check
> up as a result.
Like so?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
is_really_empty_class is liable to crash when it gets an incomplete
or dependent type. Since r11-557, we pass the yet-uninstantiated
class type S<0> of the PARM_DECL s to is_really_empty_class -- because
of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
change in cp_parser_constant_expression. Here we're not parsing
a template so we did not check COMPLETE_TYPE_P as we should.
It should work to complete the type before checking COMPLETE_TYPE_P.
PR c++/110106
gcc/cp/ChangeLog:
* constexpr.cc (potential_constant_expression_1): Try to complete the
type when !processing_template_decl.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/noexcept80.C: New test.
---
gcc/cp/constexpr.cc | 5 +++--
gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
2 files changed, 15 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On 7/20/23 14:13, Marek Polacek wrote:
On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote:
On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches?
Looks reasonable to me.
Thanks.
Though I wonder if we could also fix this by not checking potentiality
at all in this case? The problematic call to is_rvalue_constant_expression
happens from cp_parser_constant_expression with 'allow_non_constant' != 0
and with 'non_constant_p' being a dummy out argument that comes from
cp_parser_functional_cast, so the result of is_rvalue_constant_expression
is effectively unused in this case, and we should be able to safely elide
it when 'allow_non_constant && non_constant_p == nullptr'.
Sounds plausible. I think my patch could be applied first since it
removes a tiny bit of code, then I can hopefully remove the flag below,
then maybe go back and optimize the call to is_rvalue_constant_expression.
Does that sound sensible?
Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p
is also effectively unused and could be removed?
It looks that way. Seems it's only used in cp_parser_constant_expression:
10806 if (allow_non_constant_p)
10807 *non_constant_p = parser->non_integral_constant_expression_p;
but that could be easily replaced by a local var. I'd be happy to see if
we can actually do away with it. (I wonder why it was introduced and when
it actually stopped being useful.)
It was for the C++98 notion of constant-expression, which was more of a
parser-level notion, and has been supplanted by the C++11 version. I'm
happy to remove it, and therefore remove the
is_rvalue_constant_expression call.
-- >8 --
is_really_empty_class is liable to crash when it gets an incomplete
or dependent type. Since r11-557, we pass the yet-uninstantiated
class type S<0> of the PARM_DECL s to is_really_empty_class -- because
of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
change in cp_parser_constant_expression. Here we're not parsing
a template so we did not check COMPLETE_TYPE_P as we should.
PR c++/110106
gcc/cp/ChangeLog:
* constexpr.cc (potential_constant_expression_1): Check COMPLETE_TYPE_P
even when !processing_template_decl.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/noexcept80.C: New test.
---
gcc/cp/constexpr.cc | 2 +-
gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 6e8f1c2b61e..1f59c5472fb 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -9116,7 +9116,7 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
if (now && want_rval)
{
tree type = TREE_TYPE (t);
- if ((processing_template_decl && !COMPLETE_TYPE_P (type))
+ if (!COMPLETE_TYPE_P (type)
|| dependent_type_p (type)
There shouldn't be a problem completing the type here, so it seems to me
that we're missing a call to complete_type_p, at least when
!processing_template_decl. Probably need to move the dependent_type_p
check up as a result.
Jason
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On Wed, Jul 19, 2023 at 10:11:27AM -0400, Patrick Palka wrote:
> On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
>
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches?
>
> Looks reasonable to me.
Thanks.
> Though I wonder if we could also fix this by not checking potentiality
> at all in this case? The problematic call to is_rvalue_constant_expression
> happens from cp_parser_constant_expression with 'allow_non_constant' != 0
> and with 'non_constant_p' being a dummy out argument that comes from
> cp_parser_functional_cast, so the result of is_rvalue_constant_expression
> is effectively unused in this case, and we should be able to safely elide
> it when 'allow_non_constant && non_constant_p == nullptr'.
Sounds plausible. I think my patch could be applied first since it
removes a tiny bit of code, then I can hopefully remove the flag below,
then maybe go back and optimize the call to is_rvalue_constant_expression.
Does that sound sensible?
> Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p
> is also effectively unused and could be removed?
It looks that way. Seems it's only used in cp_parser_constant_expression:
10806 if (allow_non_constant_p)
10807 *non_constant_p = parser->non_integral_constant_expression_p;
but that could be easily replaced by a local var. I'd be happy to see if
we can actually do away with it. (I wonder why it was introduced and when
it actually stopped being useful.)
Thanks,
> >
> > -- >8 --
> >
> > is_really_empty_class is liable to crash when it gets an incomplete
> > or dependent type. Since r11-557, we pass the yet-uninstantiated
> > class type S<0> of the PARM_DECL s to is_really_empty_class -- because
> > of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
> > change in cp_parser_constant_expression. Here we're not parsing
> > a template so we did not check COMPLETE_TYPE_P as we should.
> >
> > PR c++/110106
> >
> > gcc/cp/ChangeLog:
> >
> > * constexpr.cc (potential_constant_expression_1): Check COMPLETE_TYPE_P
> > even when !processing_template_decl.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/noexcept80.C: New test.
> > ---
> > gcc/cp/constexpr.cc | 2 +-
> > gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> >
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 6e8f1c2b61e..1f59c5472fb 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -9116,7 +9116,7 @@ potential_constant_expression_1 (tree t, bool
> > want_rval, bool strict, bool now,
> >if (now && want_rval)
> > {
> > tree type = TREE_TYPE (t);
> > - if ((processing_template_decl && !COMPLETE_TYPE_P (type))
> > + if (!COMPLETE_TYPE_P (type)
> > || dependent_type_p (type)
> > || is_really_empty_class (type, /*ignore_vptr*/false))
> > /* An empty class has no data to read. */
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> > b/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> > new file mode 100644
> > index 000..3e90af747e2
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/110106
> > +// { dg-do compile { target c++11 } }
> > +
> > +template struct S
> > +{
> > +};
> > +
> > +struct G {
> > + G(S<0>);
> > +};
> > +
> > +void y(S<0> s) noexcept(noexcept(G{s}));
> >
> > base-commit: fca089e8a47314a40ad93527ba9f9d0d374b3afb
> > --
> > 2.41.0
> >
> >
>
Marek
Re: [PATCH] c++: fix ICE with is_really_empty_class [PR110106]
On Tue, 18 Jul 2023, Marek Polacek via Gcc-patches wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches?
Looks reasonable to me.
Though I wonder if we could also fix this by not checking potentiality
at all in this case? The problematic call to is_rvalue_constant_expression
happens from cp_parser_constant_expression with 'allow_non_constant' != 0
and with 'non_constant_p' being a dummy out argument that comes from
cp_parser_functional_cast, so the result of is_rvalue_constant_expression
is effectively unused in this case, and we should be able to safely elide
it when 'allow_non_constant && non_constant_p == nullptr'.
Relatedly, ISTM the member cp_parser::non_integral_constant_expression_p
is also effectively unused and could be removed?
>
> -- >8 --
>
> is_really_empty_class is liable to crash when it gets an incomplete
> or dependent type. Since r11-557, we pass the yet-uninstantiated
> class type S<0> of the PARM_DECL s to is_really_empty_class -- because
> of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
> change in cp_parser_constant_expression. Here we're not parsing
> a template so we did not check COMPLETE_TYPE_P as we should.
>
> PR c++/110106
>
> gcc/cp/ChangeLog:
>
> * constexpr.cc (potential_constant_expression_1): Check COMPLETE_TYPE_P
> even when !processing_template_decl.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/noexcept80.C: New test.
> ---
> gcc/cp/constexpr.cc | 2 +-
> gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
> 2 files changed, 13 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
>
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 6e8f1c2b61e..1f59c5472fb 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -9116,7 +9116,7 @@ potential_constant_expression_1 (tree t, bool
> want_rval, bool strict, bool now,
>if (now && want_rval)
> {
> tree type = TREE_TYPE (t);
> - if ((processing_template_decl && !COMPLETE_TYPE_P (type))
> + if (!COMPLETE_TYPE_P (type)
> || dependent_type_p (type)
> || is_really_empty_class (type, /*ignore_vptr*/false))
> /* An empty class has no data to read. */
> diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> b/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> new file mode 100644
> index 000..3e90af747e2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
> @@ -0,0 +1,12 @@
> +// PR c++/110106
> +// { dg-do compile { target c++11 } }
> +
> +template struct S
> +{
> +};
> +
> +struct G {
> + G(S<0>);
> +};
> +
> +void y(S<0> s) noexcept(noexcept(G{s}));
>
> base-commit: fca089e8a47314a40ad93527ba9f9d0d374b3afb
> --
> 2.41.0
>
>
[PATCH] c++: fix ICE with is_really_empty_class [PR110106]
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and branches?
-- >8 --
is_really_empty_class is liable to crash when it gets an incomplete
or dependent type. Since r11-557, we pass the yet-uninstantiated
class type S<0> of the PARM_DECL s to is_really_empty_class -- because
of the potential_rvalue_constant_expression -> is_rvalue_constant_expression
change in cp_parser_constant_expression. Here we're not parsing
a template so we did not check COMPLETE_TYPE_P as we should.
PR c++/110106
gcc/cp/ChangeLog:
* constexpr.cc (potential_constant_expression_1): Check COMPLETE_TYPE_P
even when !processing_template_decl.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/noexcept80.C: New test.
---
gcc/cp/constexpr.cc | 2 +-
gcc/testsuite/g++.dg/cpp0x/noexcept80.C | 12
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept80.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 6e8f1c2b61e..1f59c5472fb 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -9116,7 +9116,7 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
if (now && want_rval)
{
tree type = TREE_TYPE (t);
- if ((processing_template_decl && !COMPLETE_TYPE_P (type))
+ if (!COMPLETE_TYPE_P (type)
|| dependent_type_p (type)
|| is_really_empty_class (type, /*ignore_vptr*/false))
/* An empty class has no data to read. */
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
b/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
new file mode 100644
index 000..3e90af747e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept80.C
@@ -0,0 +1,12 @@
+// PR c++/110106
+// { dg-do compile { target c++11 } }
+
+template struct S
+{
+};
+
+struct G {
+ G(S<0>);
+};
+
+void y(S<0> s) noexcept(noexcept(G{s}));
base-commit: fca089e8a47314a40ad93527ba9f9d0d374b3afb
--
2.41.0
