Re: [PATCH 3/5] c: New warning option -Wstring-plus-int

[Jeff Law]

On 02/22/2017 04:01 AM, Xi Ruoyao wrote:

This patch adds warning option -Wstring-plus-int for C.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* c/c-typeck.c (parser_build_binary_op, build_modify_expr): checking 
for -Wstring-plus-int

---
  gcc/c/c-typeck.c | 28 
  1 file changed, 28 insertions(+)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 49b99b5..288d44b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3631,6 +3631,24 @@ parser_build_binary_op (location_t location, enum 
tree_code code,
  ? arg2.original_type
  : TREE_TYPE (arg2.value));
  
+  /* Warn about adding string literals/pointers and chars.  */

+  if (warn_string_plus_int && (TREE_CODE (type1) == POINTER_TYPE ||
+   TREE_CODE (type2) == POINTER_TYPE))
Please use POINTER_TYPE_P rather than checking the underlying tree code. 
 This is also formatted incorrectly.  It should look like this


  if (warn_string_plus_int
  && (POINTER_TYPE_P (type1) || POINTER_TYPE_P (type2))



+  enum tree_code code_ptr = (TREE_CODE (type1) == POINTER_TYPE ?
+ code1 : code2);
+  tree type_ptr = (TREE_CODE (type1) == POINTER_TYPE ?
+   type1 : type2);
+  tree type_int = (TREE_CODE (type1) == POINTER_TYPE ?
+   type2 : type1) > +  enum tree_code code_int = 
TREE_CODE (TREE_CODE (type1)
+   == POINTER_TYPE ?
+   arg2.value : arg1.value);
Similarly, please use POINTER_TYPE_P.  That will likely eliminate the 
need to warp the first 3 assigments at all.  The last should be line 
wrapped like this:


  enum tree_code code_int =  (POINTER_TYPE_P (type1)
  ? arg2.value
  : arg1.value);

Note how we bring the operator down to the new line rather than keep it 
at the end of a long line.


Please also verify that instead of 8 spaces that you use tabs.  I think 
the assignments above are using 8 spaces rather than tabs.



@@ -5768,6 +5786,16 @@ build_modify_expr (location_t location, tree lhs, tree 
lhs_origtype,
  rhseval = newrhs;
}
  
+  /* Warn about adding string literals/pointers and chars.  */

+ if (modifycode == PLUS_EXPR && warn_string_plus_int)
+   {
+ tree lhs_type1 = (lhs_origtype ? lhs_origtype :
+  TREE_TYPE (lhs));
+ tree rhs_type1 = (rhs_origtype ? rhs_origtype :
+  TREE_TYPE (rhs));
+ maybe_warn_string_plus_int (location, lhs_type1, rhs_type1,
+ TREE_CODE (lhs), TREE_CODE (rhs));
+   }

Formatting fixes here too.  See examples above for proper formatting.

Since you're relatively new to GCC development, please make the changes 
noted above and resubmit this patch.


jeff

[PATCH 3/5] c: New warning option -Wstring-plus-int

[Xi Ruoyao]

This patch adds warning option -Wstring-plus-int for C.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* c/c-typeck.c (parser_build_binary_op, build_modify_expr): checking 
for -Wstring-plus-int

---
 gcc/c/c-typeck.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 49b99b5..288d44b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3631,6 +3631,24 @@ parser_build_binary_op (location_t location, enum 
tree_code code,
 ? arg2.original_type
 : TREE_TYPE (arg2.value));
 
+  /* Warn about adding string literals/pointers and chars.  */
+  if (warn_string_plus_int && (TREE_CODE (type1) == POINTER_TYPE ||
+   TREE_CODE (type2) == POINTER_TYPE))
+{
+  enum tree_code code_ptr = (TREE_CODE (type1) == POINTER_TYPE ?
+ code1 : code2);
+  tree type_ptr = (TREE_CODE (type1) == POINTER_TYPE ?
+   type1 : type2);
+  tree type_int = (TREE_CODE (type1) == POINTER_TYPE ?
+   type2 : type1);
+  enum tree_code code_int = TREE_CODE (TREE_CODE (type1)
+   == POINTER_TYPE ?
+   arg2.value : arg1.value);
+
+     maybe_warn_string_plus_int (location, type_ptr, type_int,
+     code_ptr, code_int);
+}
+
   result.value = build_binary_op (location, code,
      arg1.value, arg2.value, 1);
   result.original_code = code;
@@ -5768,6 +5786,16 @@ build_modify_expr (location_t location, tree lhs, tree 
lhs_origtype,
      rhseval = newrhs;
    }
 
+  /* Warn about adding string literals/pointers and chars.  */
+     if (modifycode == PLUS_EXPR && warn_string_plus_int)
+   {
+     tree lhs_type1 = (lhs_origtype ? lhs_origtype :
+      TREE_TYPE (lhs));
+     tree rhs_type1 = (rhs_origtype ? rhs_origtype :
+      TREE_TYPE (rhs));
+     maybe_warn_string_plus_int (location, lhs_type1, rhs_type1,
+     TREE_CODE (lhs), TREE_CODE (rhs));
+   }
      newrhs = build_binary_op (location,
    modifycode, lhs, newrhs, 1);


-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University