Re: [PATCH] c++, v4: Implement C++26 P2741R3 - user-generated static_assert messages [PR110348]

[Jason Merrill <jason@redhat.com>]

On 11/23/23 03:32, Jakub Jelinek wrote:
> On Wed, Nov 22, 2023 at 04:53:48PM -0500, Jason Merrill wrote:
>> I agree it's weird to get two of the same error, but maybe instead of
>> duplicating the error, we could look up data only if size succeeded, and
>> then error once if either failed?
> 
> Here is what I've committed after another bootstrap/regtest on x86_64-linux
> and i686-linux.  Besides the above requested change I've tweaked 2 lines
> in the test not to rely on a particular std::size_t exact type because
> otherwise the test failed on i686-linux.  And accepting there only the
> current
> unsigned int
> long unsigned int
> long long unsinged int
> unsigned __int20__ (or how exactly is this one spelled in diagnostics)
> seems fragile.
> 
> --- gcc/cp/semantics.cc.jj	2023-11-22 11:30:08.019325101 +0100
> +++ gcc/cp/semantics.cc	2023-11-22 22:58:25.194480633 +0100
> @@ -11485,9 +11534,89 @@ finish_static_assert (tree condition, tr
>   	  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;
> +	  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);

Jonathan pointed out elsewhere that this gets leaked if error return 
prevents us from getting to the XDELETEVEC.

> +		  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);
> +			  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;
> +	    }
>   
>   	  /* See if we can find which clause was failing (for logical AND).  */
>   	  tree bad = find_failing_clause (NULL, orig_condition);
> @@ -11497,12 +11626,13 @@ finish_static_assert (tree condition, tr
>   
>   	  auto_diagnostic_group d;
>   
> -          /* Report the error. */
> +	  /* Report the error. */
>   	  if (len == 0)
>   	    error_at (cloc, "static assertion failed");
>   	  else
> -	    error_at (cloc, "static assertion failed: %s",
> -		      TREE_STRING_POINTER (message));
> +	    error_at (cloc, "static assertion failed: %.*s", len, msg);
> +
> +	  XDELETEVEC (buf);
>   
>   	  diagnose_failing_condition (bad, cloc, show_expr_p);
>   	}