[RFC] c++: static_assert (false) in template [DR2518]

Message ID 20230217203209.2141339-1-jason@redhat.com
State Accepted
Headers
Series [RFC] c++: static_assert (false) in template [DR2518] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Jason Merrill Feb. 17, 2023, 8:32 p.m. UTC
  Tested x86_64-pc-linux-gnu.  This isn't really a regression fix, but it's very
safe, and fixes a longstanding annoyance, so I'm leaning toward putting it into
GCC 13.  Thoughts?

-- 8< --

For a long time, people have expected to be able to write
static_assert (false) in a template and only have it diagnosed if the
template is instantiated, but we (and other implementations) gave an error
about the uninstantiated template because the standard says that if no valid
instantiation of the template is possible, the program is ill-formed, no
diagnostic required, and we try to diagnose IFNDR things when feasible.

At the meeting last week we were looking at CWG2518, which wanted to specify
that an implementation must not accept a program containing a failing #error
or static_assert.  We also looked at P2593, which proposed allowing
static_assert in an uninstantiated template.  We ended up combining these
two in order to avoid requiring implementations to reject programs with
static_assert (false) in uninstantiated templates.

The committee accepted this as a DR, so I'm making the change to all
standard modes.  This behavior was also conformant previously, since no
diagnostic was required in this case.

We continue to diagnose non-constant or otherwise ill-formed conditions, so
no changes to existing tests were needed.

	DR 2518
	PR c++/52809
	PR c++/53638
	PR c++/87389
	PR c++/89741
	PR c++/92099
	PR c++/104041
	PR c++/104691

gcc/cp/ChangeLog:

	* semantics.cc (finish_static_assert): Don't diagnose in
	template context.

gcc/testsuite/ChangeLog:

	* g++.dg/DRs/dr2518.C: New test.
---
 gcc/cp/semantics.cc               | 17 ++++++++++-------
 gcc/testsuite/g++.dg/DRs/dr2518.C |  7 +++++++
 2 files changed, 17 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2518.C


base-commit: 07f497c2da3600cc99cd7d1b5c6726972fb2b5a1
  

Comments

Jakub Jelinek Feb. 17, 2023, 9 p.m. UTC | #1
On Fri, Feb 17, 2023 at 03:32:09PM -0500, Jason Merrill via Gcc-patches wrote:
> Tested x86_64-pc-linux-gnu.  This isn't really a regression fix, but it's very
> safe, and fixes a longstanding annoyance, so I'm leaning toward putting it into
> GCC 13.  Thoughts?

I think it is ok for GCC 13.

I bet for the testsuite coverage of the whole DR2518 wording we have already
tons of testcases which verify we actually error on #error directives and
warn on #warning directives and error in failed static_assert unless in
uninstantiated templates that it isn't worth repeating them in the
dr2518*.C testcase(s).

I'm a little bit surprised we didn't check for errors from static_asserts
failed asserts in uninstantiated templates...

	Jakub
  
Marek Polacek Feb. 17, 2023, 10:45 p.m. UTC | #2
On Fri, Feb 17, 2023 at 03:32:09PM -0500, Jason Merrill via Gcc-patches wrote:
> Tested x86_64-pc-linux-gnu.  This isn't really a regression fix, but it's very
> safe, and fixes a longstanding annoyance, so I'm leaning toward putting it into
> GCC 13.  Thoughts?

LGTM.
 
> -- 8< --
> 
> For a long time, people have expected to be able to write
> static_assert (false) in a template and only have it diagnosed if the
> template is instantiated, but we (and other implementations) gave an error
> about the uninstantiated template because the standard says that if no valid
> instantiation of the template is possible, the program is ill-formed, no
> diagnostic required, and we try to diagnose IFNDR things when feasible.
> 
> At the meeting last week we were looking at CWG2518, which wanted to specify
> that an implementation must not accept a program containing a failing #error
> or static_assert.  We also looked at P2593, which proposed allowing
> static_assert in an uninstantiated template.  We ended up combining these
> two in order to avoid requiring implementations to reject programs with
> static_assert (false) in uninstantiated templates.
> 
> The committee accepted this as a DR, so I'm making the change to all
> standard modes.  This behavior was also conformant previously, since no
> diagnostic was required in this case.
> 
> We continue to diagnose non-constant or otherwise ill-formed conditions, so
> no changes to existing tests were needed.
> 
> 	DR 2518
> 	PR c++/52809
> 	PR c++/53638
> 	PR c++/87389
> 	PR c++/89741
> 	PR c++/92099
> 	PR c++/104041
> 	PR c++/104691
> 
> gcc/cp/ChangeLog:
> 
> 	* semantics.cc (finish_static_assert): Don't diagnose in
> 	template context.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/DRs/dr2518.C: New test.
> ---
>  gcc/cp/semantics.cc               | 17 ++++++++++-------
>  gcc/testsuite/g++.dg/DRs/dr2518.C |  7 +++++++
>  2 files changed, 17 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/DRs/dr2518.C
> 
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index c2df0b69b30..79b7cc72f21 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -11232,14 +11232,16 @@ finish_static_assert (tree condition, tree message, location_t location,
>    if (check_for_bare_parameter_packs (condition))
>      condition = error_mark_node;
>  
> +  /* Save the condition in case it was a concept check.  */
> +  tree orig_condition = condition;
> +
>    if (instantiation_dependent_expression_p (condition))
>      {
>        /* We're in a template; build a STATIC_ASSERT and put it in
>           the right place. */
> -      tree assertion;
> -
> -      assertion = make_node (STATIC_ASSERT);
> -      STATIC_ASSERT_CONDITION (assertion) = condition;
> +    defer:
> +      tree assertion = make_node (STATIC_ASSERT);
> +      STATIC_ASSERT_CONDITION (assertion) = orig_condition;
>        STATIC_ASSERT_MESSAGE (assertion) = message;
>        STATIC_ASSERT_SOURCE_LOCATION (assertion) = location;
>  
> @@ -11253,9 +11255,6 @@ finish_static_assert (tree condition, tree message, location_t location,
>        return;
>      }
>  
> -  /* Save the condition in case it was a concept check.  */
> -  tree orig_condition = condition;
> -
>    /* Fold the expression and convert it to a boolean value. */
>    condition = contextual_conv_bool (condition, complain);
>    condition = fold_non_dependent_expr (condition, complain,
> @@ -11270,6 +11269,10 @@ finish_static_assert (tree condition, tree message, location_t location,
>  
>        if (integer_zerop (condition))
>  	{
> +	  /* CWG2518: static_assert failure in a template is not IFNDR.  */
> +	  if (processing_template_decl)
> +	    goto defer;
> +
>  	  int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT
>  				     (TREE_TYPE (TREE_TYPE (message))));
>  	  int len = TREE_STRING_LENGTH (message) / sz - 1;
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2518.C b/gcc/testsuite/g++.dg/DRs/dr2518.C
> new file mode 100644
> index 00000000000..240186211e6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/DRs/dr2518.C
> @@ -0,0 +1,7 @@
> +// CWG 2518
> +// { dg-do compile { target c++11 } }
> +
> +template <class T> void f()
> +{
> +  static_assert (false, "");
> +}
> 
> base-commit: 07f497c2da3600cc99cd7d1b5c6726972fb2b5a1
> -- 
> 2.31.1
> 

Marek
  

Patch

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index c2df0b69b30..79b7cc72f21 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -11232,14 +11232,16 @@  finish_static_assert (tree condition, tree message, location_t location,
   if (check_for_bare_parameter_packs (condition))
     condition = error_mark_node;
 
+  /* Save the condition in case it was a concept check.  */
+  tree orig_condition = condition;
+
   if (instantiation_dependent_expression_p (condition))
     {
       /* We're in a template; build a STATIC_ASSERT and put it in
          the right place. */
-      tree assertion;
-
-      assertion = make_node (STATIC_ASSERT);
-      STATIC_ASSERT_CONDITION (assertion) = condition;
+    defer:
+      tree assertion = make_node (STATIC_ASSERT);
+      STATIC_ASSERT_CONDITION (assertion) = orig_condition;
       STATIC_ASSERT_MESSAGE (assertion) = message;
       STATIC_ASSERT_SOURCE_LOCATION (assertion) = location;
 
@@ -11253,9 +11255,6 @@  finish_static_assert (tree condition, tree message, location_t location,
       return;
     }
 
-  /* Save the condition in case it was a concept check.  */
-  tree orig_condition = condition;
-
   /* Fold the expression and convert it to a boolean value. */
   condition = contextual_conv_bool (condition, complain);
   condition = fold_non_dependent_expr (condition, complain,
@@ -11270,6 +11269,10 @@  finish_static_assert (tree condition, tree message, location_t location,
 
       if (integer_zerop (condition))
 	{
+	  /* CWG2518: static_assert failure in a template is not IFNDR.  */
+	  if (processing_template_decl)
+	    goto defer;
+
 	  int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT
 				     (TREE_TYPE (TREE_TYPE (message))));
 	  int len = TREE_STRING_LENGTH (message) / sz - 1;
diff --git a/gcc/testsuite/g++.dg/DRs/dr2518.C b/gcc/testsuite/g++.dg/DRs/dr2518.C
new file mode 100644
index 00000000000..240186211e6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2518.C
@@ -0,0 +1,7 @@ 
+// CWG 2518
+// { dg-do compile { target c++11 } }
+
+template <class T> void f()
+{
+  static_assert (false, "");
+}