public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c-family: Tweak -Woverflow diagnostic
@ 2022-03-30 22:28 Marek Polacek
  2022-04-01 18:07 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Marek Polacek @ 2022-03-30 22:28 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill, Joseph Myers

When g++ emits

warning: overflow in conversion from 'int' to 'char' changes value from '300' to '',''

for code like "char c = 300;" it might raise a few eyebrows.  With this
warning we're not interested in the ASCII representation of the char, only
the numerical value, so convert constants of type char to int.  It looks
like this conversion only needs to be done for char_type_node.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?  I'm also happy
to defer this to GCC 13.

gcc/c-family/ChangeLog:

	* c-warn.cc (warnings_for_convert_and_check): Convert constants of type
	char to int.

gcc/testsuite/ChangeLog:

	* c-c++-common/Wconversion-1.c: New test.
---
 gcc/c-family/c-warn.cc                     | 16 +++++++++++-----
 gcc/testsuite/c-c++-common/Wconversion-1.c | 14 ++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wconversion-1.c

diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index f24ac5d0539..cae89294aea 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -1404,8 +1404,14 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
     result = TREE_OPERAND (result, 1);
 
   bool cst = TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant;
-
   tree exprtype = TREE_TYPE (expr);
+  tree result_diag;
+  /* We're interested in the actual numerical value here, not its ASCII
+     representation.  */
+  if (cst && TYPE_MAIN_VARIANT (TREE_TYPE (result)) == char_type_node)
+    result_diag = fold_convert (integer_type_node, result);
+  else
+    result_diag = result;
 
   if (TREE_CODE (expr) == INTEGER_CST
       && (TREE_CODE (type) == INTEGER_TYPE
@@ -1430,7 +1436,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
 				  "changes value from %qE to %qE")
 			     : G_("unsigned conversion from %qT to %qT "
 				  "changes value from %qE to %qE")),
-			    exprtype, type, expr, result);
+			    exprtype, type, expr, result_diag);
 	      else
 		warning_at (loc, OPT_Woverflow,
 			    (TYPE_UNSIGNED (exprtype)
@@ -1449,7 +1455,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
 	    warning_at (loc, OPT_Woverflow,
 			"overflow in conversion from %qT to %qT "
 			"changes value from %qE to %qE",
-			exprtype, type, expr, result);
+			exprtype, type, expr, result_diag);
 	  else
 	    warning_at (loc, OPT_Woverflow,
 			"overflow in conversion from %qT to %qT "
@@ -1466,7 +1472,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
 	    warning_at (loc, OPT_Woverflow,
 			"overflow in conversion from %qT to %qT "
 			"changes value from %qE to %qE",
-			exprtype, type, expr, result);
+			exprtype, type, expr, result_diag);
 	  else
 	    warning_at (loc, OPT_Woverflow,
 			"overflow in conversion from %qT to %qT "
@@ -1483,7 +1489,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
 	warning_at (loc, OPT_Woverflow,
 		    "overflow in conversion from %qT to %qT "
 		    "changes value from %qE to %qE",
-		    exprtype, type, expr, result);
+		    exprtype, type, expr, result_diag);
       else
 	warning_at (loc, OPT_Woverflow,
 		    "overflow in conversion from %qT to %qT "
diff --git a/gcc/testsuite/c-c++-common/Wconversion-1.c b/gcc/testsuite/c-c++-common/Wconversion-1.c
new file mode 100644
index 00000000000..ed65918c70f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wconversion-1.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-Wconversion" } */
+
+typedef char T;
+
+void g()
+{
+  char c = 300; /* { dg-warning "conversion from .int. to .char. changes value from .300. to .44." } */
+  T t = 300; /* { dg-warning "conversion from .int. to .T. {aka .char.} changes value from .300. to .44." } */
+  signed char sc = 300; /* { dg-warning "conversion from .int. to .signed char. changes value from .300. to .44." } */
+  unsigned char uc = 300; /* { dg-warning "conversion from .int. to .unsigned char. changes value from .300. to .44." } */
+  unsigned char uc2 = 300u; /* { dg-warning "conversion from .unsigned int. to .unsigned char. changes value from .300. to .44." } */
+  char c2 = (double)1.0 + 200; /* { dg-warning "overflow in conversion from .double. to .char. changes value from .2.01e\\+2. to .127." } */
+}

base-commit: b4e4b35f4ebe561826489bed971324efc99c5423
-- 
2.35.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c-family: Tweak -Woverflow diagnostic
  2022-03-30 22:28 [PATCH] c-family: Tweak -Woverflow diagnostic Marek Polacek
@ 2022-04-01 18:07 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2022-04-01 18:07 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches, Joseph Myers

On 3/30/22 18:28, Marek Polacek wrote:
> When g++ emits
> 
> warning: overflow in conversion from 'int' to 'char' changes value from '300' to '',''
> 
> for code like "char c = 300;" it might raise a few eyebrows.  With this
> warning we're not interested in the ASCII representation of the char, only
> the numerical value, so convert constants of type char to int.  It looks
> like this conversion only needs to be done for char_type_node.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?  I'm also happy
> to defer this to GCC 13.

OK for stage 1.

> gcc/c-family/ChangeLog:
> 
> 	* c-warn.cc (warnings_for_convert_and_check): Convert constants of type
> 	char to int.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/Wconversion-1.c: New test.
> ---
>   gcc/c-family/c-warn.cc                     | 16 +++++++++++-----
>   gcc/testsuite/c-c++-common/Wconversion-1.c | 14 ++++++++++++++
>   2 files changed, 25 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/c-c++-common/Wconversion-1.c
> 
> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
> index f24ac5d0539..cae89294aea 100644
> --- a/gcc/c-family/c-warn.cc
> +++ b/gcc/c-family/c-warn.cc
> @@ -1404,8 +1404,14 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
>       result = TREE_OPERAND (result, 1);
>   
>     bool cst = TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant;
> -
>     tree exprtype = TREE_TYPE (expr);
> +  tree result_diag;
> +  /* We're interested in the actual numerical value here, not its ASCII
> +     representation.  */
> +  if (cst && TYPE_MAIN_VARIANT (TREE_TYPE (result)) == char_type_node)
> +    result_diag = fold_convert (integer_type_node, result);
> +  else
> +    result_diag = result;
>   
>     if (TREE_CODE (expr) == INTEGER_CST
>         && (TREE_CODE (type) == INTEGER_TYPE
> @@ -1430,7 +1436,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
>   				  "changes value from %qE to %qE")
>   			     : G_("unsigned conversion from %qT to %qT "
>   				  "changes value from %qE to %qE")),
> -			    exprtype, type, expr, result);
> +			    exprtype, type, expr, result_diag);
>   	      else
>   		warning_at (loc, OPT_Woverflow,
>   			    (TYPE_UNSIGNED (exprtype)
> @@ -1449,7 +1455,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
>   	    warning_at (loc, OPT_Woverflow,
>   			"overflow in conversion from %qT to %qT "
>   			"changes value from %qE to %qE",
> -			exprtype, type, expr, result);
> +			exprtype, type, expr, result_diag);
>   	  else
>   	    warning_at (loc, OPT_Woverflow,
>   			"overflow in conversion from %qT to %qT "
> @@ -1466,7 +1472,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
>   	    warning_at (loc, OPT_Woverflow,
>   			"overflow in conversion from %qT to %qT "
>   			"changes value from %qE to %qE",
> -			exprtype, type, expr, result);
> +			exprtype, type, expr, result_diag);
>   	  else
>   	    warning_at (loc, OPT_Woverflow,
>   			"overflow in conversion from %qT to %qT "
> @@ -1483,7 +1489,7 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr,
>   	warning_at (loc, OPT_Woverflow,
>   		    "overflow in conversion from %qT to %qT "
>   		    "changes value from %qE to %qE",
> -		    exprtype, type, expr, result);
> +		    exprtype, type, expr, result_diag);
>         else
>   	warning_at (loc, OPT_Woverflow,
>   		    "overflow in conversion from %qT to %qT "
> diff --git a/gcc/testsuite/c-c++-common/Wconversion-1.c b/gcc/testsuite/c-c++-common/Wconversion-1.c
> new file mode 100644
> index 00000000000..ed65918c70f
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wconversion-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wconversion" } */
> +
> +typedef char T;
> +
> +void g()
> +{
> +  char c = 300; /* { dg-warning "conversion from .int. to .char. changes value from .300. to .44." } */
> +  T t = 300; /* { dg-warning "conversion from .int. to .T. {aka .char.} changes value from .300. to .44." } */
> +  signed char sc = 300; /* { dg-warning "conversion from .int. to .signed char. changes value from .300. to .44." } */
> +  unsigned char uc = 300; /* { dg-warning "conversion from .int. to .unsigned char. changes value from .300. to .44." } */
> +  unsigned char uc2 = 300u; /* { dg-warning "conversion from .unsigned int. to .unsigned char. changes value from .300. to .44." } */
> +  char c2 = (double)1.0 + 200; /* { dg-warning "overflow in conversion from .double. to .char. changes value from .2.01e\\+2. to .127." } */
> +}
> 
> base-commit: b4e4b35f4ebe561826489bed971324efc99c5423


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-04-01 18:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-30 22:28 [PATCH] c-family: Tweak -Woverflow diagnostic Marek Polacek
2022-04-01 18:07 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).