public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: for contracts, cdtors never return this
@ 2023-11-19  7:28 Alexandre Oliva
  2023-11-20 15:40 ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Oliva @ 2023-11-19  7:28 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell


When targetm.cxx.cdtor_return_this() holds, cdtors have a
non-VOID_TYPE_P result, but IMHO this ABI implementation detail
shouldn't leak to the abstract language conceptual framework, in which
cdtors don't have return values.  For contracts, specifically those
that establish postconditions on results, such a leakage is present,
and the present patch puts an end to it: with it, cdtors get an error
for result postconditions regardless of the ABI.  This fixes
g++.dg/contracts/contracts-ctor-dtor2.C on arm-eabi.

Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default
cpu on trunk, and with tms570 on gcc-13.  Ok to install?


for  gcc/cp/ChangeLog

	* contracts.cc (check_postcondition_result): Cope with
	cdtor_return_this.
---
 gcc/cp/contracts.cc |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
index 66d2298a9bfac..035ca4827e758 100644
--- a/gcc/cp/contracts.cc
+++ b/gcc/cp/contracts.cc
@@ -636,7 +636,11 @@ make_postcondition_variable (cp_expr id)
 bool
 check_postcondition_result (tree decl, tree type, location_t loc)
 {
-  if (VOID_TYPE_P (type))
+  /* Do not be confused by targetm.cxx.cdtor_return_this ();
+     conceptually, cdtors have no return value.  */
+  if (VOID_TYPE_P (type)
+      || DECL_CONSTRUCTOR_P (decl)
+      || DECL_DESTRUCTOR_P (decl))
     {
       error_at (loc,
 		DECL_CONSTRUCTOR_P (decl)


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: for contracts, cdtors never return this
  2023-11-19  7:28 [PATCH] c++: for contracts, cdtors never return this Alexandre Oliva
@ 2023-11-20 15:40 ` Jason Merrill
  2024-02-07 16:22   ` Torbjorn SVENSSON
  0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2023-11-20 15:40 UTC (permalink / raw)
  To: Alexandre Oliva, gcc-patches; +Cc: Nathan Sidwell

On 11/19/23 02:28, Alexandre Oliva wrote:
> 
> When targetm.cxx.cdtor_return_this() holds, cdtors have a
> non-VOID_TYPE_P result, but IMHO this ABI implementation detail
> shouldn't leak to the abstract language conceptual framework, in which
> cdtors don't have return values.  For contracts, specifically those
> that establish postconditions on results, such a leakage is present,
> and the present patch puts an end to it: with it, cdtors get an error
> for result postconditions regardless of the ABI.  This fixes
> g++.dg/contracts/contracts-ctor-dtor2.C on arm-eabi.
> 
> Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default
> cpu on trunk, and with tms570 on gcc-13.  Ok to install?

OK.

> 
> for  gcc/cp/ChangeLog
> 
> 	* contracts.cc (check_postcondition_result): Cope with
> 	cdtor_return_this.
> ---
>   gcc/cp/contracts.cc |    6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
> index 66d2298a9bfac..035ca4827e758 100644
> --- a/gcc/cp/contracts.cc
> +++ b/gcc/cp/contracts.cc
> @@ -636,7 +636,11 @@ make_postcondition_variable (cp_expr id)
>   bool
>   check_postcondition_result (tree decl, tree type, location_t loc)
>   {
> -  if (VOID_TYPE_P (type))
> +  /* Do not be confused by targetm.cxx.cdtor_return_this ();
> +     conceptually, cdtors have no return value.  */
> +  if (VOID_TYPE_P (type)
> +      || DECL_CONSTRUCTOR_P (decl)
> +      || DECL_DESTRUCTOR_P (decl))
>       {
>         error_at (loc,
>   		DECL_CONSTRUCTOR_P (decl)
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: for contracts, cdtors never return this
  2023-11-20 15:40 ` Jason Merrill
@ 2024-02-07 16:22   ` Torbjorn SVENSSON
  2024-02-07 19:46     ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Torbjorn SVENSSON @ 2024-02-07 16:22 UTC (permalink / raw)
  To: Jason Merrill, Alexandre Oliva, gcc-patches; +Cc: Nathan Sidwell, Yvan Roux

Hi,

Is it okay to backport 71804526d3a71a8c0f189a89ce3aa615784bfd8b to 
releases/gcc-13?

Without this backport, I see these failures on arm-none-eabi:

FAIL: g++.dg/contracts/contracts-ctor-dtor2.C    (test for errors, line 23)
FAIL: g++.dg/contracts/contracts-ctor-dtor2.C    (test for errors, line 27

Kind regards,
Torbjörn

On 2023-11-20 16:40, Jason Merrill wrote:
> On 11/19/23 02:28, Alexandre Oliva wrote:
>>
>> When targetm.cxx.cdtor_return_this() holds, cdtors have a
>> non-VOID_TYPE_P result, but IMHO this ABI implementation detail
>> shouldn't leak to the abstract language conceptual framework, in which
>> cdtors don't have return values.  For contracts, specifically those
>> that establish postconditions on results, such a leakage is present,
>> and the present patch puts an end to it: with it, cdtors get an error
>> for result postconditions regardless of the ABI.  This fixes
>> g++.dg/contracts/contracts-ctor-dtor2.C on arm-eabi.
>>
>> Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default
>> cpu on trunk, and with tms570 on gcc-13.  Ok to install?
> 
> OK.
> 
>>
>> for  gcc/cp/ChangeLog
>>
>>     * contracts.cc (check_postcondition_result): Cope with
>>     cdtor_return_this.
>> ---
>>   gcc/cp/contracts.cc |    6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
>> index 66d2298a9bfac..035ca4827e758 100644
>> --- a/gcc/cp/contracts.cc
>> +++ b/gcc/cp/contracts.cc
>> @@ -636,7 +636,11 @@ make_postcondition_variable (cp_expr id)
>>   bool
>>   check_postcondition_result (tree decl, tree type, location_t loc)
>>   {
>> -  if (VOID_TYPE_P (type))
>> +  /* Do not be confused by targetm.cxx.cdtor_return_this ();
>> +     conceptually, cdtors have no return value.  */
>> +  if (VOID_TYPE_P (type)
>> +      || DECL_CONSTRUCTOR_P (decl)
>> +      || DECL_DESTRUCTOR_P (decl))
>>       {
>>         error_at (loc,
>>           DECL_CONSTRUCTOR_P (decl)
>>
>>
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: for contracts, cdtors never return this
  2024-02-07 16:22   ` Torbjorn SVENSSON
@ 2024-02-07 19:46     ` Jason Merrill
  2024-02-09  8:47       ` Torbjorn SVENSSON
  0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2024-02-07 19:46 UTC (permalink / raw)
  To: Torbjorn SVENSSON, Alexandre Oliva, gcc-patches; +Cc: Yvan Roux

On 2/7/24 11:22, Torbjorn SVENSSON wrote:
> Hi,
> 
> Is it okay to backport 71804526d3a71a8c0f189a89ce3aa615784bfd8b to 
> releases/gcc-13?

OK.

> Without this backport, I see these failures on arm-none-eabi:
> 
> FAIL: g++.dg/contracts/contracts-ctor-dtor2.C    (test for errors, line 23)
> FAIL: g++.dg/contracts/contracts-ctor-dtor2.C    (test for errors, line 27
> 
> Kind regards,
> Torbjörn
> 
> On 2023-11-20 16:40, Jason Merrill wrote:
>> On 11/19/23 02:28, Alexandre Oliva wrote:
>>>
>>> When targetm.cxx.cdtor_return_this() holds, cdtors have a
>>> non-VOID_TYPE_P result, but IMHO this ABI implementation detail
>>> shouldn't leak to the abstract language conceptual framework, in which
>>> cdtors don't have return values.  For contracts, specifically those
>>> that establish postconditions on results, such a leakage is present,
>>> and the present patch puts an end to it: with it, cdtors get an error
>>> for result postconditions regardless of the ABI.  This fixes
>>> g++.dg/contracts/contracts-ctor-dtor2.C on arm-eabi.
>>>
>>> Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default
>>> cpu on trunk, and with tms570 on gcc-13.  Ok to install?
>>
>> OK.
>>
>>>
>>> for  gcc/cp/ChangeLog
>>>
>>>     * contracts.cc (check_postcondition_result): Cope with
>>>     cdtor_return_this.
>>> ---
>>>   gcc/cp/contracts.cc |    6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
>>> index 66d2298a9bfac..035ca4827e758 100644
>>> --- a/gcc/cp/contracts.cc
>>> +++ b/gcc/cp/contracts.cc
>>> @@ -636,7 +636,11 @@ make_postcondition_variable (cp_expr id)
>>>   bool
>>>   check_postcondition_result (tree decl, tree type, location_t loc)
>>>   {
>>> -  if (VOID_TYPE_P (type))
>>> +  /* Do not be confused by targetm.cxx.cdtor_return_this ();
>>> +     conceptually, cdtors have no return value.  */
>>> +  if (VOID_TYPE_P (type)
>>> +      || DECL_CONSTRUCTOR_P (decl)
>>> +      || DECL_DESTRUCTOR_P (decl))
>>>       {
>>>         error_at (loc,
>>>           DECL_CONSTRUCTOR_P (decl)
>>>
>>>
>>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: for contracts, cdtors never return this
  2024-02-07 19:46     ` Jason Merrill
@ 2024-02-09  8:47       ` Torbjorn SVENSSON
  0 siblings, 0 replies; 5+ messages in thread
From: Torbjorn SVENSSON @ 2024-02-09  8:47 UTC (permalink / raw)
  To: Jason Merrill, Alexandre Oliva, gcc-patches; +Cc: Yvan Roux



On 2024-02-07 20:46, Jason Merrill wrote:
> On 2/7/24 11:22, Torbjorn SVENSSON wrote:
>> Hi,
>>
>> Is it okay to backport 71804526d3a71a8c0f189a89ce3aa615784bfd8b to 
>> releases/gcc-13?
> 
> OK.

Pushed as eae51472f68d9f922aa3a1a636f81467bfdae87a.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-02-09  8:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-19  7:28 [PATCH] c++: for contracts, cdtors never return this Alexandre Oliva
2023-11-20 15:40 ` Jason Merrill
2024-02-07 16:22   ` Torbjorn SVENSSON
2024-02-07 19:46     ` Jason Merrill
2024-02-09  8:47       ` Torbjorn SVENSSON

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).