Re: [PATCH v7] c++: Move consteval folding to cp_fold_r

2023-09-19 Thread Jason Merrill

On 9/19/23 09:01, Marek Polacek wrote:

On Mon, Sep 18, 2023 at 09:36:31PM -0400, Jason Merrill wrote:

On 9/18/23 17:42, Marek Polacek wrote:

+  /* The purpose of this is not to emit errors for mce_unknown.  */
+  const tsubst_flags_t complain = (data->flags == ff_fold_immediate
+  ? tf_none : tf_error);


Maybe check flags & ff_mce_false, instead?  OK with that change.


Thanks!  And what do you think about:

--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1162,7 +1162,8 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
tree stmt = *stmt_p;
enum tree_code code = TREE_CODE (stmt);

-  cp_fold_immediate_r (stmt_p, walk_subtrees, data);
+  if (cxx_dialect > cxx17)
+cp_fold_immediate_r (stmt_p, walk_subtrees, data);

*stmt_p = stmt = cp_fold (*stmt_p, data->flags);

since we can recurse on ?:, this could save some time.  It's sad that
it checks cxx_dialect in every invocation of cp_fold_r but still, it
should help.


Sure.

Jason



Re: [PATCH v7] c++: Move consteval folding to cp_fold_r

2023-09-19 Thread Marek Polacek
On Mon, Sep 18, 2023 at 09:36:31PM -0400, Jason Merrill wrote:
> On 9/18/23 17:42, Marek Polacek wrote:
> > +  /* The purpose of this is not to emit errors for mce_unknown.  */
> > +  const tsubst_flags_t complain = (data->flags == ff_fold_immediate
> > +  ? tf_none : tf_error);
> 
> Maybe check flags & ff_mce_false, instead?  OK with that change.

Thanks!  And what do you think about:

--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1162,7 +1162,8 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
   tree stmt = *stmt_p;
   enum tree_code code = TREE_CODE (stmt);

-  cp_fold_immediate_r (stmt_p, walk_subtrees, data);
+  if (cxx_dialect > cxx17)
+cp_fold_immediate_r (stmt_p, walk_subtrees, data);

   *stmt_p = stmt = cp_fold (*stmt_p, data->flags);


since we can recurse on ?:, this could save some time.  It's sad that
it checks cxx_dialect in every invocation of cp_fold_r but still, it
should help.

Thanks again for the reviews,

Marek



Re: [PATCH v7] c++: Move consteval folding to cp_fold_r

2023-09-18 Thread Jason Merrill via Gcc-patches

On 9/18/23 17:42, Marek Polacek wrote:

+  /* The purpose of this is not to emit errors for mce_unknown.  */
+  const tsubst_flags_t complain = (data->flags == ff_fold_immediate
+  ? tf_none : tf_error);


Maybe check flags & ff_mce_false, instead?  OK with that change.

Thanks,
Jason



[PATCH v7] c++: Move consteval folding to cp_fold_r

2023-09-18 Thread Marek Polacek via Gcc-patches
On Sat, Sep 16, 2023 at 04:22:44PM -0400, Jason Merrill wrote:
> On 9/15/23 16:32, Marek Polacek wrote:
> > On Fri, Sep 15, 2023 at 02:08:46PM -0400, Jason Merrill wrote:
> > > On 9/13/23 20:02, Marek Polacek wrote:
> > > > On Wed, Sep 13, 2023 at 05:57:47PM -0400, Jason Merrill wrote:
> > > > > On 9/13/23 16:56, Marek Polacek wrote:
> > > > > > On Tue, Sep 12, 2023 at 05:26:25PM -0400, Jason Merrill wrote:
> > > > > > > On 9/8/23 14:24, Marek Polacek wrote:
> > > > > > > > +  switch (TREE_CODE (stmt))
> > > > > > > > +{
> > > > > > > > +/* Unfortunately we must handle code like
> > > > > > > > +false ? bar () : 42
> > > > > > > > +   where we have to check bar too.  */
> > > > > > > > +case COND_EXPR:
> > > > > > > > +  if (cp_fold_immediate_r (&TREE_OPERAND (stmt, 1), 
> > > > > > > > walk_subtrees, data))
> > > > > > > > +   return error_mark_node;
> > > > > > > > +  if (TREE_OPERAND (stmt, 2)
> > > > > > > > + && cp_fold_immediate_r (&TREE_OPERAND (stmt, 2), 
> > > > > > > > walk_subtrees, data))
> > > > > > > > +   return error_mark_node;
> > > > > > > 
> > > > > > > Is this necessary?  Doesn't walk_tree already walk into the arms 
> > > > > > > of
> > > > > > > COND_EXPR?
> > > > > > 
> > > > > > Unfortunately yes.  The cp_fold call in cp_fold_r could fold the ?: 
> > > > > > into
> > > > > > a constant before we see it here.  I've added a comment saying just 
> > > > > > that.
> > > > > 
> > > > > Ah.  But in that case I guess we need to walk into the arms, not just 
> > > > > check
> > > > > the top-level expression in them.
> > > > Arg, of course.  I was fooled into thinking that it would recurse, but
> > > > you're right.  Fixed by using cp_walk_tree as I intended.  Tested in
> > > > consteval34.C.
> > > > 
> > > > > But maybe cp_fold_r should do that before the cp_fold, instead of this
> > > > > function?
> > > > 
> > > > I...am not sure how that would be better than what I did.
> > > 
> > > Callers of cp_fold_immediate don't need this because cp_fold_r isn't
> > > involved, so it isn't folding anything.
> > 
> > This is true.
> > > cp_fold_r can walk the arms with cp_fold_r and then clear *walk_subtrees 
> > > to
> > > avoid walking the arms again normally.
> > 
> > I didn't think we wanted to do everything cp_fold_r does even in dead
> > branches, but ok.
> 
> Ah, that's a good point.  With the recursive walk in cp_fold_immediate_r, I
> suppose we could suppress it when called from cp_fold_immediate with a new
> fold_flag?  That would still allow for cp_walk_tree_without_duplicates.

Yeah, I like that.  Done here.
 
> Incidentally, I notice you check for null op2 of COND_EXPR, should probably
> also check op1.

Added, though I'm not sure it's necessary.  I've added some more tests to
consteval34.C to also check for a ?: c.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
In the review of P2564:

it turned out that in order to correctly handle an example in the paper,
we should stop doing immediate evaluation in build_over_call and
bot_replace, and instead do it in cp_fold_r.  This patch does that.

Another benefit is that this is a pretty significant simplification, at
least in my opinion.  Also, this fixes the c++/110997 ICE (but the test
doesn't compile yet).

The main drawback seems to be that cp_fold_r doesn't process
uninstantiated templates.  We still have to handle things like
"false ? foo () : 1".  To that end, I've added cp_fold_immediate, called
on dead branches in cxx_eval_conditional_expression.

You'll see that I've reintroduced ADDR_EXPR_DENOTES_CALL_P here.  This
is to detect

  *(&foo)) ()
  (s.*&S::foo) ()

which were deemed ill-formed.

gcc/cp/ChangeLog:

* call.cc (build_over_call): Set ADDR_EXPR_DENOTES_CALL_P.  Don't handle
immediate_invocation_p here.
* constexpr.cc (cxx_eval_call_expression): Use mce_true for
DECL_IMMEDIATE_FUNCTION_P.
(cxx_eval_conditional_expression): Call cp_fold_immediate.
* cp-gimplify.cc (enum fold_flags): Add ff_fold_immediate.
(maybe_replace_decl): Make static.
(cp_fold_r): Expand immediate invocations.
(cp_fold_immediate_r): New.
(cp_fold_immediate): New.
* cp-tree.h (ADDR_EXPR_DENOTES_CALL_P): Define.
(cp_fold_immediate): Declare.
* tree.cc (bot_replace): Don't handle immediate invocations here.

libstdc++-v3/ChangeLog:

* testsuite/20_util/allocator/105975.cc: Add dg-error.

gcc/testsuite/ChangeLog:

* g++.dg/cpp23/consteval-if2.C: Add xfail.
* g++.dg/cpp2a/consteval-memfn1.C: Adjust.
* g++.dg/cpp2a/consteval11.C: Remove dg-message.
* g++.dg/cpp2a/consteval3.C: Remove dg-message and dg-error.
* g++.dg/cpp2a/consteval9.C: Remove dg-message.
* g++.dg/cpp2a/consteval32.C: New test.
* g++.dg/cpp2a/consteval33.C: New test.
* g++.dg/cpp2a/co