diff mbox series

[v2,1/2] Factor out static_assert constexpr string extraction for reuse

Message ID 20240603034520.1552764-1-ak@linux.intel.com
State New
Headers show
Series [v2,1/2] Factor out static_assert constexpr string extraction for reuse | expand

Commit Message

Andi Kleen June 3, 2024, 3:45 a.m. UTC
No intentional semantics change.

gcc/cp/ChangeLog:

	* cp-tree.h (struct cstr): Add structure.
	(get_cstr): Declare.
	(extract_cstr): Declare.
	(free_cstr): Declare.
	* semantics.cc (finish_static_assert): Factor out constant
	string expression extraction code and move to...
	(get_cstr): Here.
	(extract_cstr): Dito.
	(free_cstr): Dito.
---
 gcc/cp/cp-tree.h    |  17 +++
 gcc/cp/semantics.cc | 292 +++++++++++++++++++++++++-------------------
 2 files changed, 184 insertions(+), 125 deletions(-)

Comments

Jason Merrill June 3, 2024, 2:36 p.m. UTC | #1
On 6/2/24 23:45, Andi Kleen wrote:
> No intentional semantics change.
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (struct cstr): Add structure.
> 	(get_cstr): Declare.
> 	(extract_cstr): Declare.
> 	(free_cstr): Declare.
> 	* semantics.cc (finish_static_assert): Factor out constant
> 	string expression extraction code and move to...
> 	(get_cstr): Here.
> 	(extract_cstr): Dito.
> 	(free_cstr): Dito.
> ---
>   gcc/cp/cp-tree.h    |  17 +++
>   gcc/cp/semantics.cc | 292 +++++++++++++++++++++++++-------------------
>   2 files changed, 184 insertions(+), 125 deletions(-)
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 565e4a9290e2..25b8033db788 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -9015,6 +9015,23 @@ struct push_access_scope_guard
>     }
>   };
>   
> +/* Extracting strings from constexpr */
> +
> +/* Temporary data for extracting constant string.  */
> +struct cstr
> +{
> +  tree message;
> +  tree message_data;
> +  tree message_sz;
> +  char *buf;
> +};
> +
> +bool get_cstr (tree message, location_t location, const char *what, const char *what2,
> +	       cstr &cstr);
> +bool extract_cstr (const char *what, const char *what2, location_t location,
> +		   cstr &cstr, const char * & msg, int &len);
> +void free_cstr (cstr &cstr);

get_ and extract_ are confusingly similar names.  Let's make cstr more 
of a class, initialized at least from 'message', and perhaps from the 
other context information.

Then get_ can be something like cstr::type_check.  And then free_cstr is 
the destructor, and the copy constructor is deleted.

And please don't use the same name for the class and parameter/variables 
of that type.

> +	  error_at (location, "%qs %s must be a string "

 From ABOUT-GCC-NLS: "Avoid using %s to compose a diagnostic message 
from multiple translatable strings; instead, write out the full 
diagnostic message for each variant. Only use %s for message components 
that do not need translation, such as keywords."

Also, if you are passing an English string for a diagnostic to a 
function that isn't from diagnostic.h, you need to use the intl.h macros 
to mark it for translation.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 565e4a9290e2..25b8033db788 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -9015,6 +9015,23 @@  struct push_access_scope_guard
   }
 };
 
+/* Extracting strings from constexpr */
+
+/* Temporary data for extracting constant string.  */
+struct cstr
+{
+  tree message;
+  tree message_data;
+  tree message_sz;
+  char *buf;
+};
+
+bool get_cstr (tree message, location_t location, const char *what, const char *what2,
+	       cstr &cstr);
+bool extract_cstr (const char *what, const char *what2, location_t location,
+		   cstr &cstr, const char * & msg, int &len);
+void free_cstr (cstr &cstr);
+
 /* True if TYPE is an extended floating-point type.  */
 
 inline bool
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index f90c304a65b7..9b0a2aee4e49 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -11558,74 +11558,196 @@  init_cp_semantics (void)
 }
 
 
-/* Build a STATIC_ASSERT for a static assertion with the condition
-   CONDITION and the message text MESSAGE.  LOCATION is the location
-   of the static assertion in the source code.  When MEMBER_P, this
-   static assertion is a member of a class.  If SHOW_EXPR_P is true,
-   print the condition (because it was instantiation-dependent).  */
+/* Get constant string from MESSAGE at LOCATION for WHAT and WHAT2 and save into CSTR. Returns
+   true if successfull, otherwise false.  */
 
-void
-finish_static_assert (tree condition, tree message, location_t location,
-		      bool member_p, bool show_expr_p)
+bool
+get_cstr (tree message, location_t location, const char *what, const char *what2,
+	  cstr &cstr)
 {
   tsubst_flags_t complain = tf_warning_or_error;
-  tree message_sz = NULL_TREE, message_data = NULL_TREE;
+  cstr = { message, NULL_TREE, NULL_TREE, NULL };
 
   if (message == NULL_TREE
       || message == error_mark_node
-      || condition == NULL_TREE
-      || condition == error_mark_node)
-    return;
-
-  if (check_for_bare_parameter_packs (condition)
       || check_for_bare_parameter_packs (message))
-    return;
+    return false;
 
   if (TREE_CODE (message) != STRING_CST
       && !type_dependent_expression_p (message))
     {
-      message_sz
+      cstr.message_sz
 	= finish_class_member_access_expr (message,
 					   get_identifier ("size"),
 					   false, complain);
-      if (message_sz != error_mark_node)
-	message_data
+      if (cstr.message_sz != error_mark_node)
+	cstr.message_data
 	  = finish_class_member_access_expr (message,
 					     get_identifier ("data"),
 					     false, complain);
-      if (message_sz == error_mark_node || message_data == error_mark_node)
+      if (cstr.message_sz == error_mark_node || cstr.message_data == error_mark_node)
 	{
-	  error_at (location, "%<static_assert%> message must be a string "
-			      "literal or object with %<size%> and "
-			      "%<data%> members");
-	  return;
+	  error_at (location, "%qs %s must be a string "
+		    "literal or object with %<size%> and "
+		    "%<data%> members", what, what2);
+	  return false;
 	}
       releasing_vec size_args, data_args;
-      message_sz = finish_call_expr (message_sz, &size_args, false, false,
+      cstr.message_sz = finish_call_expr (cstr.message_sz, &size_args, false, false,
 				     complain);
-      message_data = finish_call_expr (message_data, &data_args, false, false,
+      cstr.message_data = finish_call_expr (cstr.message_data, &data_args, false, false,
 				       complain);
-      if (message_sz == error_mark_node || message_data == error_mark_node)
-	return;
-      message_sz = build_converted_constant_expr (size_type_node, message_sz,
-						  complain);
-      if (message_sz == error_mark_node)
+      if (cstr.message_sz == error_mark_node || cstr.message_data == error_mark_node)
+	return false;
+      cstr.message_sz = build_converted_constant_expr (size_type_node, cstr.message_sz,
+						       complain);
+      if (cstr.message_sz == error_mark_node)
 	{
-	  error_at (location, "%<static_assert%> message %<size()%> "
-			      "must be implicitly convertible to "
-			      "%<std::size_t%>");
-	  return;
+	  error_at (location, "%qs %s %<size()%> "
+		    "must be implicitly convertible to "
+		    "%<std::size_t%>", what, what2);
+	  return false;
 	}
-      message_data = build_converted_constant_expr (const_string_type_node,
-						    message_data, complain);
-      if (message_data == error_mark_node)
+      cstr.message_data = build_converted_constant_expr (const_string_type_node,
+							 cstr.message_data, complain);
+      if (cstr.message_data == error_mark_node)
 	{
-	  error_at (location, "%<static_assert%> message %<data()%> "
-			      "must be implicitly convertible to "
-			      "%<const char*%>");
-	  return;
+	  error_at (location, "%qs %s %<data()%> "
+		    "must be implicitly convertible to "
+		    "%<const char*%>", what, what2);
+	  return false;
+	}
+    }
+  return true;
+}
+
+/* Extract constant string CSTR into output paramter MSG with output LEN,
+   using WHAT and WHAT2 for error messages.
+   Returns true if successfull, otherwise false. CSTR must be freed with free_cstr
+   if return was true.  */
+
+bool
+extract_cstr (const char *what, const char *what2, location_t location,
+	      cstr &cstr, const char * & msg, int &len)
+{
+  tsubst_flags_t complain = tf_warning_or_error;
+
+  msg = NULL;
+  if (cstr.message_sz && cstr.message_data)
+    {
+      tree msz = cxx_constant_value (cstr.message_sz, NULL_TREE, complain);
+      if (!tree_fits_uhwi_p (msz))
+	{
+	  error_at (location,
+		    "%qs %s %<size()%> "
+		    "must be a constant expression", what, what2);
+	  return false;
+	}
+      else if ((unsigned HOST_WIDE_INT) (int) tree_to_uhwi (msz)
+	       != tree_to_uhwi (msz))
+	{
+	  error_at (location,
+		    "%qs message %<size()%> "
+		    "%qE too large", what, msz);
+	  return false;
+	}
+      len = tree_to_uhwi (msz);
+      tree data = maybe_constant_value (cstr.message_data, NULL_TREE,
+					mce_true);
+      if (!reduced_constant_expression_p (data))
+	data = NULL_TREE;
+      if (len)
+	{
+	  if (data)
+	    msg = c_getstr (data);
+	  if (msg == NULL)
+	    cstr.buf = XNEWVEC (char, len);
+	  for (int i = 0; i < len; ++i)
+	    {
+	      tree t = cstr.message_data;
+	      if (i)
+		t = build2 (POINTER_PLUS_EXPR,
+			    TREE_TYPE (cstr.message_data), cstr.message_data,
+			    size_int (i));
+	      t = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (t)), t);
+	      tree t2 = cxx_constant_value (t, NULL_TREE, complain);
+	      if (!tree_fits_shwi_p (t2))
+		{
+		  error_at (location,
+			    "%qs %s %<data()[%d]%> "
+			    "must be a constant expression", what, what2, i);
+		  XDELETEVEC (cstr.buf);
+		  return false;
+		}
+	      if (msg == NULL)
+		cstr.buf[i] = tree_to_shwi (t2);
+	      /* If c_getstr worked, just verify the first and
+		 last characters using constant evaluation.  */
+	      else if (len > 2 && i == 0)
+		i = len - 2;
+	    }
+	  if (msg == NULL)
+	    msg = cstr.buf;
+	}
+      else if (!data)
+	{
+	  /* We don't have any function to test whether some
+	     expression is a core constant expression.  So, instead
+	     test whether (message.data (), 0) is a constant
+	     expression.  */
+	  data = build2 (COMPOUND_EXPR, integer_type_node,
+			 cstr.message_data, integer_zero_node);
+	  tree t = cxx_constant_value (data, NULL_TREE, complain);
+	  if (!integer_zerop (t))
+	    {
+	      error_at (location,
+			"%qs %s %<data()%> "
+			"must be a core constant expression", what, what2);
+	      return false;
+	    }
 	}
     }
+  else
+    {
+      tree eltype = TREE_TYPE (TREE_TYPE (cstr.message));
+      int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (eltype));
+      msg = TREE_STRING_POINTER (cstr.message);
+      len = TREE_STRING_LENGTH (cstr.message) / sz - 1;
+    }
+
+  return true;
+}
+
+/* Free constant string data CSTR.  */
+
+void
+free_cstr (cstr &cstr)
+{
+  XDELETEVEC (cstr.buf);
+}
+
+/* Build a STATIC_ASSERT for a static assertion with the condition
+   CONDITION and the message text MESSAGE.  LOCATION is the location
+   of the static assertion in the source code.  When MEMBER_P, this
+   static assertion is a member of a class.  If SHOW_EXPR_P is true,
+   print the condition (because it was instantiation-dependent).  */
+
+void
+finish_static_assert (tree condition, tree message, location_t location,
+		      bool member_p, bool show_expr_p)
+{
+  tsubst_flags_t complain = tf_warning_or_error;
+
+  if (condition == NULL_TREE
+      || condition == error_mark_node)
+    return;
+
+  if (check_for_bare_parameter_packs (condition))
+    return;
+
+  cstr cstr;
+  if (!get_cstr (message, location, "static_assert", "message", cstr))
+    return;
 
   /* Save the condition in case it was a concept check.  */
   tree orig_condition = condition;
@@ -11638,7 +11760,7 @@  finish_static_assert (tree condition, tree message, location_t location,
     defer:
       tree assertion = make_node (STATIC_ASSERT);
       STATIC_ASSERT_CONDITION (assertion) = orig_condition;
-      STATIC_ASSERT_MESSAGE (assertion) = message;
+      STATIC_ASSERT_MESSAGE (assertion) = cstr.message;
       STATIC_ASSERT_SOURCE_LOCATION (assertion) = location;
 
       if (member_p)
@@ -11671,88 +11793,8 @@  finish_static_assert (tree condition, tree message, location_t location,
 
 	  int len;
 	  const char *msg = NULL;
-	  char *buf = NULL;
-	  if (message_sz && message_data)
-	    {
-	      tree msz = cxx_constant_value (message_sz, NULL_TREE, complain);
-	      if (!tree_fits_uhwi_p (msz))
-		{
-		  error_at (location,
-			    "%<static_assert%> message %<size()%> "
-			    "must be a constant expression");
-		  return;
-		}
-	      else if ((unsigned HOST_WIDE_INT) (int) tree_to_uhwi (msz)
-		       != tree_to_uhwi (msz))
-		{
-		  error_at (location,
-			    "%<static_assert%> message %<size()%> "
-			    "%qE too large", msz);
-		  return;
-		}
-	      len = tree_to_uhwi (msz);
-	      tree data = maybe_constant_value (message_data, NULL_TREE,
-						mce_true);
-	      if (!reduced_constant_expression_p (data))
-		data = NULL_TREE;
-	      if (len)
-		{
-		  if (data)
-		    msg = c_getstr (data);
-		  if (msg == NULL)
-		    buf = XNEWVEC (char, len);
-		  for (int i = 0; i < len; ++i)
-		    {
-		      tree t = message_data;
-		      if (i)
-			t = build2 (POINTER_PLUS_EXPR,
-				    TREE_TYPE (message_data), message_data,
-				    size_int (i));
-		      t = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (t)), t);
-		      tree t2 = cxx_constant_value (t, NULL_TREE, complain);
-		      if (!tree_fits_shwi_p (t2))
-			{
-			  error_at (location,
-				    "%<static_assert%> message %<data()[%d]%> "
-				    "must be a constant expression", i);
-			  XDELETEVEC (buf);
-			  return;
-			}
-		      if (msg == NULL)
-			buf[i] = tree_to_shwi (t2);
-		      /* If c_getstr worked, just verify the first and
-			 last characters using constant evaluation.  */
-		      else if (len > 2 && i == 0)
-			i = len - 2;
-		    }
-		  if (msg == NULL)
-		    msg = buf;
-		}
-	      else if (!data)
-		{
-		  /* We don't have any function to test whether some
-		     expression is a core constant expression.  So, instead
-		     test whether (message.data (), 0) is a constant
-		     expression.  */
-		  data = build2 (COMPOUND_EXPR, integer_type_node,
-				 message_data, integer_zero_node);
-		  tree t = cxx_constant_value (data, NULL_TREE, complain);
-		  if (!integer_zerop (t))
-		    {
-		      error_at (location,
-				"%<static_assert%> message %<data()%> "
-				"must be a core constant expression");
-		      return;
-		    }
-		}
-	    }
-	  else
-	    {
-	      tree eltype = TREE_TYPE (TREE_TYPE (message));
-	      int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (eltype));
-	      msg = TREE_STRING_POINTER (message);
-	      len = TREE_STRING_LENGTH (message) / sz - 1;
-	    }
+	  if (!extract_cstr ("static_assert", "message", location, cstr, msg, len))
+	    return;
 
 	  /* See if we can find which clause was failing (for logical AND).  */
 	  tree bad = find_failing_clause (NULL, orig_condition);
@@ -11768,7 +11810,7 @@  finish_static_assert (tree condition, tree message, location_t location,
 	  else
 	    error_at (cloc, "static assertion failed: %.*s", len, msg);
 
-	  XDELETEVEC (buf);
+	  free_cstr (cstr);
 
 	  diagnose_failing_condition (bad, cloc, show_expr_p);
 	}