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

[Jason Merrill <jason@redhat.com>]

On 11/23/23 11:46, Marek Polacek wrote:
> v5 greatly simplifies the code.

Indeed, it's much cleaner now.

> I still need a new ff_ flag to signal that we can return immediately
> after seeing an i-e expr.

That's still not clear to me:

> +      /* In turn, maybe promote the function we find ourselves in...  */
> +      if ((data->flags & ff_find_escalating_expr)
> +         && DECL_IMMEDIATE_FUNCTION_P (decl)
> +         /* ...but not if the call to DECL was constant; that is the
> +            "an immediate invocation that is not a constant expression"
> +            case.  */
> +         && (e = cxx_constant_value (stmt, tf_none), e == error_mark_node))
> +       {
> +         /* Since we had to set DECL_ESCALATION_CHECKED_P before the walk,
> +            we call promote_function_to_consteval directly which doesn't
> +            check unchecked_immediate_escalating_function_p.  */
> +         if (current_function_decl)
> +           promote_function_to_consteval (current_function_decl);
> +         *walk_subtrees = 0;
> +         return stmt;
> +       }

This is the one use of ff_find_escalating_expr, and it seems redundant 
with the code immediately below, where we use complain (derived from 
ff_mce_false) to decide whether to return immediately.  Can we remove 
this hunk and the flag, and merge find_escalating_expr with 
cp_fold_immediate?

I think you want to walk the function body for three-ish reasons:
1) at EOF, to check for escalation
2) at EOF, to check for errors
3) at error time, to explain escalation

It's not clear to me that we need a flag to distinguish between them. 
When we encounter an immediate-escalating expression E:

A) if we're in an immediate-escalating function, escalate and return E 
(#1, #3).
B) otherwise, if we're diagnosing, error and continue (#2).
C) otherwise, return E (individual expression mce_unknown walk from 
constexpr.cc).

> @@ -1178,11 +1388,19 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_
> )
>           *walk_subtrees = 0;
>           /* Don't return yet, still need the cp_fold below.  */
>         }
> -      cp_fold_immediate_r (stmt_p, walk_subtrees, data);
> +      else
> +       cp_fold_immediate_r (stmt_p, walk_subtrees, data);
>      }
>  
>    *stmt_p = stmt = cp_fold (*stmt_p, data->flags);
>  
> +  /* For certain trees, like +foo(), the cp_fold below will remove the +,

s/below/above/?

> +/* We've stashed immediate-escalating functions.  Now see if they indeed
> +   ought to be promoted to consteval.  */
> +
> +void
> +process_pending_immediate_escalating_fns ()
> +{
> +  /* This will be null for -fno-immediate-escalation.  */
> +  if (!deferred_escalating_exprs)
> +    return;
> +
> +  for (auto e : *deferred_escalating_exprs)
> +    if (TREE_CODE (e) == FUNCTION_DECL && !DECL_ESCALATION_CHECKED_P (e))
> +      cp_fold_immediate (&DECL_SAVED_TREE (e), mce_false, e);
> +}
> +
> +/* We've escalated every function that could have been promoted to
> +   consteval.  Check that we are not taking the address of a consteval
> +   function.  */
> +
> +void
> +check_immediate_escalating_refs ()
> +{
> +  /* This will be null for -fno-immediate-escalation.  */
> +  if (!deferred_escalating_exprs)
> +    return;
> +
> +  for (auto ref : *deferred_escalating_exprs)
> +    {
> +      if (TREE_CODE (ref) == FUNCTION_DECL)
> +       continue;
> +      tree decl = (TREE_CODE (ref) == PTRMEM_CST
> +                  ? PTRMEM_CST_MEMBER (ref)
> +                  : TREE_OPERAND (ref, 0));
> +      if (DECL_IMMEDIATE_FUNCTION_P (decl))
> +       taking_address_of_imm_fn_error (ref, decl);
> +    }
> +
> +  deferred_escalating_exprs = nullptr;
>  }

Could these be merged, so you do a single loop of cp_fold_immediate over 
function bodies or non-function expressions?  I'd expect that to work.

Jason