[v2,1/2] c++: overeager type completion in convert_to_void [PR111419]

Message ID 20230918161202.2884010-1-ppalka@redhat.com
State New
Headers
Series [v2,1/2] c++: overeager type completion in convert_to_void [PR111419] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Testing failed
linaro-tcwg-bot/tcwg_gcc_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 fail Testing failed

Commit Message

Patrick Palka Sept. 18, 2023, 4:12 p.m. UTC
  Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

-- >8 --

Here convert_to_void always completes the type of an indirection or
id-expression, but according to [expr.context] an lvalue-to-rvalue
conversion is applied to a discarded-value expression only if "the
expression is a glvalue of volatile-qualified type".  This patch
restricts convert_to_void's type completion to match.

	PR c++/111419

gcc/cp/ChangeLog:

	* cvt.cc (convert_to_void) <case INDIRECT_REF>: Only call
	complete_type if the type is volatile.
	<case VAR_DECL>: Only call complete_type if the type is volatile.
	Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-requires36.C: New test.
	* g++.dg/expr/discarded1.C: New test.
	* g++.dg/expr/discarded1a.C: New test.
---
 gcc/cp/cvt.cc                                   | 10 ++++++----
 .../g++.dg/cpp2a/concepts-requires36.C          | 16 ++++++++++++++++
 gcc/testsuite/g++.dg/expr/discarded1.C          | 15 +++++++++++++++
 gcc/testsuite/g++.dg/expr/discarded1a.C         | 17 +++++++++++++++++
 4 files changed, 54 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
 create mode 100644 gcc/testsuite/g++.dg/expr/discarded1.C
 create mode 100644 gcc/testsuite/g++.dg/expr/discarded1a.C
  

Comments

Patrick Palka Sept. 18, 2023, 4:19 p.m. UTC | #1
On Mon, 18 Sep 2023, Patrick Palka wrote:

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> -- >8 --
> 
> Here convert_to_void always completes the type of an indirection or
> id-expression, but according to [expr.context] an lvalue-to-rvalue
> conversion is applied to a discarded-value expression only if "the
> expression is a glvalue of volatile-qualified type".  This patch
> restricts convert_to_void's type completion to match.
> 
> 	PR c++/111419
> 
> gcc/cp/ChangeLog:
> 
> 	* cvt.cc (convert_to_void) <case INDIRECT_REF>: Only call
> 	complete_type if the type is volatile.
> 	<case VAR_DECL>: Only call complete_type if the type is volatile.
> 	Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-requires36.C: New test.
> 	* g++.dg/expr/discarded1.C: New test.
> 	* g++.dg/expr/discarded1a.C: New test.
> ---
>  gcc/cp/cvt.cc                                   | 10 ++++++----
>  .../g++.dg/cpp2a/concepts-requires36.C          | 16 ++++++++++++++++
>  gcc/testsuite/g++.dg/expr/discarded1.C          | 15 +++++++++++++++
>  gcc/testsuite/g++.dg/expr/discarded1a.C         | 17 +++++++++++++++++
>  4 files changed, 54 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
>  create mode 100644 gcc/testsuite/g++.dg/expr/discarded1.C
>  create mode 100644 gcc/testsuite/g++.dg/expr/discarded1a.C
> 
> diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
> index c6b52f07050..4424670356c 100644
> --- a/gcc/cp/cvt.cc
> +++ b/gcc/cp/cvt.cc
> @@ -1250,9 +1250,10 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
>      case INDIRECT_REF:
>        {
>  	tree type = TREE_TYPE (expr);
> -	int is_reference = TYPE_REF_P (TREE_TYPE (TREE_OPERAND (expr, 0)));

Oops, this removal of is_reference should happen in the follow-up patch
instead, consider that fixed.

>  	int is_volatile = TYPE_VOLATILE (type);
> -	int is_complete = COMPLETE_TYPE_P (complete_type (type));
> +	if (is_volatile)
> +	  complete_type (type);
> +	int is_complete = COMPLETE_TYPE_P (type);
>  
>  	/* Can't load the value if we don't know the type.  */
>  	if (is_volatile && !is_complete)
> @@ -1412,9 +1413,10 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
>        {
>  	/* External variables might be incomplete.  */
>  	tree type = TREE_TYPE (expr);
> -	int is_complete = COMPLETE_TYPE_P (complete_type (type));
>  
> -	if (TYPE_VOLATILE (type) && !is_complete && (complain & tf_warning))
> +	if (TYPE_VOLATILE (type)
> +	    && !COMPLETE_TYPE_P (complete_type (type))
> +	    && (complain & tf_warning))
>  	  switch (implicit)
>  	    {
>  	      case ICV_CAST:
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
> new file mode 100644
> index 00000000000..8d3a4fcd2aa
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
> @@ -0,0 +1,16 @@
> +// PR c++/111419
> +// { dg-do compile { target c++20 } }
> +
> +template<class F>
> +concept invocable = requires(F& f) { f(); };
> +
> +template<class F>
> +concept deref_invocable = requires(F& f) { *f(); };
> +
> +struct Incomplete;
> +
> +template<class T>
> +struct Holder { T t; };
> +
> +static_assert(invocable<Holder<Incomplete>& ()>);
> +static_assert(deref_invocable<Holder<Incomplete>* ()>);
> diff --git a/gcc/testsuite/g++.dg/expr/discarded1.C b/gcc/testsuite/g++.dg/expr/discarded1.C
> new file mode 100644
> index 00000000000..c0c22e9e95b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/expr/discarded1.C
> @@ -0,0 +1,15 @@
> +// PR c++/111419
> +
> +struct Incomplete;
> +
> +template<class T> struct Holder { T t; }; // { dg-bogus "incomplete" }
> +
> +extern Holder<Incomplete> a;
> +extern Holder<Incomplete>& b;
> +extern Holder<Incomplete>* c;
> +
> +int main() {
> +  a;
> +  b;
> +  *c;
> +}
> diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C b/gcc/testsuite/g++.dg/expr/discarded1a.C
> new file mode 100644
> index 00000000000..1c4dff4553e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
> @@ -0,0 +1,17 @@
> +// A version of discarded1.C using volatile types.
> +// PR c++/111419
> +
> +struct Incomplete;
> +
> +template<class T, int> struct Holder { T t; }; // { dg-error "incomplete" }
> +
> +extern volatile Holder<Incomplete, 0> a;
> +extern volatile Holder<Incomplete, 1>& b;
> +extern volatile Holder<Incomplete, 2>* c;
> +
> +int main() {
> +  a; // { dg-message "required from here" }
> +  b; // { dg-message "required from here" }
> +  // { dg-warning "implicit dereference will not access object" "" { target *-*-* } .-1 }
> +  *c; // { dg-message "required from here" }
> +}
> -- 
> 2.42.0.216.gbda494f404
> 
>
  
Jason Merrill Sept. 19, 2023, 1:45 a.m. UTC | #2
On 9/18/23 12:19, Patrick Palka wrote:
> On Mon, 18 Sep 2023, Patrick Palka wrote:
> 
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>> trunk?
>>
>> -- >8 --
>>
>> Here convert_to_void always completes the type of an indirection or
>> id-expression, but according to [expr.context] an lvalue-to-rvalue
>> conversion is applied to a discarded-value expression only if "the
>> expression is a glvalue of volatile-qualified type".  This patch
>> restricts convert_to_void's type completion to match.
>>
>> 	PR c++/111419
>>
>> gcc/cp/ChangeLog:
>>
>> 	* cvt.cc (convert_to_void) <case INDIRECT_REF>: Only call
>> 	complete_type if the type is volatile.
>> 	<case VAR_DECL>: Only call complete_type if the type is volatile.
>> 	Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/cpp2a/concepts-requires36.C: New test.
>> 	* g++.dg/expr/discarded1.C: New test.
>> 	* g++.dg/expr/discarded1a.C: New test.
>> ---
>>   gcc/cp/cvt.cc                                   | 10 ++++++----
>>   .../g++.dg/cpp2a/concepts-requires36.C          | 16 ++++++++++++++++
>>   gcc/testsuite/g++.dg/expr/discarded1.C          | 15 +++++++++++++++
>>   gcc/testsuite/g++.dg/expr/discarded1a.C         | 17 +++++++++++++++++
>>   4 files changed, 54 insertions(+), 4 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
>>   create mode 100644 gcc/testsuite/g++.dg/expr/discarded1.C
>>   create mode 100644 gcc/testsuite/g++.dg/expr/discarded1a.C
>>
>> diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
>> index c6b52f07050..4424670356c 100644
>> --- a/gcc/cp/cvt.cc
>> +++ b/gcc/cp/cvt.cc
>> @@ -1250,9 +1250,10 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
>>       case INDIRECT_REF:
>>         {
>>   	tree type = TREE_TYPE (expr);
>> -	int is_reference = TYPE_REF_P (TREE_TYPE (TREE_OPERAND (expr, 0)));
> 
> Oops, this removal of is_reference should happen in the follow-up patch
> instead, consider that fixed.

OK with that fixed.

>>   	int is_volatile = TYPE_VOLATILE (type);
>> -	int is_complete = COMPLETE_TYPE_P (complete_type (type));
>> +	if (is_volatile)
>> +	  complete_type (type);
>> +	int is_complete = COMPLETE_TYPE_P (type);
>>   
>>   	/* Can't load the value if we don't know the type.  */
>>   	if (is_volatile && !is_complete)
>> @@ -1412,9 +1413,10 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
>>         {
>>   	/* External variables might be incomplete.  */
>>   	tree type = TREE_TYPE (expr);
>> -	int is_complete = COMPLETE_TYPE_P (complete_type (type));
>>   
>> -	if (TYPE_VOLATILE (type) && !is_complete && (complain & tf_warning))
>> +	if (TYPE_VOLATILE (type)
>> +	    && !COMPLETE_TYPE_P (complete_type (type))
>> +	    && (complain & tf_warning))
>>   	  switch (implicit)
>>   	    {
>>   	      case ICV_CAST:
>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
>> new file mode 100644
>> index 00000000000..8d3a4fcd2aa
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
>> @@ -0,0 +1,16 @@
>> +// PR c++/111419
>> +// { dg-do compile { target c++20 } }
>> +
>> +template<class F>
>> +concept invocable = requires(F& f) { f(); };
>> +
>> +template<class F>
>> +concept deref_invocable = requires(F& f) { *f(); };
>> +
>> +struct Incomplete;
>> +
>> +template<class T>
>> +struct Holder { T t; };
>> +
>> +static_assert(invocable<Holder<Incomplete>& ()>);
>> +static_assert(deref_invocable<Holder<Incomplete>* ()>);
>> diff --git a/gcc/testsuite/g++.dg/expr/discarded1.C b/gcc/testsuite/g++.dg/expr/discarded1.C
>> new file mode 100644
>> index 00000000000..c0c22e9e95b
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/expr/discarded1.C
>> @@ -0,0 +1,15 @@
>> +// PR c++/111419
>> +
>> +struct Incomplete;
>> +
>> +template<class T> struct Holder { T t; }; // { dg-bogus "incomplete" }
>> +
>> +extern Holder<Incomplete> a;
>> +extern Holder<Incomplete>& b;
>> +extern Holder<Incomplete>* c;
>> +
>> +int main() {
>> +  a;
>> +  b;
>> +  *c;
>> +}
>> diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C b/gcc/testsuite/g++.dg/expr/discarded1a.C
>> new file mode 100644
>> index 00000000000..1c4dff4553e
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
>> @@ -0,0 +1,17 @@
>> +// A version of discarded1.C using volatile types.
>> +// PR c++/111419
>> +
>> +struct Incomplete;
>> +
>> +template<class T, int> struct Holder { T t; }; // { dg-error "incomplete" }
>> +
>> +extern volatile Holder<Incomplete, 0> a;
>> +extern volatile Holder<Incomplete, 1>& b;
>> +extern volatile Holder<Incomplete, 2>* c;
>> +
>> +int main() {
>> +  a; // { dg-message "required from here" }
>> +  b; // { dg-message "required from here" }
>> +  // { dg-warning "implicit dereference will not access object" "" { target *-*-* } .-1 }
>> +  *c; // { dg-message "required from here" }
>> +}
>> -- 
>> 2.42.0.216.gbda494f404
>>
>>
>
  

Patch

diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
index c6b52f07050..4424670356c 100644
--- a/gcc/cp/cvt.cc
+++ b/gcc/cp/cvt.cc
@@ -1250,9 +1250,10 @@  convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
     case INDIRECT_REF:
       {
 	tree type = TREE_TYPE (expr);
-	int is_reference = TYPE_REF_P (TREE_TYPE (TREE_OPERAND (expr, 0)));
 	int is_volatile = TYPE_VOLATILE (type);
-	int is_complete = COMPLETE_TYPE_P (complete_type (type));
+	if (is_volatile)
+	  complete_type (type);
+	int is_complete = COMPLETE_TYPE_P (type);
 
 	/* Can't load the value if we don't know the type.  */
 	if (is_volatile && !is_complete)
@@ -1412,9 +1413,10 @@  convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
       {
 	/* External variables might be incomplete.  */
 	tree type = TREE_TYPE (expr);
-	int is_complete = COMPLETE_TYPE_P (complete_type (type));
 
-	if (TYPE_VOLATILE (type) && !is_complete && (complain & tf_warning))
+	if (TYPE_VOLATILE (type)
+	    && !COMPLETE_TYPE_P (complete_type (type))
+	    && (complain & tf_warning))
 	  switch (implicit)
 	    {
 	      case ICV_CAST:
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
new file mode 100644
index 00000000000..8d3a4fcd2aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
@@ -0,0 +1,16 @@ 
+// PR c++/111419
+// { dg-do compile { target c++20 } }
+
+template<class F>
+concept invocable = requires(F& f) { f(); };
+
+template<class F>
+concept deref_invocable = requires(F& f) { *f(); };
+
+struct Incomplete;
+
+template<class T>
+struct Holder { T t; };
+
+static_assert(invocable<Holder<Incomplete>& ()>);
+static_assert(deref_invocable<Holder<Incomplete>* ()>);
diff --git a/gcc/testsuite/g++.dg/expr/discarded1.C b/gcc/testsuite/g++.dg/expr/discarded1.C
new file mode 100644
index 00000000000..c0c22e9e95b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/discarded1.C
@@ -0,0 +1,15 @@ 
+// PR c++/111419
+
+struct Incomplete;
+
+template<class T> struct Holder { T t; }; // { dg-bogus "incomplete" }
+
+extern Holder<Incomplete> a;
+extern Holder<Incomplete>& b;
+extern Holder<Incomplete>* c;
+
+int main() {
+  a;
+  b;
+  *c;
+}
diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C b/gcc/testsuite/g++.dg/expr/discarded1a.C
new file mode 100644
index 00000000000..1c4dff4553e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
@@ -0,0 +1,17 @@ 
+// A version of discarded1.C using volatile types.
+// PR c++/111419
+
+struct Incomplete;
+
+template<class T, int> struct Holder { T t; }; // { dg-error "incomplete" }
+
+extern volatile Holder<Incomplete, 0> a;
+extern volatile Holder<Incomplete, 1>& b;
+extern volatile Holder<Incomplete, 2>* c;
+
+int main() {
+  a; // { dg-message "required from here" }
+  b; // { dg-message "required from here" }
+  // { dg-warning "implicit dereference will not access object" "" { target *-*-* } .-1 }
+  *c; // { dg-message "required from here" }
+}