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

Jason Merrill jason@redhat.com
Wed Oct 20 23:16:44 GMT 2021


On 10/19/21 08:00, Jakub Jelinek wrote:
> On Mon, Oct 18, 2021 at 12:42:00PM -0400, Jason Merrill wrote:
>>> --- gcc/cp/typeck.c.jj	2021-10-05 09:53:55.382734051 +0200
>>> +++ gcc/cp/typeck.c	2021-10-15 19:28:38.034213437 +0200
>>> @@ -6773,9 +6773,21 @@ cp_build_addr_expr_1 (tree arg, bool str
>>>    	    return error_mark_node;
>>>    	  }
>>> +	if (TREE_CODE (t) == FUNCTION_DECL
>>> +	    && DECL_IMMEDIATE_FUNCTION_P (t)
>>> +	    && cp_unevaluated_operand == 0
>>> +	    && (current_function_decl == NULL_TREE
>>> +		|| !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)))
>>
>> This doesn't cover some of the other cases of immediate context; we should
>> probably factor most of immediate_invocation_p out into a function called
>> something like in_immediate_context and use it here, and in several other
>> places as well.
> 
> You're right, I've done that for the two spots in cp_build_addr_expr_1
> and added testsuite coverage for where it changed behavior.
> While doing that I've discovered further issues.
> 
> One is that we weren't diagnosing PMFs referring to immediate methods
> returned from immediate functions (either directly or embedded in
> aggregates).  I'm not sure if it can only appear as PTRMEM_CST which
> I've handled (cp_walk_subtree only walks the type and not the
> PTRMEM_CST_MEMBER) or something else.
> 
> Another issue is that while default arg in immediate function
> containing &immediate_fn works properly, if it is immediate_fn
> instead, we were incorrectly rejecting it.
> I've handled this in build_over_call, though with this usage
> in_consteval_if_p is slightly misnamed, it stands for in consteval
> if or some other reason why we are currently in immediate function context.
> Though, that flag alone can't be all the reasons for being in immediate
> function contexts, as I've tried the other reasons can't be handled in such
> a bool and need to be tested too.
> 
> And another thing isn't in a patch, but I'm wondering whether we don't
> handle it incorrectly.  constexpr.c has:
>    /* Check that immediate invocation does not return an expression referencing
>       any immediate function decls.  They need to be allowed while parsing
>       immediate functions, but can't leak outside of them.  */
>    if (is_consteval
>        && t != r
>        && (current_function_decl == NULL_TREE
> 	  || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)))
> as condition for the discovery of embedded immediate FUNCTION_DECLs
> (or now PTRMEM_CSTs).  If I remove the && (current... ..._decl))
> then g++.dg/cpp2a/consteval7.C's
> struct S { int b; int (*c) (); };
> consteval S baz () { return { 5, foo }; }
> consteval int qux () { S s = baz (); return s.b + s.c (); }
> consteval int quux () { constexpr S s = baz (); return s.b + s.c (); }
> quux line fails, but based on
> http://eel.is/c++draft/expr.const#11
> I wonder if it shouldn't fail (clang++ -std=c++20 rejects it),
> and be only accepted without the constexpr keyword before S s.
> Also wonder about e.g.
> consteval int foo () { return 42; }
> 
> consteval int
> bar ()
> {
>    auto fn1 = foo;  // This must be ok
>    constexpr auto fn2 = foo; // Isn't this an error?
>    return fn1 () + fn2 ();
> }
> 
> constexpr int
> baz ()
> {
>    if consteval {
>      auto fn1 = foo; // This must be ok
>      constexpr auto fn2 = foo; // Isn't this an error?
>      return fn1 () + fn2 ();
>    }
>    return 0;
> }
> 
> auto a = bar ();
> 
> static_assert (bar () == 84);
> static_assert (baz () == 84);
> (again, clang++ -std=c++20 rejects the fn2 = foo; case,
> but doesn't implement consteval if, so can't test the other one).
> For taking address of an immediate function or method if it is taken
> outside of immediate function context we already have diagnostics
> about it, but shouldn't the immediate FUNCTION_DECL discovery in
> cxx_eval_outermost_constant_expression be instead guarded with something
> like
>    if (is_consteval || in_immediate_context ())
> and be done regardless of whether t != r?
> 
> 2021-10-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102753
> 	* cp-tree.h (in_immediate_context): Declare.
> 	* call.c (in_immediate_context): New function.
> 	(immediate_invocation_p): Use it.
> 	(build_over_call): Temporarily set in_consteval_if_p for
> 	convert_default_arg calls of immediate invocations.
> 	* typeck.c (cp_build_addr_expr_1): Diagnose taking address of
> 	an immediate method.  Use t instead of TREE_OPERAND (arg, 1).
> 	Use in_immediate_context function.
> 	* constexpr.c (find_immediate_fndecl): Handle PTRMEM_CST
> 	which refers to immediate function decl.
> 
> 	* g++.dg/cpp2a/consteval20.C: New test.
> 	* g++.dg/cpp2a/consteval21.C: New test.
> 	* g++.dg/cpp2a/consteval22.C: New test.
> 	* g++.dg/cpp2a/consteval23.C: New test.
> 	* g++.dg/cpp23/consteval-if11.C: New test.
> 
> --- gcc/cp/cp-tree.h.jj	2021-10-15 11:58:44.968133548 +0200
> +++ gcc/cp/cp-tree.h	2021-10-19 10:40:58.375799274 +0200
> @@ -6547,6 +6547,7 @@ extern tree perform_direct_initializatio
>                                                          tsubst_flags_t);
>   extern vec<tree,va_gc> *resolve_args (vec<tree,va_gc>*, tsubst_flags_t);
>   extern tree in_charge_arg_for_name		(tree);
> +extern bool in_immediate_context		();
>   extern tree build_cxx_call			(tree, int, tree *,
>   						 tsubst_flags_t,
>   						 tree = NULL_TREE);
> --- gcc/cp/call.c.jj	2021-10-15 11:58:44.947133850 +0200
> +++ gcc/cp/call.c	2021-10-19 13:04:42.333421774 +0200
> @@ -9025,6 +9025,19 @@ build_trivial_dtor_call (tree instance,
>   		 instance, clobber);
>   }
>   
> +/* Return true if in an immediate function context.  */

or an unevaluated operand, or a subexpression of an immediate invocation.

Hmm...that suggests that in consteval23.C, bar(foo) should also be OK, 
because 'foo' is a subexpression of an immediate invocation.  We can 
handle that by...

> +
> +bool
> +in_immediate_context ()
> +{
> +  return (cp_unevaluated_operand != 0
> +	  || (current_function_decl != NULL_TREE
> +	      && DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
> +	  || (current_binding_level->kind == sk_function_parms
> +	      && current_binding_level->immediate_fn_ctx_p)
> +	  || in_consteval_if_p);
> +}
> +
>   /* Return true if a call to FN with number of arguments NARGS
>      is an immediate invocation.  */
>   
> @@ -9451,6 +9459,12 @@ build_over_call (struct z_candidate *can
>       }
>   
>     /* Default arguments */
> +  bool save_in_consteval_if_p = in_consteval_if_p;
> +  /* If the call is immediate function invocation, make sure
> +     taking address of immediate functions is allowed in default
> +     arguments.  */
> +  if (immediate_invocation_p (STRIP_TEMPLATE (fn), nargs))
> +    in_consteval_if_p = true;

...moving this earlier in build_over_call, e.g. shortly after 
not_really_used:

You can also use make_temp_override.

And update the comment for saved_scope::consteval_if_p to note that it 
is also set while processing an immediate invocation.

>     for (; parm && parm != void_list_node; parm = TREE_CHAIN (parm), i++)
>       {
>         if (TREE_VALUE (parm) == error_mark_node)
> @@ -9463,6 +9477,7 @@ build_over_call (struct z_candidate *can
>           return error_mark_node;
>         argarray[j++] = val;
>       }
> +  in_consteval_if_p = save_in_consteval_if_p;
>   
>     /* Ellipsis */
>     int magic = magic_varargs_p (fn);
> --- gcc/cp/typeck.c.jj	2021-10-18 11:01:08.635858336 +0200
> +++ gcc/cp/typeck.c	2021-10-19 10:42:18.454671723 +0200
> @@ -6773,9 +6773,19 @@ 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, TREE_OPERAND (arg, 1));
> +	t = make_ptrmem_cst (type, t);
>   	return t;
>         }
>   
> @@ -6800,9 +6810,7 @@ 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)
> -	  && cp_unevaluated_operand == 0
> -	  && (current_function_decl == NULL_TREE
> -	      || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)))
> +	  && !in_immediate_context ())
>   	{
>   	  if (complain & tf_error)
>   	    error_at (loc, "taking address of an immediate function %qD",
> --- gcc/cp/constexpr.c.jj	2021-10-19 09:24:41.938242276 +0200
> +++ gcc/cp/constexpr.c	2021-10-19 12:22:35.583964001 +0200
> @@ -7276,6 +7276,10 @@ find_immediate_fndecl (tree *tp, int */*
>   {
>     if (TREE_CODE (*tp) == FUNCTION_DECL && DECL_IMMEDIATE_FUNCTION_P (*tp))
>       return *tp;
> +  if (TREE_CODE (*tp) == PTRMEM_CST
> +      && TREE_CODE (PTRMEM_CST_MEMBER (*tp)) == FUNCTION_DECL
> +      && DECL_IMMEDIATE_FUNCTION_P (PTRMEM_CST_MEMBER (*tp)))
> +    return PTRMEM_CST_MEMBER (*tp);
>     return NULL_TREE;
>   }
>   
> --- gcc/testsuite/g++.dg/cpp2a/consteval20.C.jj	2021-10-19 10:29:30.471484783 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/consteval20.C	2021-10-19 10:29:30.471484783 +0200
> @@ -0,0 +1,24 @@
> +// PR c++/102753
> +// { dg-do compile { target c++20 } }
> +
> +struct S {
> +  consteval int foo () const { return 42; }
> +};
> +
> +constexpr S s;
> +
> +int
> +bar ()
> +{
> +  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" }
> +
> +consteval int
> +baz ()
> +{
> +  return (s.*&S::foo) ();
> +}
> +
> +static_assert (baz () == 42);
> --- gcc/testsuite/g++.dg/cpp2a/consteval21.C.jj	2021-10-19 11:09:18.890838778 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/consteval21.C	2021-10-19 11:57:28.309175141 +0200
> @@ -0,0 +1,35 @@
> +// PR c++/102753
> +// { dg-do compile { target c++20 } }
> +
> +struct S {
> +  constexpr S () : s (0) {}
> +  consteval int foo () { return 1; }
> +  virtual consteval int bar () { return 2; }
> +  int s;
> +};
> +
> +consteval int foo () { return 42; }
> +
> +consteval int
> +bar (int (*fn) () = &foo)
> +{
> +  return fn ();
> +}
> +
> +consteval int
> +baz (int (S::*fn) () = &S::foo)
> +{
> +  S s;
> +  return (s.*fn) ();
> +}
> +
> +consteval int
> +qux (int (S::*fn) () = &S::bar)
> +{
> +  S s;
> +  return (s.*fn) ();
> +}
> +
> +static_assert (bar () == 42);
> +static_assert (baz () == 1);
> +static_assert (qux () == 2);
> --- gcc/testsuite/g++.dg/cpp2a/consteval22.C.jj	2021-10-19 11:21:44.271346868 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/consteval22.C	2021-10-19 12:23:31.783173408 +0200
> @@ -0,0 +1,34 @@
> +// PR c++/102753
> +// { dg-do compile { target c++20 } }
> +
> +struct S {
> +  constexpr S () : s (0) {}
> +  consteval int foo () { return 1; }
> +  virtual consteval int bar () { return 2; }
> +  int s;
> +};
> +typedef int (S::*P) ();
> +
> +consteval P
> +foo ()
> +{
> +  return &S::foo;
> +}
> +
> +consteval P
> +bar ()
> +{
> +  return &S::bar;
> +}
> +
> +consteval int
> +baz ()
> +{
> +  S s;
> +  return (s.*(foo ())) () + (s.*(bar ())) ();
> +}
> +
> +static_assert (baz () == 3);
> +
> +constexpr P a = foo ();		// { dg-error "immediate evaluation returns address of immediate function" }
> +constexpr P b = bar ();		// { dg-error "immediate evaluation returns address of immediate function" }
> --- gcc/testsuite/g++.dg/cpp2a/consteval23.C.jj	2021-10-19 12:23:54.235857548 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/consteval23.C	2021-10-19 12:24:33.931299123 +0200
> @@ -0,0 +1,12 @@
> +// PR c++/102753
> +// { dg-do compile { target c++20 } }
> +
> +consteval int foo () { return 42; }
> +
> +consteval int
> +bar (int (*fn) () = foo)
> +{
> +  return fn ();
> +}
> +
> +static_assert (bar () == 42);
> --- gcc/testsuite/g++.dg/cpp23/consteval-if11.C.jj	2021-10-19 11:17:25.964982502 +0200
> +++ gcc/testsuite/g++.dg/cpp23/consteval-if11.C	2021-10-19 11:35:38.878602026 +0200
> @@ -0,0 +1,27 @@
> +// PR c++/102753
> +// { dg-do compile { target c++20 } }
> +// { dg-options "" }
> +
> +struct S {
> +  constexpr S () : s (0) {}
> +  consteval int foo () { return 1; }
> +  virtual consteval int bar () { return 2; }
> +  int s;
> +};
> +
> +consteval int foo () { return 42; }
> +
> +constexpr int
> +bar ()
> +{
> +  if consteval {	// { dg-warning "'if consteval' only available with" "" { target c++20_only } }
> +    int (*fn1) () = foo;
> +    int (S::*fn2) () = &S::foo;
> +    int (S::*fn3) () = &S::bar;
> +    S s;
> +    return fn1 () + (s.*fn2) () + (s.*fn3) ();
> +  }
> +  return 0;
> +}
> +
> +static_assert (bar () == 45);
> 
> 
> 	Jakub
> 



More information about the Gcc-patches mailing list