diff mbox

[C++17] Implement N3928 - Extending static_assert

Message ID 55453092.5050501@verizon.net
State New
Headers show

Commit Message

Ed Smith-Rowland May 2, 2015, 8:16 p.m. UTC
This extends' static assert to not require a message string.
I elected to make this work also for C++11 and C++14 and warn only with 
-pedantic.
I think many people just write
   static_assert(thing, "");
.

I took the path of building an empty string in the parser in this case.
I wasn't sure if setting message to NULL_TREE would cause sadness later 
on or not.

I also, perhaps in a fit of overzealousness made finish_static_assert 
not print the extra ": " and an empty message in this case.

I didn't modify _Static_assert for C.

Built and tested on x86_64-linux.

OK
cp/

2015-05-02  Edward Smith-Rowland  <3dw4rd@verizon.net>

	Implement N3928 - Extending static_assert
	* parser.c (cp_parser_static_assert): Support static_assert with
	no message string.  Supply an empty string in this case.
	* semantics.c (finish_static_assert): Don't try to print a message if
	the message strnig is empty.


testsuite/

2015-05-02  Edward Smith-Rowland  <3dw4rd@verizon.net>

	Implement N3928 - Extending static_assert
	* g++.dg/cpp0x/static_assert8.C: Adjust.
	* g++.dg/cpp0x/static_assert12.C: New.
	* g++.dg/cpp0x/static_assert13.C: New.
	* g++.dg/cpp1y/static_assert1.C: New.
	* g++.dg/cpp1y/static_assert2.C: New.
	* g++.dg/cpp1z/static_assert-nomsg.C: New.

Comments

Marek Polacek May 4, 2015, 3:28 p.m. UTC | #1
On Sat, May 02, 2015 at 04:16:18PM -0400, Ed Smith-Rowland wrote:
> This extends' static assert to not require a message string.
> I elected to make this work also for C++11 and C++14 and warn only with
> -pedantic.
> I think many people just write
>   static_assert(thing, "");
> .
> 
> I took the path of building an empty string in the parser in this case.
> I wasn't sure if setting message to NULL_TREE would cause sadness later on
> or not.
> 
> I also, perhaps in a fit of overzealousness made finish_static_assert not
> print the extra ": " and an empty message in this case.
> 
> I didn't modify _Static_assert for C.

I'm not aware of any C DR that is asking for _Static_assert (cst-expr), so
I suppose there's no need to change C at this point.

	Marek
Ed Smith-Rowland May 20, 2015, 1:48 a.m. UTC | #2
On 05/02/2015 04:16 PM, Ed Smith-Rowland wrote:
> This extends' static assert to not require a message string.
> I elected to make this work also for C++11 and C++14 and warn only 
> with -pedantic.
> I think many people just write
>   static_assert(thing, "");
> .
>
> I took the path of building an empty string in the parser in this case.
> I wasn't sure if setting message to NULL_TREE would cause sadness 
> later on or not.
>
> I also, perhaps in a fit of overzealousness made finish_static_assert 
> not print the extra ": " and an empty message in this case.
>
> I didn't modify _Static_assert for C.
>
> Built and tested on x86_64-linux.
>
> OK
>

Ping? Comments?
Jason Merrill May 20, 2015, 3:28 p.m. UTC | #3
On 05/02/2015 04:16 PM, Ed Smith-Rowland wrote:
> This extends' static assert to not require a message string.
> I elected to make this work also for C++11 and C++14 and warn only with
> -pedantic.
> I think many people just write
>    static_assert(thing, "");
> .
>
> I took the path of building an empty string in the parser in this case.
> I wasn't sure if setting message to NULL_TREE would cause sadness later
> on or not.

Hmm.  Yes, this technically implements the feature, but my impression of 
the (non-normative) intent was that they wanted leaving out the string 
to print the argument expression, in about the same way as

#define BOOST_STATIC_ASSERT( B ) static_assert(B, #B)

So the patch is OK as is, but you might also look into some libcpp magic 
to insert a second argument that stringizes the first.

Jason
Jason Merrill June 15, 2015, 4:05 p.m. UTC | #4
On 05/20/2015 11:28 AM, Jason Merrill wrote:
> On 05/02/2015 04:16 PM, Ed Smith-Rowland wrote:
>> This extends' static assert to not require a message string.
>> I elected to make this work also for C++11 and C++14 and warn only with
>> -pedantic.
>> I think many people just write
>>    static_assert(thing, "");
>> .
>>
>> I took the path of building an empty string in the parser in this case.
>> I wasn't sure if setting message to NULL_TREE would cause sadness later
>> on or not.
>
> Hmm.  Yes, this technically implements the feature, but my impression of
> the (non-normative) intent was that they wanted leaving out the string
> to print the argument expression, in about the same way as
>
> #define BOOST_STATIC_ASSERT( B ) static_assert(B, #B)
>
> So the patch is OK as is, but you might also look into some libcpp magic
> to insert a second argument that stringizes the first.

Are you planning to check this in?

Jason
Ed Smith-Rowland June 15, 2015, 11:14 p.m. UTC | #5
On 06/15/2015 12:05 PM, Jason Merrill wrote:
> On 05/20/2015 11:28 AM, Jason Merrill wrote:
>> On 05/02/2015 04:16 PM, Ed Smith-Rowland wrote:
>>> This extends' static assert to not require a message string.
>>> I elected to make this work also for C++11 and C++14 and warn only with
>>> -pedantic.
>>> I think many people just write
>>>    static_assert(thing, "");
>>> .
>>>
>>> I took the path of building an empty string in the parser in this case.
>>> I wasn't sure if setting message to NULL_TREE would cause sadness later
>>> on or not.
>>
>> Hmm.  Yes, this technically implements the feature, but my impression of
>> the (non-normative) intent was that they wanted leaving out the string
>> to print the argument expression, in about the same way as
>>
>> #define BOOST_STATIC_ASSERT( B ) static_assert(B, #B)
>>
>> So the patch is OK as is, but you might also look into some libcpp magic
>> to insert a second argument that stringizes the first.
>
> Are you planning to check this in?
>
> Jason
>
>
>

Jason,

I wanted to fix it up as per your suggestion.  If someone wants it now I 
can retest and commit.  Otherwise give me a bit more time.

Also, if you or someone else really has the whole enchilada then by all 
means just commit that.

Ed
Jason Merrill June 17, 2015, 2:23 p.m. UTC | #6
On 06/15/2015 07:14 PM, Ed Smith-Rowland wrote:
> I wanted to fix it up as per your suggestion.  If someone wants it now I
> can retest and commit.  Otherwise give me a bit more time.

Someone in LWG was asking about it, and I figured it wouldn't hurt to 
have this version in now.  Glad to hear you're working on the 
improvement as well, thanks!

Jason
Ed Smith-Rowland June 17, 2015, 5:53 p.m. UTC | #7
On 06/17/2015 10:23 AM, Jason Merrill wrote:
> On 06/15/2015 07:14 PM, Ed Smith-Rowland wrote:
>> I wanted to fix it up as per your suggestion.  If someone wants it now I
>> can retest and commit.  Otherwise give me a bit more time.
>
> Someone in LWG was asking about it, and I figured it wouldn't hurt to 
> have this version in now.  Glad to hear you're working on the 
> improvement as well, thanks!
>
> Jason
>
>
I tried the obvious: an error message with %qE and got 'false'. 
constexpr values are evaluated early on.

Is there a possibility that late folding could help or is that 
completely different?

Ed
Jason Merrill June 17, 2015, 7:22 p.m. UTC | #8
On 06/17/2015 01:53 PM, Ed Smith-Rowland wrote:
> I tried the obvious: an error message with %qE and got 'false'.
> constexpr values are evaluated early on.
>
> Is there a possibility that late folding could help or is that
> completely different?

Late folding could help, but I think handling it in libcpp (by actually 
stringizing the argument) would work better.

Jason
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 222723)
+++ cp/parser.c	(working copy)
@@ -12153,6 +12153,7 @@ 
 
    static_assert-declaration:
      static_assert ( constant-expression , string-literal ) ; 
+     static_assert ( constant-expression ) ; (C++1Z)
 
    If MEMBER_P, this static_assert is a class member.  */
 
@@ -12190,20 +12191,35 @@ 
                                    /*allow_non_constant_p=*/true,
                                    /*non_constant_p=*/&dummy);
 
-  /* Parse the separating `,'.  */
-  cp_parser_require (parser, CPP_COMMA, RT_COMMA);
+  if (cp_lexer_peek_token (parser->lexer)->type == CPP_CLOSE_PAREN)
+    {
+      if (cxx_dialect < cxx1z)
+	pedwarn (input_location, OPT_Wpedantic,
+		 "static_assert without a message "
+		 "only available with -std=c++1z or -std=gnu++1z");
+      /* Eat the ')'  */
+      cp_lexer_consume_token (parser->lexer);
+      message = build_string (1, "");
+      TREE_TYPE (message) = char_array_type_node;
+      fix_string_type (message);
+    }
+  else
+    {
+      /* Parse the separating `,'.  */
+      cp_parser_require (parser, CPP_COMMA, RT_COMMA);
 
-  /* Parse the string-literal message.  */
-  message = cp_parser_string_literal (parser, 
-                                      /*translate=*/false,
-                                      /*wide_ok=*/true);
+      /* Parse the string-literal message.  */
+      message = cp_parser_string_literal (parser, 
+                                	  /*translate=*/false,
+                                	  /*wide_ok=*/true);
 
-  /* A `)' completes the static assertion.  */
-  if (!cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN))
-    cp_parser_skip_to_closing_parenthesis (parser, 
-                                           /*recovering=*/true, 
-                                           /*or_comma=*/false,
-					   /*consume_paren=*/true);
+      /* A `)' completes the static assertion.  */
+      if (!cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN))
+	cp_parser_skip_to_closing_parenthesis (parser, 
+                                               /*recovering=*/true, 
+                                               /*or_comma=*/false,
+					       /*consume_paren=*/true);
+    }
 
   /* A semicolon terminates the declaration.  */
   cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 222723)
+++ cp/semantics.c	(working copy)
@@ -7184,8 +7184,17 @@ 
       input_location = location;
       if (TREE_CODE (condition) == INTEGER_CST 
           && integer_zerop (condition))
-        /* Report the error. */
-        error ("static assertion failed: %s", TREE_STRING_POINTER (message));
+	{
+	  int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT
+				     (TREE_TYPE (TREE_TYPE (message))));
+	  int len = TREE_STRING_LENGTH (message) / sz - 1;
+          /* Report the error. */
+	  if (len == 0)
+            error ("static assertion failed");
+	  else
+            error ("static assertion failed: %s",
+		   TREE_STRING_POINTER (message));
+	}
       else if (condition && condition != error_mark_node)
 	{
 	  error ("non-constant condition for static assertion");
Index: testsuite/g++.dg/cpp0x/static_assert12.C
===================================================================
--- testsuite/g++.dg/cpp0x/static_assert12.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/static_assert12.C	(working copy)
@@ -0,0 +1,30 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++11 -pedantic" }
+
+template<typename T>
+  struct is_float
+  {
+    static constexpr bool value = false;
+  };
+
+template<>
+  struct is_float<float>
+  {
+    static constexpr bool value = true;
+  };
+
+template<typename T>
+  T
+  float_thing(T __x)
+  {
+    static_assert(is_float<T>::value, ""); // { dg-error "static assertion failed" }
+    static_assert(is_float<T>::value); // { dg-error "static assertion failed" }
+  }
+
+int
+main()
+{
+  float_thing(1);
+}
+
+// { dg-warning "static_assert without a message only available with " "" { target *-*-* } 21 }
Index: testsuite/g++.dg/cpp0x/static_assert13.C
===================================================================
--- testsuite/g++.dg/cpp0x/static_assert13.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/static_assert13.C	(working copy)
@@ -0,0 +1,28 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++11" }
+
+template<typename T>
+  struct is_float
+  {
+    static constexpr bool value = false;
+  };
+
+template<>
+  struct is_float<float>
+  {
+    static constexpr bool value = true;
+  };
+
+template<typename T>
+  T
+  float_thing(T __x)
+  {
+    static_assert(is_float<T>::value, ""); // { dg-error "static assertion failed" }
+    static_assert(is_float<T>::value); // { dg-error "static assertion failed" }
+  }
+
+int
+main()
+{
+  float_thing(1);
+}
Index: testsuite/g++.dg/cpp0x/static_assert8.C
===================================================================
--- testsuite/g++.dg/cpp0x/static_assert8.C	(revision 222723)
+++ testsuite/g++.dg/cpp0x/static_assert8.C	(working copy)
@@ -1,7 +1,9 @@ 
 // { dg-do compile { target c++11 } }
 
-static_assert (1 == 0); // { dg-error "expected (string-literal|',') before" }
+static_assert (1 == 0); // { dg-error "static assertion failed" }
 
 static_assert (1 == 0,); // { dg-error "expected string-literal before '\\)'" }
 
 static_assert (1 == 0, "oops"); // { dg-error "static assertion failed" }
+
+// { dg-error "static_assert without a message only available with " "" { target *-*-* } 3 }
Index: testsuite/g++.dg/cpp1y/static_assert1.C
===================================================================
--- testsuite/g++.dg/cpp1y/static_assert1.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/static_assert1.C	(working copy)
@@ -0,0 +1,30 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++14 -pedantic" }
+
+template<typename T>
+  struct is_float
+  {
+    static constexpr bool value = false;
+  };
+
+template<>
+  struct is_float<float>
+  {
+    static constexpr bool value = true;
+  };
+
+template<typename T>
+  T
+  float_thing(T __x)
+  {
+    static_assert(is_float<T>::value, ""); // { dg-error "static assertion failed" }
+    static_assert(is_float<T>::value); // { dg-error "static assertion failed" }
+  }
+
+int
+main()
+{
+  float_thing(1);
+}
+
+// { dg-warning "static_assert without a message only available with " "" { target *-*-* } 21 }
Index: testsuite/g++.dg/cpp1y/static_assert2.C
===================================================================
--- testsuite/g++.dg/cpp1y/static_assert2.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/static_assert2.C	(working copy)
@@ -0,0 +1,28 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++14" }
+
+template<typename T>
+  struct is_float
+  {
+    static constexpr bool value = false;
+  };
+
+template<>
+  struct is_float<float>
+  {
+    static constexpr bool value = true;
+  };
+
+template<typename T>
+  T
+  float_thing(T __x)
+  {
+    static_assert(is_float<T>::value, ""); // { dg-error "static assertion failed" }
+    static_assert(is_float<T>::value); // { dg-error "static assertion failed" }
+  }
+
+int
+main()
+{
+  float_thing(1);
+}
Index: testsuite/g++.dg/cpp1z/static_assert-nomsg.C
===================================================================
--- testsuite/g++.dg/cpp1z/static_assert-nomsg.C	(revision 0)
+++ testsuite/g++.dg/cpp1z/static_assert-nomsg.C	(working copy)
@@ -0,0 +1,27 @@ 
+// { dg-do compile { target c++1z } }
+
+template<typename T>
+  struct is_float
+  {
+    static constexpr bool value = false;
+  };
+
+template<>
+  struct is_float<float>
+  {
+    static constexpr bool value = true;
+  };
+
+template<typename T>
+  T
+  float_thing(T __x)
+  {
+    static_assert(is_float<T>::value, ""); // { dg-error "static assertion failed" }
+    static_assert(is_float<T>::value); // { dg-error "static assertion failed" }
+  }
+
+int
+main()
+{
+  float_thing(1);
+}