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;
- saved_non_integral_constant_expression_p =
parser->non_integral_constant_expression_p;
+ bool saved_non_integral_constant_expression_p
+ = parser->non_integral_constant_expression_p;
/* We are now parsing a constant-expression. */
parser->integral_constant_expression_p = true;
parser->allow_non_integral_constant_expression_p
@@ -10780,6 +10777,7 @@ cp_parser_constant_expression (cp_parser* parser,
For example, cp_parser_initializer_clauses uses this function to
determine whether a particular assignment-expression is in fact
constant. */
+ cp_expr expression;
if (strict_p)
expression = cp_parser_conditional_expression (parser);
else
@@ -10789,7 +10787,8 @@ cp_parser_constant_expression (cp_parser* parser,
= saved_integral_constant_expression_p;
parser->allow_non_integral_constant_expression_p
= saved_allow_non_integral_constant_expression_p;
- if (cxx_dialect >= cxx11)
+ if (cxx_dialect >= cxx11
+ && (!allow_non_constant_p || non_constant_p))
{
/* Require an rvalue constant expression here; that's what our
callers expect. Reference constant expressions are handled
@@ -10803,7 +10802,7 @@ cp_parser_constant_expression (cp_parser* parser,
if (!is_const && !allow_non_constant_p)
require_rvalue_constant_expression (decay);
}
- if (allow_non_constant_p)
+ if (allow_non_constant_p && non_constant_p)
*non_constant_p = parser->non_integral_constant_expression_p;
parser->non_integral_constant_expression_p
= saved_non_integral_constant_expression_p;
@@ -16400,12 +16399,11 @@ cp_parser_linkage_specification (cp_parser* parser,
tree prefix_attr)
If MEMBER_P, this static_assert is a class member. */
static void
-cp_parser_static_assert(cp_parser *parser, bool member_p)
+cp_parser_static_assert (cp_parser *parser, bool member_p)
{
cp_expr condition;
location_t token_loc;
tree message;
- bool dummy;
/* Peek at the `static_assert' token so we can keep track of exactly
where the static assertion started. */
@@ -16430,7 +16428,7 @@ cp_parser_static_assert(cp_parser *parser, bool
member_p)
condition =
cp_parser_constant_expression (parser,
/*allow_non_constant_p=*/true,
- /*non_constant_p=*/&dummy);
+ /*non_constant_p=*/nullptr);
if (cp_lexer_peek_token (parser->lexer)->type == CPP_CLOSE_PAREN)
{
base-commit: 6e424febfbcb27c21a7fe3a137e614765f9cf9d2
