c++/contracts: ICE with contract assert on non empty statement [PR 117579]

Message ID 20241202155913.29628-2-dinka.ranns@gmail.com
State New
Headers
Series c++/contracts: ICE with contract assert on non empty statement [PR 117579] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed

Commit Message

Nina Ranns Dec. 2, 2024, 3:59 p.m. UTC
  Tested on x86_64-pc-linux-gnu.
First time using git send-email, let me know if anything needs to be
done differently.
OK for trunk?
Thanks,
Nina

----

Contract assert is an attribute on a non empty statement. Currently we
assert that the statement is empty before emitting the assertion. This
has been changed to a conditional check that the statement is empty
before the assertion is emitted.

	PR c++/117579

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_statement): assertion replaced with a
	conditional check that the statement containing a contract
	assert is empty.

gcc/testsuite/ChangeLog:

	* g++.dg/contracts/pr117579.C: New test.

Signed-off-by: Nina Ranns <dinka.ranns@gmail.com>
---
 gcc/cp/parser.cc                          | 6 ++++--
 gcc/testsuite/g++.dg/contracts/pr117579.C | 9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/contracts/pr117579.C
  

Comments

Jason Merrill Dec. 2, 2024, 5:04 p.m. UTC | #1
On 12/2/24 10:59 AM, Nina Ranns wrote:
> 
> Tested on x86_64-pc-linux-gnu.
> First time using git send-email, let me know if anything needs to be
> done differently.

Thanks, just a few tweaks.

> OK for trunk?

This question implies to me that the sender has commit access, which I 
don't think you do yet since you aren't in MAINTAINERS.  But I can't 
think of a similar standard question for contributors without commit 
access; I guess just leave it out.

> Thanks,
> Nina
> 
> ----

For git-am this line should contain scissors, e.g.

-- 8< --

> Contract assert is an attribute on a non empty statement. Currently we

"on an empty statement", right?

> assert that the statement is empty before emitting the assertion. This
> has been changed to a conditional check that the statement is empty
> before the assertion is emitted.
> 
> 	PR c++/117579
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_statement): assertion replaced with a

Capitalize the beginning of the sentence, and change to active voice, 
e.g. "Replace assertion..."

> 	conditional check that the statement containing a contract
> 	assert is empty.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/contracts/pr117579.C: New test.
> 
> Signed-off-by: Nina Ranns <dinka.ranns@gmail.com>
> ---
>   gcc/cp/parser.cc                          | 6 ++++--
>   gcc/testsuite/g++.dg/contracts/pr117579.C | 9 +++++++++
>   2 files changed, 13 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/contracts/pr117579.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index f60ed47dfd7..e655e5cc0db 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -13082,8 +13082,10 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>         if (cp_contract_assertion_p (std_attrs))
>   	{
>   	  /* Add the assertion as a statement in the current block.  */
> -	  gcc_assert (!statement || statement == error_mark_node);
> -	  emit_assertion (std_attrs);
> +	  if (!statement)
> +	    emit_assertion (std_attrs);
> +	  /* we already checked that the contract assertion is followed by

Capitalize the beginning of the sentence.

> +	   a semicolon.  */
>   	  std_attrs = NULL_TREE;
>   	}
>       }
> diff --git a/gcc/testsuite/g++.dg/contracts/pr117579.C b/gcc/testsuite/g++.dg/contracts/pr117579.C
> new file mode 100644
> index 00000000000..f15cdf0c78d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/contracts/pr117579.C
> @@ -0,0 +1,9 @@
> +// check that contract assertion on a non empty statement doesn't cause an ICE

Capitalize the beginning of the sentence and end with a period.

> +// { dg-do compile }

This is the default; it's OK to have this line, but unnecessary.

> +// { dg-options "-std=c++2a -fcontracts " }
> +
> +void f();
> +int main ()
> +{
> +  [[assert: true]] f(); // { dg-error "assertions must be followed by" }
> +}
  

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f60ed47dfd7..e655e5cc0db 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -13082,8 +13082,10 @@  cp_parser_statement (cp_parser* parser, tree in_statement_expr,
       if (cp_contract_assertion_p (std_attrs))
 	{
 	  /* Add the assertion as a statement in the current block.  */
-	  gcc_assert (!statement || statement == error_mark_node);
-	  emit_assertion (std_attrs);
+	  if (!statement)
+	    emit_assertion (std_attrs);
+	  /* we already checked that the contract assertion is followed by
+	   a semicolon.  */
 	  std_attrs = NULL_TREE;
 	}
     }
diff --git a/gcc/testsuite/g++.dg/contracts/pr117579.C b/gcc/testsuite/g++.dg/contracts/pr117579.C
new file mode 100644
index 00000000000..f15cdf0c78d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/contracts/pr117579.C
@@ -0,0 +1,9 @@ 
+// check that contract assertion on a non empty statement doesn't cause an ICE
+// { dg-do compile }
+// { dg-options "-std=c++2a -fcontracts " }
+
+void f();
+int main ()
+{
+  [[assert: true]] f(); // { dg-error "assertions must be followed by" }
+}