diff mbox series

c++: Relax checking assert about elision to support -fno-elide-constructors [PR114619]

Message ID 01020192a40a6da2-b98a9365-88fe-42d1-92a1-4c58c9d88909-000000@eu-west-1.amazonses.com
State New
Headers show
Series c++: Relax checking assert about elision to support -fno-elide-constructors [PR114619] | expand

Commit Message

Simon Martin Oct. 19, 2024, 9:09 a.m. UTC 
We currently ICE in checking mode with cxx_dialect < 17 on the following
valid code

=== cut here ===
struct X {
  X(const X&) {}
};
extern X x;
void foo () {
  new X[1]{x};
}
=== cut here ===

The problem is that cp_gimplify_expr gcc_checking_asserts that a
TARGET_EXPR is not TARGET_EXPR_ELIDING_P (or cannot be elided), while in
this case with cxx_dialect < 17, it is TARGET_EXPR_ELIDING_P but we have
not even tried to elide.

This patch relaxes that gcc_checking_assert to not fail when using
cxx_dialect < 17 and -fno-elide-constructors (I considered being more
clever at setting TARGET_EXPR_ELIDING_P appropriately but it looks more
risky and not worth the extra complexity for a checking assert).

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/114619

gcc/cp/ChangeLog:

	* cp-gimplify.cc (cp_gimplify_expr): Relax gcc_checking_assert
	to support the usage of -fno-elide-constructors with c++ < 17.

gcc/testsuite/ChangeLog:

	* g++.dg/init/no-elide3.C: New test.

---
 gcc/cp/cp-gimplify.cc                 |  7 ++++++-
 gcc/testsuite/g++.dg/init/no-elide3.C | 11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/init/no-elide3.C

Comments

Simon Martin Oct. 30, 2024, 10:46 a.m. UTC | #1 
On 19 Oct 2024, at 11:09, Simon Martin wrote:

> We currently ICE in checking mode with cxx_dialect < 17 on the 
> following
> valid code
>
> === cut here ===
> struct X {
>   X(const X&) {}
> };
> extern X x;
> void foo () {
>   new X[1]{x};
> }
> === cut here ===
>
> The problem is that cp_gimplify_expr gcc_checking_asserts that a
> TARGET_EXPR is not TARGET_EXPR_ELIDING_P (or cannot be elided), while 
> in
> this case with cxx_dialect < 17, it is TARGET_EXPR_ELIDING_P but we 
> have
> not even tried to elide.
>
> This patch relaxes that gcc_checking_assert to not fail when using
> cxx_dialect < 17 and -fno-elide-constructors (I considered being more
> clever at setting TARGET_EXPR_ELIDING_P appropriately but it looks 
> more
> risky and not worth the extra complexity for a checking assert).
>
> Successfully tested on x86_64-pc-linux-gnu.
Friendly ping. Thanks!

>
> 	PR c++/114619
>
> gcc/cp/ChangeLog:
>
> 	* cp-gimplify.cc (cp_gimplify_expr): Relax gcc_checking_assert
> 	to support the usage of -fno-elide-constructors with c++ < 17.
>
> gcc/testsuite/ChangeLog:
>
> 	* g++.dg/init/no-elide3.C: New test.
>
> ---
>  gcc/cp/cp-gimplify.cc                 |  7 ++++++-
>  gcc/testsuite/g++.dg/init/no-elide3.C | 11 +++++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/init/no-elide3.C
>
> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> index 003e68f1ea7..354ea73c63b 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -908,7 +908,12 @@ cp_gimplify_expr (tree *expr_p, gimple_seq 
> *pre_p, gimple_seq *post_p)
>  	 gimplify_init_ctor_preeval can materialize subobjects of a 
> CONSTRUCTOR
>  	 on the rhs of an assignment, as in constexpr-aggr1.C.  */
>        gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
> -			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));
> +			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p))
> +			   /* If we explicitly asked not to elide and it's not
> +			      required by the standard, we can't expect elision
> +			      to have happened.  */
> +			   || (cxx_dialect < cxx17
> +			       && !flag_elide_constructors));
>        ret = GS_UNHANDLED;
>        break;
>
> diff --git a/gcc/testsuite/g++.dg/init/no-elide3.C 
> b/gcc/testsuite/g++.dg/init/no-elide3.C
> new file mode 100644
> index 00000000000..9377d9f0161
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/init/no-elide3.C
> @@ -0,0 +1,11 @@
> +// PR c++/114619
> +// { dg-do "compile" { target c++11 } }
> +// { dg-options "-fno-elide-constructors" }
> +
> +struct X {
> +  X(const X&) {}
> +};
> +extern X x;
> +void foo () {
> +  new X[1]{x};
> +}
> -- 
> 2.44.0
Simon Martin Nov. 11, 2024, 7:35 p.m. UTC | #2 
Hi,

On 30 Oct 2024, at 11:46, Simon Martin wrote:

> On 19 Oct 2024, at 11:09, Simon Martin wrote:
>
>> We currently ICE in checking mode with cxx_dialect < 17 on the
>> following
>> valid code
>>
>> === cut here ===
>> struct X {
>>   X(const X&) {}
>> };
>> extern X x;
>> void foo () {
>>   new X[1]{x};
>> }
>> === cut here ===
>>
>> The problem is that cp_gimplify_expr gcc_checking_asserts that a
>> TARGET_EXPR is not TARGET_EXPR_ELIDING_P (or cannot be elided), while
>> in
>> this case with cxx_dialect < 17, it is TARGET_EXPR_ELIDING_P but we
>> have
>> not even tried to elide.
>>
>> This patch relaxes that gcc_checking_assert to not fail when using
>> cxx_dialect < 17 and -fno-elide-constructors (I considered being more
>> clever at setting TARGET_EXPR_ELIDING_P appropriately but it looks
>> more
>> risky and not worth the extra complexity for a checking assert).
>>
>> Successfully tested on x86_64-pc-linux-gnu.
> Friendly ping. Thanks!
Friendly ping. Thanks!

Simon

>
>>
>> 	PR c++/114619
>>
>> gcc/cp/ChangeLog:
>>
>> 	* cp-gimplify.cc (cp_gimplify_expr): Relax gcc_checking_assert
>> 	to support the usage of -fno-elide-constructors with c++ < 17.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/init/no-elide3.C: New test.
>>
>> ---
>>  gcc/cp/cp-gimplify.cc                 |  7 ++++++-
>>  gcc/testsuite/g++.dg/init/no-elide3.C | 11 +++++++++++
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/g++.dg/init/no-elide3.C
>>
>> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
>> index 003e68f1ea7..354ea73c63b 100644
>> --- a/gcc/cp/cp-gimplify.cc
>> +++ b/gcc/cp/cp-gimplify.cc
>> @@ -908,7 +908,12 @@ cp_gimplify_expr (tree *expr_p, gimple_seq
>> *pre_p, gimple_seq *post_p)
>>  	 gimplify_init_ctor_preeval can materialize subobjects of a
>> CONSTRUCTOR
>>  	 on the rhs of an assignment, as in constexpr-aggr1.C.  */
>>        gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
>> -			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));
>> +			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p))
>> +			   /* If we explicitly asked not to elide and it's not
>> +			      required by the standard, we can't expect elision
>> +			      to have happened.  */
>> +			   || (cxx_dialect < cxx17
>> +			       && !flag_elide_constructors));
>>        ret = GS_UNHANDLED;
>>        break;
>>
>> diff --git a/gcc/testsuite/g++.dg/init/no-elide3.C
>> b/gcc/testsuite/g++.dg/init/no-elide3.C
>> new file mode 100644
>> index 00000000000..9377d9f0161
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/init/no-elide3.C
>> @@ -0,0 +1,11 @@
>> +// PR c++/114619
>> +// { dg-do "compile" { target c++11 } }
>> +// { dg-options "-fno-elide-constructors" }
>> +
>> +struct X {
>> +  X(const X&) {}
>> +};
>> +extern X x;
>> +void foo () {
>> +  new X[1]{x};
>> +}
>> -- 
>> 2.44.0
Simon Martin Jan. 7, 2025, 3:13 p.m. UTC | #3 
Hi,

On 11 Nov 2024, at 20:35, Simon Martin wrote:

> Hi,
>
> On 30 Oct 2024, at 11:46, Simon Martin wrote:
>
>> On 19 Oct 2024, at 11:09, Simon Martin wrote:
>>
>>> We currently ICE in checking mode with cxx_dialect < 17 on the
>>> following
>>> valid code
>>>
>>> === cut here ===
>>> struct X {
>>>   X(const X&) {}
>>> };
>>> extern X x;
>>> void foo () {
>>>   new X[1]{x};
>>> }
>>> === cut here ===
>>>
>>> The problem is that cp_gimplify_expr gcc_checking_asserts that a
>>> TARGET_EXPR is not TARGET_EXPR_ELIDING_P (or cannot be elided), while
>>> in
>>> this case with cxx_dialect < 17, it is TARGET_EXPR_ELIDING_P but we
>>> have
>>> not even tried to elide.
>>>
>>> This patch relaxes that gcc_checking_assert to not fail when using
>>> cxx_dialect < 17 and -fno-elide-constructors (I considered being more
>>> clever at setting TARGET_EXPR_ELIDING_P appropriately but it looks
>>> more
>>> risky and not worth the extra complexity for a checking assert).
>>>
>>> Successfully tested on x86_64-pc-linux-gnu.
>> Friendly ping. Thanks!
> Friendly ping. Thanks!
Ping. Thanks!

Simon
>>
>>>
>>> 	PR c++/114619
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* cp-gimplify.cc (cp_gimplify_expr): Relax gcc_checking_assert
>>> 	to support the usage of -fno-elide-constructors with c++ < 17.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/init/no-elide3.C: New test.
>>>
>>> ---
>>>  gcc/cp/cp-gimplify.cc                 |  7 ++++++-
>>>  gcc/testsuite/g++.dg/init/no-elide3.C | 11 +++++++++++
>>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>>  create mode 100644 gcc/testsuite/g++.dg/init/no-elide3.C
>>>
>>> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
>>> index 003e68f1ea7..354ea73c63b 100644
>>> --- a/gcc/cp/cp-gimplify.cc
>>> +++ b/gcc/cp/cp-gimplify.cc
>>> @@ -908,7 +908,12 @@ cp_gimplify_expr (tree *expr_p, gimple_seq
>>> *pre_p, gimple_seq *post_p)
>>>  	 gimplify_init_ctor_preeval can materialize subobjects of a
>>> CONSTRUCTOR
>>>  	 on the rhs of an assignment, as in constexpr-aggr1.C.  */
>>>        gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
>>> -			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));
>>> +			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p))
>>> +			   /* If we explicitly asked not to elide and it's not
>>> +			      required by the standard, we can't expect elision
>>> +			      to have happened.  */
>>> +			   || (cxx_dialect < cxx17
>>> +			       && !flag_elide_constructors));
>>>        ret = GS_UNHANDLED;
>>>        break;
>>>
>>> diff --git a/gcc/testsuite/g++.dg/init/no-elide3.C
>>> b/gcc/testsuite/g++.dg/init/no-elide3.C
>>> new file mode 100644
>>> index 00000000000..9377d9f0161
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/init/no-elide3.C
>>> @@ -0,0 +1,11 @@
>>> +// PR c++/114619
>>> +// { dg-do "compile" { target c++11 } }
>>> +// { dg-options "-fno-elide-constructors" }
>>> +
>>> +struct X {
>>> +  X(const X&) {}
>>> +};
>>> +extern X x;
>>> +void foo () {
>>> +  new X[1]{x};
>>> +}
>>> -- 
>>> 2.44.0
diff mbox series

Patch

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 003e68f1ea7..354ea73c63b 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -908,7 +908,12 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	 gimplify_init_ctor_preeval can materialize subobjects of a CONSTRUCTOR
 	 on the rhs of an assignment, as in constexpr-aggr1.C.  */
       gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
-			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));
+			   || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p))
+			   /* If we explicitly asked not to elide and it's not
+			      required by the standard, we can't expect elision
+			      to have happened.  */
+			   || (cxx_dialect < cxx17
+			       && !flag_elide_constructors));
       ret = GS_UNHANDLED;
       break;
 
diff --git a/gcc/testsuite/g++.dg/init/no-elide3.C b/gcc/testsuite/g++.dg/init/no-elide3.C
new file mode 100644
index 00000000000..9377d9f0161
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/no-elide3.C
@@ -0,0 +1,11 @@ 
+// PR c++/114619
+// { dg-do "compile" { target c++11 } }
+// { dg-options "-fno-elide-constructors" }
+
+struct X {
+  X(const X&) {}
+};
+extern X x;
+void foo () {
+  new X[1]{x};
+}