[PATCH 1/2] c++: casting array prvalue [PR112658, PR94264]

[PATCH 1/2] c++: casting array prvalue [PR112658, PR94264]

[Patrick Palka]

Bootstrapped and regtested on x86-64-pc-linux-gnu, does this look OK for
trunk?

-- >8 --

Here we deem the array-to-pointer conversions in both calls as invalid,
but we fail to issue a diagnostic for the second call, ultimately because
cp_build_c_cast doesn't replay errors from build_const_cast_1.  This means
the second call get silently discarded leading to wrong/unexpected code.

This patch fixes this issue.  I'm not sure if we want to accept these
conversions in the first place (that's PR94264 or at least related to
it), but at least we're more consistent now.

	PR c++/112658
	PR c++/94264

gcc/cp/ChangeLog:

	* typeck.cc (cp_build_c_cast): If we're committed to a const_cast
	and the result is erroneous, call build_const_cast_1 a second
	time to issue errors.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/initlist-array20.C: New test.
---
 gcc/cp/typeck.cc                              |  4 +++-
 gcc/testsuite/g++.dg/cpp0x/initlist-array20.C | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-array20.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index e995fb6ddd7..b112bea4d1e 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -9209,6 +9209,8 @@ cp_build_c_cast (location_t loc, tree type, tree expr,
 	  maybe_warn_about_useless_cast (loc, type, value, complain);
 	  maybe_warn_about_cast_ignoring_quals (loc, type, complain);
 	}
+      else if (complain & tf_error)
+	build_const_cast_1 (loc, type, value, tf_error, &valid_p);
       return result;
     }
 
@@ -9244,7 +9246,7 @@ cp_build_c_cast (location_t loc, tree type, tree expr,
 	 to succeed.  */
       if (!same_type_p (non_reference (type), non_reference (result_type)))
 	{
-	  result = build_const_cast_1 (loc, type, result, false, &valid_p);
+	  result = build_const_cast_1 (loc, type, result, tf_none, &valid_p);
 	  gcc_assert (valid_p);
 	}
       return result;
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-array20.C b/gcc/testsuite/g++.dg/cpp0x/initlist-array20.C
new file mode 100644
index 00000000000..967b67023f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-array20.C
@@ -0,0 +1,10 @@
+// PR c++/112658
+// { dg-do compile { target c++11 } }
+
+void f(int*);
+
+int main() {
+  using array = int[];
+  f(array{42}); // { dg-error "address of temporary array" }
+  f((int*)array{42}); // { dg-error "address of temporary array" }
+}
-- 
2.43.0.rc1

[PATCH 2/2] c++: guard more against undiagnosed error_mark_node [PR112658]

[Patrick Palka]

This adds a sanity check to cp_parser_expression_statement similar to
the one in finish_expr_stmt added by r6-6795-g0fd9d4921f7ba2, which
effectively downgrades accepts-invalid/wrong-code bugs like this one
into ice-on-invalid/ice-on-valid ones.

	PR c++/112658

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_expression_statement): If the statement
	is erroneous, make sure we've seen an error.
---
 gcc/cp/parser.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 2464d1a0783..743d6517b09 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -12962,6 +12962,9 @@ cp_parser_expression_statement (cp_parser* parser, tree in_statement_expr)
       if (statement == error_mark_node
 	  && !cp_parser_uncommitted_to_tentative_parse_p (parser))
 	{
+	  /* If we ran into a problem, make sure we complained.  */
+	  gcc_assert (seen_error ());
+
 	  cp_parser_skip_to_end_of_block_or_statement (parser);
 	  return error_mark_node;
 	}
-- 
2.43.0.rc1

Re: [PATCH 1/2] c++: casting array prvalue [PR112658, PR94264]

[Jason Merrill]

On 11/28/23 11:51, Patrick Palka wrote:
> Bootstrapped and regtested on x86-64-pc-linux-gnu, does this look OK for
> trunk?
> 
> -- >8 --
> 
> Here we deem the array-to-pointer conversions in both calls as invalid,
> but we fail to issue a diagnostic for the second call, ultimately because
> cp_build_c_cast doesn't replay errors from build_const_cast_1.  This means
> the second call get silently discarded leading to wrong/unexpected code.
> 
> This patch fixes this issue.  I'm not sure if we want to accept these
> conversions in the first place (that's PR94264 or at least related to
> it), but at least we're more consistent now.

I've now fixed that bug, thanks for the pointer.

The cp_build_c_cast change is OK, but the testcase won't error anymore. 
Do you have an idea for an alternate test?  If not, it's OK to apply the 
fix anyway.

Jason

Re: [PATCH 2/2] c++: guard more against undiagnosed error_mark_node [PR112658]

[Jason Merrill]

On 11/28/23 11:51, Patrick Palka wrote:
> This adds a sanity check to cp_parser_expression_statement similar to
> the one in finish_expr_stmt added by r6-6795-g0fd9d4921f7ba2, which
> effectively downgrades accepts-invalid/wrong-code bugs like this one
> into ice-on-invalid/ice-on-valid ones.

OK.

> 	PR c++/112658
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_expression_statement): If the statement
> 	is erroneous, make sure we've seen an error.
> ---
>   gcc/cp/parser.cc | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 2464d1a0783..743d6517b09 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -12962,6 +12962,9 @@ cp_parser_expression_statement (cp_parser* parser, tree in_statement_expr)
>         if (statement == error_mark_node
>   	  && !cp_parser_uncommitted_to_tentative_parse_p (parser))
>   	{
> +	  /* If we ran into a problem, make sure we complained.  */
> +	  gcc_assert (seen_error ());
> +
>   	  cp_parser_skip_to_end_of_block_or_statement (parser);
>   	  return error_mark_node;
>   	}