On Tue, Nov 23, 2021 at 03:45:20PM -0500, Jason Merrill wrote:
> > 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.
> 
> How about checking in cp_fold_r instead of cp_fold?

That seems to work.

> > 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've checked in a patch to give PTRMEM_CST a location wrapper; perhaps that
> will be helpful.

Unfortunately, the location wrappers are optimized away before we get a
chance to use them in cp_fold_r.
So, on the following patch, we get the location right on PTRMEM_CSTs not
used inside of initializers, but for PTRMEM_CSTs in initializers we report
them at UNKNOWN_LOCATION.
As PTRMEM_CST is a C++ FE tree, I wonder if we couldn't instead of your
patch or in addition to it do:
 struct GTY(()) ptrmem_cst {
   struct tree_common common;
   tree member;
+  location_t locus;
 };
#define PTRMEM_CST_LOCATION(NODE) \
  (((ptrmem_cst_t)PTRMEM_CST_CHECK (NODE))->locus)
and in make_ptrmem_cst set PTRMEM_CST_LOCATION to input_location.

> > 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.
> 
> iloc_sentinel seems relevant.
> 
> > 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?
> 
> Maybe just add the handling to walk_tree_1?

The vast majority of walk_tree_1 users don't want it to change
input_location as it goes.  But if we have PTRMEM_CST_LOCATION, I'd think we
don't really need it.

--- gcc/cp/typeck.c.jj  2021-11-24 09:54:11.521738651 +0100
+++ gcc/cp/typeck.c     2021-11-24 16:28:52.281468302 +0100
@@ -6780,19 +6780,15 @@ 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);
+
+       /* For addresses of immediate member functions ensure we have
+          EXPR_LOCATION set for possible later diagnostics.  */
+       if (TREE_CODE (TREE_OPERAND (arg, 1)) == FUNCTION_DECL
+           && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (arg, 1)))
+         t = maybe_wrap_with_location (t, input_location);
        return t;
       }
 
@@ -6816,15 +6812,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);
@@ -6865,6 +6852,13 @@ cp_build_addr_expr_1 (tree arg, bool str
                              complain);
     }
 
+  /* For addresses of immediate functions ensure we have EXPR_LOCATION
+     set for possible later diagnostics.  */
+  if (TREE_CODE (val) == ADDR_EXPR
+      && TREE_CODE (TREE_OPERAND (val, 0)) == FUNCTION_DECL
+      && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (val, 0)))
+    SET_EXPR_LOCATION (val, input_location);
+
   return val;
 }
 
--- gcc/cp/cp-gimplify.c.jj     2021-11-19 10:04:54.343314733 +0100
+++ gcc/cp/cp-gimplify.c        2021-11-24 16:20:54.930275872 +0100
@@ -900,8 +900,46 @@ struct cp_genericize_data
 static tree
 cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data)
 {
-  tree stmt;
-  enum tree_code code;
+  tree stmt = *stmt_p;
+  enum tree_code code = TREE_CODE (stmt);
+  location_t loc = UNKNOWN_LOCATION;
+
+  switch (code)
+    {
+    case NON_LVALUE_EXPR:
+      if (TREE_CODE (TREE_OPERAND (stmt, 0)) != PTRMEM_CST)
+       break;
+      loc = EXPR_LOCATION (stmt);
+      stmt = TREE_OPERAND (stmt, 0);
+      /* FALLTHRU */
+    case PTRMEM_CST:
+      if (TREE_CODE (PTRMEM_CST_MEMBER (stmt)) == FUNCTION_DECL
+         && DECL_IMMEDIATE_FUNCTION_P (PTRMEM_CST_MEMBER (stmt)))
+       {
+         if (!((hash_set<tree> *) data)->add (stmt))
+           error_at (loc,
+                     "taking address of an immediate function %qD",
+                     PTRMEM_CST_MEMBER (stmt));
+         stmt = *stmt_p = build_zero_cst (TREE_TYPE (stmt));
+         break;
+       }
+      break;
+
+    case ADDR_EXPR:
+      if (TREE_CODE (TREE_OPERAND (stmt, 0)) == FUNCTION_DECL
+         && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0)))
+       {
+         loc = EXPR_LOCATION (stmt);
+         error_at (loc, "taking address of an immediate function %qD",
+                   TREE_OPERAND (stmt, 0));
+         stmt = *stmt_p = build_zero_cst (TREE_TYPE (stmt));
+         break;
+       }
+      break;
+
+    default:
+      break;
+    }
 
   *stmt_p = stmt = cp_fold (*stmt_p);
 
@@ -917,12 +955,16 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
     }
 
   code = TREE_CODE (stmt);
-  if (code == OMP_FOR || code == OMP_SIMD || code == OMP_DISTRIBUTE
-      || code == OMP_LOOP || code == OMP_TASKLOOP || code == OACC_LOOP)
+  switch (code)
     {
       tree x;
       int i, n;
-
+    case OMP_FOR:
+    case OMP_SIMD:
+    case OMP_DISTRIBUTE:
+    case OMP_LOOP:
+    case OMP_TASKLOOP:
+    case OACC_LOOP:
       cp_walk_tree (&OMP_FOR_BODY (stmt), cp_fold_r, data, NULL);
       cp_walk_tree (&OMP_FOR_CLAUSES (stmt), cp_fold_r, data, NULL);
       cp_walk_tree (&OMP_FOR_INIT (stmt), cp_fold_r, data, NULL);
@@ -961,6 +1003,22 @@ 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;
+
+    case IF_STMT:
+      if (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;
+       }
+      break;
+
+    default:
+      break;
     }
 
   return NULL;
@@ -1477,9 +1535,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;
          }
--- gcc/cp/pt.c.jj      2021-11-24 15:05:23.336928234 +0100
+++ gcc/cp/pt.c 2021-11-24 15:34:29.018014159 +0100
@@ -17012,6 +17012,12 @@ tsubst_copy (tree t, tree args, tsubst_f
        r = build1 (code, type, op0);
        if (code == ALIGNOF_EXPR)
          ALIGNOF_EXPR_STD_P (r) = ALIGNOF_EXPR_STD_P (t);
+       /* For addresses of immediate functions ensure we have EXPR_LOCATION
+          set for possible later diagnostics.  */
+       if (code == ADDR_EXPR
+           && TREE_CODE (op0) == FUNCTION_DECL
+           && DECL_IMMEDIATE_FUNCTION_P (op0))
+         SET_EXPR_LOCATION (r, input_location);
        return r;
       }
 
--- gcc/testsuite/g++.dg/cpp2a/consteval23.C.jj 2021-10-27 09:03:07.576043195 
+0200
+++ gcc/testsuite/g++.dg/cpp2a/consteval23.C    2021-11-24 16:41:39.431532798 
+0100
@@ -2,6 +2,7 @@
 // { dg-do compile { target c++20 } }
 
 consteval int foo () { return 42; }
+constexpr auto baz (int (*fn) ()) { return fn; }
 
 consteval int
 bar (int (*fn) () = foo)
@@ -11,3 +12,6 @@ bar (int (*fn) () = foo)
 
 static_assert (bar () == 42);
 static_assert (bar (foo) == 42);
+static_assert (bar (&foo) == 42);
+static_assert (bar (baz (foo)) == 42);
+static_assert (bar (baz (&foo)) == 42);
--- gcc/testsuite/g++.dg/cpp2a/consteval20.C.jj 2021-10-27 09:03:07.576043195 
+0200
+++ gcc/testsuite/g++.dg/cpp2a/consteval20.C    2021-11-24 16:49:40.481679281 
+0100
@@ -10,10 +10,14 @@ constexpr S s;
 int
 bar ()
 {
+  auto c = &S::foo;                    // { dg-error "taking address of an 
immediate function" }
+  constexpr auto d = &S::foo;          // { dg-error "taking address of an 
immediate function" }
+  static auto e = &S::foo;             // { dg-error "taking address of an 
immediate function" }
   return (s.*&S::foo) ();              // { dg-error "taking address of an 
immediate function" }
 }
 
 constexpr auto a = &S::foo;            // { dg-error "taking address of an 
immediate function" }
+auto b = &S::foo;                      // { dg-error "taking address of an 
immediate function" }
 
 consteval int
 baz ()


        Jakub

Reply via email to