Re: [PATCH, v2] c++: Diagnose taking address of an immediate member function [PR102753]

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Oct 26, 2021 at 04:58:11PM -0400, Jason Merrill wrote:
> > I'm afraid I don't have a good idea where to move that diagnostic to though,
> > it would need to be done somewhere where we are certain we aren't in a
> > subexpression of immediate invocation.  Given statement expressions, even
> > diagnostics after parsing whole statements might not be good enough, e.g.
> > void
> > qux ()
> > {
> >    static_assert (bar (({ constexpr auto a = 1; foo; })) == 42);
> > }
> 
> I suppose (a wrapper for) fold_build_cleanup_point_expr would be a possible
> place to check, since that's called for full-expressions.

I've played a little bit with this (tried to do it at cp_fold time), but
there are problems with that.
cp_fold of course isn't a good spot for this because it can be called from
fold_for_warn and at that point we don't know if we are inside of immediate
invocation's argument or not, or it can be called even inside of consteval
fn bodies etc.  So, let's suppose we do a separate cp_walk_tree just for
this if cxx_dialect >= cxx20 e.g. from cp_fold_function and
cp_fully_fold_init or some other useful spot, like in the patch below
we avoid walking into THEN_CLAUSE of IF_STMT_CONSTEVAL_P IF_STMTs.
And if this would be done before cp_fold_function's cp_fold_r walk,
we'd also need calls to source_location_current_p as an exception.
The major problem is the location used for the error_at,
e.g. the ADDR_EXPRs pretty much never EXPR_HAS_LOCATION and PTRMEM_CST
doesn't even have location, so while we would report diagnostics, it would
be always
cc1plus: error: taking address of an immediate function ‘consteval int S::foo() const’
etc.
I guess one option is to report it even later, during gimplification where
gimplify_expr etc. track input_location, but what to do with static
initializers?
Another option would be to have a walk_tree_1 variant that would be updating
input_location similarly to how gimplify_expr does that, i.e.
  saved_location = input_location;
  if (save_expr != error_mark_node
      && EXPR_HAS_LOCATION (*expr_p))
    input_location = EXPR_LOCATION (*expr_p);
...
  input_location = saved_location;
but probably using RAII because walk_tree_1 has a lot of returns in it.
And turn walk_tree_1 into a template instantiated twice, once as walk_tree_1
without the input_location handling in it and once with it under some
different name?
Or do we have some other expression walker that does update input_location
as it goes?

--- gcc/cp/typeck.c.jj	2021-10-27 09:03:07.555043491 +0200
+++ gcc/cp/typeck.c	2021-10-29 15:59:57.871449304 +0200
@@ -6773,16 +6773,6 @@ cp_build_addr_expr_1 (tree arg, bool str
 	    return error_mark_node;
 	  }
 
-	if (TREE_CODE (t) == FUNCTION_DECL
-	    && DECL_IMMEDIATE_FUNCTION_P (t)
-	    && !in_immediate_context ())
-	  {
-	    if (complain & tf_error)
-	      error_at (loc, "taking address of an immediate function %qD",
-			t);
-	    return error_mark_node;
-	  }
-
 	type = build_ptrmem_type (context_for_name_lookup (t),
 				  TREE_TYPE (t));
 	t = make_ptrmem_cst (type, t);
@@ -6809,15 +6799,6 @@ cp_build_addr_expr_1 (tree arg, bool str
     {
       tree stripped_arg = tree_strip_any_location_wrapper (arg);
       if (TREE_CODE (stripped_arg) == FUNCTION_DECL
-	  && DECL_IMMEDIATE_FUNCTION_P (stripped_arg)
-	  && !in_immediate_context ())
-	{
-	  if (complain & tf_error)
-	    error_at (loc, "taking address of an immediate function %qD",
-		      stripped_arg);
-	  return error_mark_node;
-	}
-      if (TREE_CODE (stripped_arg) == FUNCTION_DECL
 	  && !mark_used (stripped_arg, complain) && !(complain & tf_error))
 	return error_mark_node;
       val = build_address (arg);
--- gcc/cp/cp-gimplify.c.jj	2021-09-18 09:47:08.409573816 +0200
+++ gcc/cp/cp-gimplify.c	2021-10-29 16:48:42.308261319 +0200
@@ -902,6 +902,17 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
 	}
       cp_walk_tree (&OMP_FOR_PRE_BODY (stmt), cp_fold_r, data, NULL);
       *walk_subtrees = 0;
+      return NULL;
+    }
+
+  if (code == IF_STMT && IF_STMT_CONSTEVAL_P (stmt))
+    {
+      /* Don't walk THEN_CLAUSE (stmt) for consteval if.  IF_COND is always
+	 boolean_false_node.  */
+      cp_walk_tree (&ELSE_CLAUSE (stmt), cp_fold_r, data, NULL);
+      cp_walk_tree (&IF_SCOPE (stmt), cp_fold_r, data, NULL);
+      *walk_subtrees = 0;
+      return NULL;
     }
 
   return NULL;
@@ -1418,9 +1429,9 @@ cp_genericize_r (tree *stmt_p, int *walk
 	}
 
       if (tree fndecl = cp_get_callee_fndecl_nofold (stmt))
-	if (DECL_IMMEDIATE_FUNCTION_P (fndecl))
+	if (DECL_IMMEDIATE_FUNCTION_P (fndecl)
+	    && source_location_current_p (fndecl))
 	  {
-	    gcc_assert (source_location_current_p (fndecl));
 	    *stmt_p = cxx_constant_value (stmt);
 	    break;
 	  }
@@ -2319,8 +2330,28 @@ cp_fold (tree x)
 	}
       goto unary;
 
+    case PTRMEM_CST:
+      if (TREE_CODE (PTRMEM_CST_MEMBER (x)) == FUNCTION_DECL
+	  && DECL_IMMEDIATE_FUNCTION_P (PTRMEM_CST_MEMBER (x)))
+	{
+	  error_at (input_location,
+		    "taking address of an immediate function %qD",
+		    PTRMEM_CST_MEMBER (x));
+	  x = error_mark_node;
+	  break;
+	}
+      break;
+
     case ADDR_EXPR:
       loc = EXPR_LOCATION (x);
+      if (TREE_CODE (TREE_OPERAND (x, 0)) == FUNCTION_DECL
+	  && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (x, 0)))
+	{
+	  error_at (loc, "taking address of an immediate function %qD",
+		    TREE_OPERAND (x, 0));
+	  x = error_mark_node;
+	  break;
+	}
       op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), false);
 
       /* Cope with user tricks that amount to offsetof.  */


	Jakub