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