diff mbox series

c++: Don't reject pointer to virtual method during constant evaluation [PR117615]

Message ID 010201938d8e6923-df228ae4-b5f9-4568-8ef0-5dcb665cb17d-000000@eu-west-1.amazonses.com
State New
Headers show
Series c++: Don't reject pointer to virtual method during constant evaluation [PR117615] | expand

Commit Message

Simon Martin Dec. 3, 2024, 5:25 p.m. UTC 
We currently reject the following valid code:

=== cut here ===
struct Base {
    virtual void doit (int v) const {}
};
struct Derived : Base {
    void doit (int v) const {}
};
using fn_t = void (Base::*)(int) const;
struct Helper {
    fn_t mFn;
    constexpr Helper (auto && fn) : mFn(static_cast<fn_t>(fn)) {}
};
void foo () {
    constexpr Helper h (&Derived::doit);
}
=== cut here ===

The problem is that since r6-4014-gdcdbc004d531b4, &Derived::doit is
represented with an expression with type pointer to method and using an
INTEGER_CST (here 1), and that cxx_eval_constant_expression rejects any
such expression with a non-null INTEGER_CST.

This patch uses the same strategy as r12-4491-gf45610a45236e9 (fix for
PR c++/102786), and simply lets such expressions go through.

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/117615

gcc/cp/ChangeLog:

	* constexpr.cc (cxx_eval_constant_expression): Don't reject
	INTEGER_CSTs with type POINTER_TYPE to METHOD_TYPE.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/constexpr-virtual22.C: New test.

---
 gcc/cp/constexpr.cc                           | 27 ++++++++++++-------
 .../g++.dg/cpp2a/constexpr-virtual22.C        | 22 +++++++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C

Comments

Jason Merrill Dec. 3, 2024, 9:37 p.m. UTC | #1 
On 12/3/24 12:25 PM, Simon Martin wrote:
> We currently reject the following valid code:
> 
> === cut here ===
> struct Base {
>      virtual void doit (int v) const {}
> };
> struct Derived : Base {
>      void doit (int v) const {}
> };
> using fn_t = void (Base::*)(int) const;
> struct Helper {
>      fn_t mFn;
>      constexpr Helper (auto && fn) : mFn(static_cast<fn_t>(fn)) {}
> };
> void foo () {
>      constexpr Helper h (&Derived::doit);
> }
> === cut here ===
> 
> The problem is that since r6-4014-gdcdbc004d531b4, &Derived::doit is
> represented with an expression with type pointer to method and using an
> INTEGER_CST (here 1), and that cxx_eval_constant_expression rejects any
> such expression with a non-null INTEGER_CST.
> 
> This patch uses the same strategy as r12-4491-gf45610a45236e9 (fix for
> PR c++/102786), and simply lets such expressions go through.
> 
> Successfully tested on x86_64-pc-linux-gnu.
> 
> 	PR c++/117615
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (cxx_eval_constant_expression): Don't reject
> 	INTEGER_CSTs with type POINTER_TYPE to METHOD_TYPE.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/constexpr-virtual22.C: New test.
> 
> ---
>   gcc/cp/constexpr.cc                           | 27 ++++++++++++-------
>   .../g++.dg/cpp2a/constexpr-virtual22.C        | 22 +++++++++++++++
>   2 files changed, 40 insertions(+), 9 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 5a87fa485c6..d9636bad683 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -8277,15 +8277,24 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>   	      }
>   	    else
>   	      {
> -		/* This detects for example:
> -		     reinterpret_cast<void*>(sizeof 0)
> -		*/
> -		if (!ctx->quiet)
> -		  error_at (loc, "%<reinterpret_cast<%T>(%E)%> is not "
> -			    "a constant expression",
> -			    type, op);
> -		*non_constant_p = true;
> -		return t;
> +		if (TYPE_PTR_P (type)
> +		    && TREE_CODE (TREE_TYPE (type)) == METHOD_TYPE)
> +		  /* INTEGER_CST with pointer-to-method type is only used
> +		     for a virtual method in a pointer to member function.
> +		     Don't reject those.  */
> +		  ;

This could be "else if" added before the existing else so it doesn't 
need to be reindented.  OK with that change.

> +		else
> +		  {
> +		    /* This detects for example:
> +			 reinterpret_cast<void*>(sizeof 0)
> +		    */
> +		    if (!ctx->quiet)
> +		      error_at (loc, "%<reinterpret_cast<%T>(%E)%> is not "
> +				"a constant expression",
> +				type, op);
> +		    *non_constant_p = true;
> +		    return t;
> +		  }
>   	      }
>   	  }
>   
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
> new file mode 100644
> index 00000000000..89330bf8620
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
> @@ -0,0 +1,22 @@
> +// PR c++/117615
> +// { dg-do "compile" { target c++20 } }
> +
> +struct Base {
> +    virtual void doit (int v) const {}
> +};
> +
> +struct Derived : Base {
> +    void doit (int v) const {}
> +};
> +
> +using fn_t = void (Base::*)(int) const;
> +
> +struct Helper {
> +    fn_t mFn;
> +    constexpr Helper (auto && fn) : mFn(static_cast<fn_t>(fn)) {}
> +};
> +
> +void foo () {
> +    constexpr Helper h (&Derived::doit);
> +    constexpr Helper h2 (&Base::doit);
> +}
Simon Martin Dec. 4, 2024, 10:52 a.m. UTC | #2 
Hi Jason,

On 3 Dec 2024, at 22:37, Jason Merrill wrote:

> On 12/3/24 12:25 PM, Simon Martin wrote:
>> We currently reject the following valid code:
>>
>> === cut here ===
>> struct Base {
>>      virtual void doit (int v) const {}
>> };
>> struct Derived : Base {
>>      void doit (int v) const {}
>> };
>> using fn_t = void (Base::*)(int) const;
>> struct Helper {
>>      fn_t mFn;
>>      constexpr Helper (auto && fn) : mFn(static_cast<fn_t>(fn)) {}
>> };
>> void foo () {
>>      constexpr Helper h (&Derived::doit);
>> }
>> === cut here ===
>>
>> The problem is that since r6-4014-gdcdbc004d531b4, &Derived::doit is
>> represented with an expression with type pointer to method and using 
>> an
>> INTEGER_CST (here 1), and that cxx_eval_constant_expression rejects 
>> any
>> such expression with a non-null INTEGER_CST.
>>
>> This patch uses the same strategy as r12-4491-gf45610a45236e9 (fix 
>> for
>> PR c++/102786), and simply lets such expressions go through.
>>
>> Successfully tested on x86_64-pc-linux-gnu.
>>
>> 	PR c++/117615
>>
>> gcc/cp/ChangeLog:
>>
>> 	* constexpr.cc (cxx_eval_constant_expression): Don't reject
>> 	INTEGER_CSTs with type POINTER_TYPE to METHOD_TYPE.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/cpp2a/constexpr-virtual22.C: New test.
>>
>> ---
>>   gcc/cp/constexpr.cc                           | 27 
>> ++++++++++++-------
>>   .../g++.dg/cpp2a/constexpr-virtual22.C        | 22 +++++++++++++++
>>   2 files changed, 40 insertions(+), 9 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
>>
>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>> index 5a87fa485c6..d9636bad683 100644
>> --- a/gcc/cp/constexpr.cc
>> +++ b/gcc/cp/constexpr.cc
>> @@ -8277,15 +8277,24 @@ cxx_eval_constant_expression (const 
>> constexpr_ctx *ctx, tree t,
>>   	      }
>>   	    else
>>   	      {
>> -		/* This detects for example:
>> -		     reinterpret_cast<void*>(sizeof 0)
>> -		*/
>> -		if (!ctx->quiet)
>> -		  error_at (loc, "%<reinterpret_cast<%T>(%E)%> is not "
>> -			    "a constant expression",
>> -			    type, op);
>> -		*non_constant_p = true;
>> -		return t;
>> +		if (TYPE_PTR_P (type)
>> +		    && TREE_CODE (TREE_TYPE (type)) == METHOD_TYPE)
>> +		  /* INTEGER_CST with pointer-to-method type is only used
>> +		     for a virtual method in a pointer to member function.
>> +		     Don't reject those.  */
>> +		  ;
>
> This could be "else if" added before the existing else so it doesn't 
> need to be reindented.  OK with that change.
Thanks, merged as r15-5920-g72a2380a306a1c with the suggested change.

Since this is a reject-valid, also OK for active branches?

Simon
Jason Merrill Dec. 4, 2024, 7:11 p.m. UTC | #3 
On 12/4/24 5:52 AM, Simon Martin wrote:
> Hi Jason,
> 
> On 3 Dec 2024, at 22:37, Jason Merrill wrote:
> 
>> On 12/3/24 12:25 PM, Simon Martin wrote:
>>> We currently reject the following valid code:
>>>
>>> === cut here ===
>>> struct Base {
>>>       virtual void doit (int v) const {}
>>> };
>>> struct Derived : Base {
>>>       void doit (int v) const {}
>>> };
>>> using fn_t = void (Base::*)(int) const;
>>> struct Helper {
>>>       fn_t mFn;
>>>       constexpr Helper (auto && fn) : mFn(static_cast<fn_t>(fn)) {}
>>> };
>>> void foo () {
>>>       constexpr Helper h (&Derived::doit);
>>> }
>>> === cut here ===
>>>
>>> The problem is that since r6-4014-gdcdbc004d531b4, &Derived::doit is
>>> represented with an expression with type pointer to method and using
>>> an
>>> INTEGER_CST (here 1), and that cxx_eval_constant_expression rejects
>>> any
>>> such expression with a non-null INTEGER_CST.
>>>
>>> This patch uses the same strategy as r12-4491-gf45610a45236e9 (fix
>>> for
>>> PR c++/102786), and simply lets such expressions go through.
>>>
>>> Successfully tested on x86_64-pc-linux-gnu.
>>>
>>> 	PR c++/117615
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* constexpr.cc (cxx_eval_constant_expression): Don't reject
>>> 	INTEGER_CSTs with type POINTER_TYPE to METHOD_TYPE.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/constexpr-virtual22.C: New test.
>>>
>>> ---
>>>    gcc/cp/constexpr.cc                           | 27
>>> ++++++++++++-------
>>>    .../g++.dg/cpp2a/constexpr-virtual22.C        | 22 +++++++++++++++
>>>    2 files changed, 40 insertions(+), 9 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
>>>
>>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>>> index 5a87fa485c6..d9636bad683 100644
>>> --- a/gcc/cp/constexpr.cc
>>> +++ b/gcc/cp/constexpr.cc
>>> @@ -8277,15 +8277,24 @@ cxx_eval_constant_expression (const
>>> constexpr_ctx *ctx, tree t,
>>>    	      }
>>>    	    else
>>>    	      {
>>> -		/* This detects for example:
>>> -		     reinterpret_cast<void*>(sizeof 0)
>>> -		*/
>>> -		if (!ctx->quiet)
>>> -		  error_at (loc, "%<reinterpret_cast<%T>(%E)%> is not "
>>> -			    "a constant expression",
>>> -			    type, op);
>>> -		*non_constant_p = true;
>>> -		return t;
>>> +		if (TYPE_PTR_P (type)
>>> +		    && TREE_CODE (TREE_TYPE (type)) == METHOD_TYPE)
>>> +		  /* INTEGER_CST with pointer-to-method type is only used
>>> +		     for a virtual method in a pointer to member function.
>>> +		     Don't reject those.  */
>>> +		  ;
>>
>> This could be "else if" added before the existing else so it doesn't
>> need to be reindented.  OK with that change.
> Thanks, merged as r15-5920-g72a2380a306a1c with the suggested change.
> 
> Since this is a reject-valid, also OK for active branches?

Yes.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 5a87fa485c6..d9636bad683 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8277,15 +8277,24 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	      }
 	    else
 	      {
-		/* This detects for example:
-		     reinterpret_cast<void*>(sizeof 0)
-		*/
-		if (!ctx->quiet)
-		  error_at (loc, "%<reinterpret_cast<%T>(%E)%> is not "
-			    "a constant expression",
-			    type, op);
-		*non_constant_p = true;
-		return t;
+		if (TYPE_PTR_P (type)
+		    && TREE_CODE (TREE_TYPE (type)) == METHOD_TYPE)
+		  /* INTEGER_CST with pointer-to-method type is only used
+		     for a virtual method in a pointer to member function.
+		     Don't reject those.  */
+		  ;
+		else
+		  {
+		    /* This detects for example:
+			 reinterpret_cast<void*>(sizeof 0)
+		    */
+		    if (!ctx->quiet)
+		      error_at (loc, "%<reinterpret_cast<%T>(%E)%> is not "
+				"a constant expression",
+				type, op);
+		    *non_constant_p = true;
+		    return t;
+		  }
 	      }
 	  }
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
new file mode 100644
index 00000000000..89330bf8620
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual22.C
@@ -0,0 +1,22 @@ 
+// PR c++/117615
+// { dg-do "compile" { target c++20 } }
+
+struct Base {
+    virtual void doit (int v) const {}
+};
+
+struct Derived : Base {
+    void doit (int v) const {}
+};
+
+using fn_t = void (Base::*)(int) const;
+
+struct Helper {
+    fn_t mFn;
+    constexpr Helper (auto && fn) : mFn(static_cast<fn_t>(fn)) {}
+};
+
+void foo () {
+    constexpr Helper h (&Derived::doit);
+    constexpr Helper h2 (&Base::doit);
+}