Re: [PATCH] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-08-29 Thread Jason Merrill via Gcc-patches

On 8/23/23 15:49, Marek Polacek wrote:

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

-- >8 --

This patch implements P2564, described at , whereby
certain functions are promoted to consteval.  For example:


Great, thanks!


   consteval int id(int i) { return i; }

   template 
   constexpr int f(T t)
   {
 return t + id(t); // id causes f to be promoted to consteval
   }

   void g(int i)
   {
 f (3);
   }

now compiles.  Previously the code was ill-formed: we would complain
that 't' in 'f' is not a constant expression.  Since 'f' is now
consteval, it means that the call to id(t) is in an immediate context,
so doesn't have to produce a constant -- this is how we allow consteval
functions composition.  But making 'f' consteval also means that
the call to 'f' in 'g' must yield a constant; failure to do so results
in an error.  I made the effort to have cc1plus explain to us what's
going on.  For example, calling f(i) produces this neat diagnostic:

q.C: In function 'void g(int)':
q.C:11:5: error: call to consteval function 'f(i)' is not a constant 
expression
11 |   f (i);
   |   ~~^~~
q.C:6:16: note: 'constexpr int f(T) [with T = int]' was promoted to an 
immediate function because its body contains an immediate-escalating expression 
'id(t)'
 6 |   return t + id(t);
   |  ~~^~~

which hopefully makes it clear what's going on.

Implementing this proposal has been tricky.  One problem was delayed
instantiation: instantiating a function can set off a domino effect
where one call promotes a function to consteval but that then means
that another function should also be promoted, etc.


What I expected was that we would instantiate when we see a call, i.e. 
immediate_invocation_p would instantiate its argument if it's an 
immediate-escalating function.  But...



I previously thought that I could get past that by implementing the propagation 
in
cp_gimplify_expr at which point we have already instantiated everything
via instantiate_pending_templates.


...this sounds like a clever way to avoid the problems we tend to see 
with eager instantiation (e.g. constexpr-inst1.C).  It still seems 
promising to me.



But I realized that we don't gimplify e.g.

   static auto p = 


Indeed, just cp_fully_fold_init them, at least until we build the global 
init function.



and so we'd never detect taking the address of a consteval function.
Therefore this patch instantiates immediate-escalating functions
beforehand.


Relatedly, in one of the testcases you have


+template 
+constexpr int f2(T);
+
+// ??? clang++ emits
+// error: immediate function 'f2' used before it is defined
+// error: immediate function 'f2' used before it is defined
+// but at this point we don't know that f2 will be updated to consteval?
+auto a2 = ;
+auto b2 = ;
+
+template 
+constexpr int f2(T t) {
+return id(t);


This is a case where we can't immediately resolve the question anyway, 
we need to remember where we've seen bare references to 
immediate-escalating functions outside of other immediate-escalating (or 
already immediate) functions, and complain later if they became 
consteval when instantiated.


I don't see why this is a significant obstacle to doing escalation late.


* cp-tree.h (ADDR_EXPR_DENOTES_CALL_P): Define.


Is this flag just for rejecting consteval-prop10.C?  I think we should 
not try to reject it.  This came up previously in the context of my 
patch for CWG2631, at which point I argued to CWG that it should be 
allowed, but nobody responded.  I'll ping that thread now, but in the 
mean time let's not go out of our way to reject this testcase that seems 
to me pretty clearly allowed by the current wording: that expression 
invokes an immediate function, so it's an immediate invocation.



diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 673ec91d60e..31f78b71fe1 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9735,7 +9735,9 @@ build_trivial_dtor_call (tree instance, bool no_ptr_deref)
  
  /* Return true if in an immediate function context, or an unevaluated operand,

 or a default argument/member initializer, or a subexpression of an 
immediate
-   invocation.  */
+   invocation.
+   ??? P2564 says that "a subexpression of a manifestly constant-evaluated
+   expression or conversion" is also an immediate function context.  */


Yes, that was to allow the consteval-prop2.C

  static_assert(is_not(5, is_even));

Since the operand of static_assert is manifestly constant-evaluated, 
it's OK that we name consteval is_even there.


We currently use this function to trigger evaluating immediate 
evaluations, and later (through immediate_invocation_p) to trigger 
errors, and other constant-evaluation happens between those two, so in 
most cases it's OK that this function does not identify that situation.


But there's another example in the paper, that this patch doesn't handle:

consteval int g(int p) { return p; }
template constexpr auto f(T) { 

[PATCH] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-08-23 Thread Marek Polacek via Gcc-patches
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --

This patch implements P2564, described at , whereby
certain functions are promoted to consteval.  For example:

  consteval int id(int i) { return i; }

  template 
  constexpr int f(T t)
  {
return t + id(t); // id causes f to be promoted to consteval
  }

  void g(int i)
  {
f (3);
  }

now compiles.  Previously the code was ill-formed: we would complain
that 't' in 'f' is not a constant expression.  Since 'f' is now
consteval, it means that the call to id(t) is in an immediate context,
so doesn't have to produce a constant -- this is how we allow consteval
functions composition.  But making 'f' consteval also means that
the call to 'f' in 'g' must yield a constant; failure to do so results
in an error.  I made the effort to have cc1plus explain to us what's
going on.  For example, calling f(i) produces this neat diagnostic:

q.C: In function 'void g(int)':
q.C:11:5: error: call to consteval function 'f(i)' is not a constant 
expression
   11 |   f (i);
  |   ~~^~~
q.C:6:16: note: 'constexpr int f(T) [with T = int]' was promoted to an 
immediate function because its body contains an immediate-escalating expression 
'id(t)'
6 |   return t + id(t);
  |  ~~^~~

which hopefully makes it clear what's going on.

Implementing this proposal has been tricky.  One problem was delayed
instantiation: instantiating a function can set off a domino effect
where one call promotes a function to consteval but that then means
that another function should also be promoted, etc.  I previously
thought that I could get past that by implementing the propagation in
cp_gimplify_expr at which point we have already instantiated everything
via instantiate_pending_templates.  But I realized that we don't
gimplify e.g.

  static auto p = 

and so we'd never detect taking the address of a consteval function.
Therefore this patch instantiates immediate-escalating functions
beforehand.  And as usual, there were a lot of other things to
handle.  It's not just calls to consteval functions that we must
detect, we also have to look for id-expressions that denote an
immediate function.

I discovered two crashes:
, ICE-on-valid with NSDMI
, missing ; causes an ICE
which this patch doesn't address, but adds a dg-ice test for the former.

I left one FIXME in the patch because I'm unclear on how to properly fix
the modules problem.

PR c++/107687

gcc/c-family/ChangeLog:

* c-cppbuiltin.cc (c_cpp_builtins): Update __cpp_consteval.

gcc/cp/ChangeLog:

* call.cc (immediate_invocation_p): No longer static.
(immediate_escalating_function_p): New.
(maybe_promote_function_to_consteval): New.
(build_over_call): Set ADDR_EXPR_DENOTES_CALL_P.  Maybe promote
current_function_decl to consteval.
* constexpr.cc (instantiate_cx_fn_r): No longer static.
* cp-gimplify.cc (struct find_escalating_expr_t): New.
(find_escalating_expr_r): New.
(maybe_explain_promoted_consteval): New.
(maybe_escalate_decl_and_cfun): New.
(cp_fold_r) : Handle promoting functions to consteval.
: New case, handle promoting functions to consteval.
: Handle promoting functions to consteval.
* cp-tree.h (ADDR_EXPR_DENOTES_CALL_P): Define.
(immediate_invocation_p): Declare.
(immediate_escalating_function_p): Declare.
(maybe_promote_function_to_consteval): Declare.
(instantiate_constexpr_fns): Declare.
* typeck.cc (cp_build_addr_expr_1): SET_EXPR_LOCATION on constexpr
functions as well.

libstdc++-v3/ChangeLog:

* testsuite/20_util/integer_comparisons/greater_equal_neg.cc: Adjust
expected diagnostic.
* testsuite/20_util/integer_comparisons/greater_neg.cc: Likewise.
* testsuite/20_util/integer_comparisons/less_equal_neg.cc: Likewise.
* testsuite/20_util/optional/monadic/or_else_neg.cc: Likewise.
* testsuite/23_containers/array/creation/3_neg.cc: Likewise.
* testsuite/23_containers/span/first_neg.cc: Likewise.
* testsuite/23_containers/span/last_neg.cc: Likewise.
* testsuite/23_containers/span/subspan_neg.cc: Likewise.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/constexpr-inst1.C: Add dg-error.
* g++.dg/cpp23/consteval-if10.C: Remove dg-error.
* g++.dg/cpp23/consteval-if2.C: Likewise.
* g++.dg/cpp23/feat-cxx2b.C: Adjust expected value of __cpp_consteval.
* g++.dg/cpp26/feat-cxx26.C: Likewise.
* g++.dg/cpp2a/feat-cxx2a.C: Likewise.
* g++.dg/cpp2a/consteval-prop1.C: New test.
* g++.dg/cpp2a/consteval-prop10.C: New test.
* g++.dg/cpp2a/consteval-prop11.C: New test.
* g++.dg/cpp2a/consteval-prop12.C: New test.
* g++.dg/cpp2a/consteval-prop13.C: New test.
* g++.dg/cpp2a/consteval-prop14.C: New test.