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

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

On 9/5/23 15:59, Marek Polacek wrote:

On Tue, Sep 05, 2023 at 10:52:04AM -0400, Jason Merrill wrote:

On 9/1/23 13:23, Marek Polacek wrote:

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 as much
code as we did before: uninstantiated templates


That's acceptable, it's an optional diagnostic.


and things like "false ? foo () : 1".


This is a problem.  Maybe we want cp_fold_r to recurse into the arms of a
COND_EXPR before folding them away?  Maybe only if we know we've seen an
immediate function?


Unfortunately we had already thrown the dead branch away when we got to
cp_fold_r.  I wonder if we have to adjust cxx_eval_conditional_expression
to call cp_fold_r on the dead branch too,


Hmm, I guess so.


perhaps with a new ff_ flag to skip the whole second switch in cp_fold_r?


Or factor out the immediate function handling to a separate walk 
function that cp_fold_r also calls?



But then it's possible that the in_immediate_context checks have to stay.


We can just not do the walk in immediate (or mce_true) context, like we 
currently avoid calling cp_fold_function.  For mce_unknown I guess we'd 
want to set *non_constant_p instead of giving an error.


Jason

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

2023-09-05 Thread Marek Polacek via Gcc-patches 
On Tue, Sep 05, 2023 at 10:52:04AM -0400, Jason Merrill wrote:
> On 9/1/23 13:23, Marek Polacek wrote:
> > 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 as much
> > code as we did before: uninstantiated templates
> 
> That's acceptable, it's an optional diagnostic.
> 
> > and things like "false ? foo () : 1".
> 
> This is a problem.  Maybe we want cp_fold_r to recurse into the arms of a
> COND_EXPR before folding them away?  Maybe only if we know we've seen an
> immediate function?

Unfortunately we had already thrown the dead branch away when we got to
cp_fold_r.  I wonder if we have to adjust cxx_eval_conditional_expression
to call cp_fold_r on the dead branch too, perhaps with a new ff_ flag
to skip the whole second switch in cp_fold_r?  But then it's possible
that the in_immediate_context checks have to stay.

[ I'll address the rest later. ]

Marek

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

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

On 9/1/23 13:23, Marek Polacek wrote:

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 as much
code as we did before: uninstantiated templates


That's acceptable, it's an optional diagnostic.


and things like "false ? foo () : 1".


This is a problem.  Maybe we want cp_fold_r to recurse into the arms of 
a COND_EXPR before folding them away?  Maybe only if we know we've seen 
an immediate function?



diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 8bd5c4a47f8..af4f98b1fe1 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3135,6 +3135,11 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
  unsigned save_heap_alloc_count = ctx->global->heap_vars.length ();
  unsigned save_heap_dealloc_count = ctx->global->heap_dealloc_count;
  
+	  /* Make sure we fold std::is_constant_evaluated to true in an

+immediate function.  */
+ if (immediate_invocation_p (fun))


I think this should just check DECL_IMMEDIATE_FUNCTION_P, the context 
doesn't matter.



+   call_ctx.manifestly_const_eval = mce_true;
+
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 206e791fcfd..29132aad158 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1058,9 +1058,21 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
}
break;
  
+/* Expand immediate invocations.  */

+case CALL_EXPR:
+case AGGR_INIT_EXPR:
+  if (!in_immediate_context ())


As you mentioned in your followup, we shouldn't need to check this 
because we don't call cp_fold_r in immediate context.



+   if (tree fn = cp_get_callee (stmt))
+ if (TREE_CODE (fn) != ADDR_EXPR || ADDR_EXPR_DENOTES_CALL_P (fn))
+   if (tree fndecl = cp_get_fndecl_from_callee (fn, /*fold*/false))
+ if (DECL_IMMEDIATE_FUNCTION_P (fndecl))
+   *stmt_p = stmt = cxx_constant_value (stmt);
+  break;
+
  case ADDR_EXPR:
if (TREE_CODE (TREE_OPERAND (stmt, 0)) == FUNCTION_DECL
- && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0)))
+ && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0))
+ && !in_immediate_context ())


Likewise.


diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 799183dc646..7dfb6de2da3 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -3254,7 +3254,7 @@ bot_manip (tree* tp, int* walk_subtrees, void* data_)
 variables.  */
  
  static tree

-bot_replace (tree* t, int* walk_subtrees, void* data_)
+bot_replace (tree* t, int*, void* data_)


Generally we keep the parameter name as a comment like
int */*walk_subtrees*/


diff --git a/gcc/testsuite/g++.dg/cpp23/consteval-if2.C 
b/gcc/testsuite/g++.dg/cpp23/consteval-if2.C
index d1845da9e58..9fa95295c43 100644
--- a/gcc/testsuite/g++.dg/cpp23/consteval-if2.C
+++ b/gcc/testsuite/g++.dg/cpp23/consteval-if2.C
@@ -65,7 +65,7 @@ qux (int x)
int r = 0;
if not consteval// { dg-warning "'if consteval' only available with" "" 
{ target c++20_only } }
  {
-  r += foo (x);// { dg-error "'x' is not a constant expression" }
+  r += foo (x);// { dg-error "'x' is not a constant expression" "" { 
xfail *-*-* } }


This whole function should have a comment that these errors are not 
required because qux is never instantiated.



diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval11.C 
b/gcc/testsuite/g++.dg/cpp2a/consteval11.C
index 2f68ec0f892..9fd32dcab7b 100644
--- a/gcc/testsuite/g++.dg/cpp2a/consteval11.C
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval11.C
@@ -5,25 +5,25 @@ consteval int bar (int i) { if (i != 1) throw 1; return 0; }  // { 
dg-error "is n
  
  constexpr int a = bar (1);

  constexpr int b = bar (2);// { dg-message "in 'constexpr' expansion 
of" }
-constexpr int c = 0 ? bar (3) : 1; // { dg-message "in 'constexpr' expansion 
of" }
+constexpr int c = 0 ? bar (3) : 1;


As discussed above, we need to keep this diagnostic and the others like it.

Let's also add a test with the

template 
constexpr bool is_not(T t, F f) {
return not f(t);
}

consteval bool is_even(int i) { return i % 2 == 0; }

static_assert(is_not(5, is_even)); // ok

example from the paper.

Jason

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

2023-09-01 Thread Marek Polacek via Gcc-patches 
On Fri, Sep 01, 2023 at 01:23:48PM -0400, Marek Polacek via Gcc-patches wrote:
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
[...]
>  case ADDR_EXPR:
>if (TREE_CODE (TREE_OPERAND (stmt, 0)) == FUNCTION_DECL
> -   && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0)))
> +   && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0))
> +   && !in_immediate_context ())

This hunk isn't actually necessary.  I'm happy to drop it.  Or add the
in_immediate_context check into case PTRMEM_CST too.

Marek

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

2023-09-01 Thread Marek Polacek via Gcc-patches 
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 as much
code as we did before: uninstantiated templates and things like
"false ? foo () : 1".

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

  *()) ()
  (s.*::foo) ()

which were deemed ill-formed.

gcc/cp/ChangeLog:

* call.cc (in_immediate_context): No longer static.
(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
immediate_invocation_p.
* cp-gimplify.cc (cp_fold_r): Expand immediate invocations.
* cp-tree.h (ADDR_EXPR_DENOTES_CALL_P): Define.
(immediate_invocation_p): Declare.
* tree.cc (bot_replace): Don't handle immediate invocations here.

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.

libstdc++-v3/ChangeLog:

* testsuite/20_util/allocator/105975.cc: Add dg-error.
---
 gcc/cp/call.cc| 42 +++
 gcc/cp/constexpr.cc   |  5 +++
 gcc/cp/cp-gimplify.cc | 14 ++-
 gcc/cp/cp-tree.h  |  6 +++
 gcc/cp/tree.cc| 23 +-
 gcc/testsuite/g++.dg/cpp23/consteval-if2.C|  2 +-
 gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C |  7 
 gcc/testsuite/g++.dg/cpp2a/consteval11.C  | 37 
 gcc/testsuite/g++.dg/cpp2a/consteval3.C   |  3 +-
 gcc/testsuite/g++.dg/cpp2a/consteval32.C  |  4 ++
 gcc/testsuite/g++.dg/cpp2a/consteval33.C  | 34 +++
 gcc/testsuite/g++.dg/cpp2a/consteval9.C   |  2 +-
 .../testsuite/20_util/allocator/105975.cc |  2 +-
 13 files changed, 100 insertions(+), 81 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval32.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval33.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 40d9fdc0516..abdbc8fff8c 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9763,7 +9763,7 @@ in_immediate_context ()
 /* Return true if a call to FN with number of arguments NARGS
is an immediate invocation.  */
 
-static bool
+bool
 immediate_invocation_p (tree fn)
 {
   return (TREE_CODE (fn) == FUNCTION_DECL
@@ -10471,6 +10471,10 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
   fn = build_addr_func (fn, complain);
   if (fn == error_mark_node)
return error_mark_node;
+
+  /* We're actually invoking the function.  (Immediate functions get an
+& when invoking it even though the user didn't use &.)  */
+  ADDR_EXPR_DENOTES_CALL_P (fn) = true;
 }
 
   tree call = build_cxx_call (fn, nargs, argarray, complain|decltype_flag);
@@ -10488,41 +10492,7 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
   if (TREE_CODE (c) == CALL_EXPR)
suppress_warning (c /* Suppress all warnings.  */);
 }
-  if (TREE_CODE (fn) == ADDR_EXPR)
-{
-  tree fndecl = STRIP_TEMPLATE (TREE_OPERAND (fn, 0));
-  if (immediate_invocation_p (fndecl))
-   {
- tree obj_arg = NULL_TREE;
- /* Undo convert_from_reference called by build_cxx_call.  */
- if (REFERENCE_REF_P (call))
-   call = TREE_OPERAND (call, 0);
- if (DECL_CONSTRUCTOR_P (fndecl))
-   obj_arg = cand->first_arg ? cand->first_arg : (*args)[0];
- if (obj_arg && is_dummy_object (obj_arg))
-   {
- call = build_cplus_new (DECL_CONTEXT (fndecl), call, complain);
- obj_arg = NULL_TREE;
-   }
- /* Look through *(const T *)  */
- else if (obj_arg && INDIRECT_REF_P (obj_arg))
-   {
- tree addr = TREE_OPERAND (obj_arg, 0);
- STRIP_NOPS (addr);
- if (TREE_CODE (addr) == ADDR_EXPR)
-   {
- tree typeo = TREE_TYPE (obj_arg);
- tree typei = TREE_TYPE (TREE_OPERAND (addr, 0));
- if (same_type_ignoring_top_level_qualifiers_p