[2/5] c++: New warning option -Wstring-plus-int

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

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* c-family/c-common.h (maybe_warn_string_plus_int): new prototype
	* c-family/c-warn.h (maybe_warn_string_plus_int): new function
	* c-family/c-opt: new option -Wstring-plus-int
	* cp/call.c (build_new_op_1): Checking for -Wstring-plus-int

---
 gcc/c-family/c-common.h |  2 ++
 gcc/c-family/c-warn.c   | 34 ++++++++++++++++++++++++++++++++++
 gcc/c-family/c.opt      |  5 +++++
 gcc/cp/call.c           | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+)

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  <ryxi@stu.xidian.edu.cn>
> 
> 	* c-family/c-common.h (maybe_warn_string_plus_int): new prototype
> 	* c-family/c-warn.h (maybe_warn_string_plus_int): new function
> 	* c-family/c-opt: new option -Wstring-plus-int
> 	* cp/call.c (build_new_op_1): Checking for -Wstring-plus-int
> 
> ---
>   gcc/c-family/c-common.h |  2 ++
>   gcc/c-family/c-warn.c   | 34 ++++++++++++++++++++++++++++++++++
>   gcc/c-family/c.opt      |  5 +++++
>   gcc/cp/call.c           | 35 +++++++++++++++++++++++++++++++++++
>   4 files changed, 76 insertions(+)
> 
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 72fba56..8c748e9 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1503,6 +1503,8 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool,
>   				  bool);
>   extern void warn_for_omitted_condop (location_t, tree);
>   extern void warn_for_restrict (unsigned, vec<tree, va_gc> *);
> +extern void maybe_warn_string_plus_int (location_t, tree, tree,
> +                                        enum tree_code, enum tree_code);
>   
>   /* Places where an lvalue, or modifiable lvalue, may be required.
>      Used to select diagnostic messages in lvalue_error and
> diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> index 09c5760..0b038a7 100644
> --- a/gcc/c-family/c-warn.c
> +++ b/gcc/c-family/c-warn.c
> @@ -2290,3 +2290,37 @@ do_warn_duplicated_branches_r (tree *tp, int *, void *)
>       do_warn_duplicated_branches (*tp);
>     return NULL_TREE;
>   }
> +
> +static inline bool
> +char_type_cv_p (tree type)
Needs a comment describing what the function does.


> +{
> +  return char_type_p (build_qualified_type (type, TYPE_UNQUALIFIED));
> +}
Can you use TYPE_MAIN_VARIANT (type) here rather than building a new type?


> +
> +/* Warn about adding string literals/pointers and chars.
> +   Produce a warning when:
> +     1) arg1 is a string literal, and arg2 is a non-literal integer;
> +     2) arg1 is a string pointer, and arg2 is a character.  */
> +
> +void
> +maybe_warn_string_plus_int (location_t loc, tree type1, tree type2,
> +                            enum tree_code code1, enum tree_code code2)
> +{
> +  bool arg2_is_char = char_type_cv_p (type2);
> +  /* In C11, char16_t and char32_t are typedefs. Prevent false positives
> +     about uint_least16_t and uint_least32_t.  */
s/about/for/  I think

> +  if (type2 == uint_least16_type_node ||
> +  	  type2 == uint_least32_type_node)
> +  	arg2_is_char = false;
Formatting.  Operator goes on the next line.

> +
> +  if (code1 == STRING_CST && TREE_CODE (type2) == INTEGER_TYPE &&
> +      (code2 != INTEGER_CST || arg2_is_char))
Formatting.

> +    warning_at (loc, OPT_Wstring_plus_int,
> +                "adding %qT to string literal does not append to "
> +                "the string", type2);
> +  else if (TREE_CODE (type1) == POINTER_TYPE &&
> +  	       char_type_cv_p (TREE_TYPE (type1)) && arg2_is_char)
Formatting.


> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 93fae0d..95bda1b 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -5555,6 +5555,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
>     void *p;
>     bool strict_p;
>     bool any_viable_p;
> +  tree orig_arg1 = error_mark_node;
> +  tree orig_arg2 = error_mark_node;
>   
>     if (error_operand_p (arg1)
>         || error_operand_p (arg2)
> @@ -5605,8 +5607,20 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
>         code_orig_arg2 = TREE_CODE (TREE_TYPE (arg2));
>         break;
>   
> +    case PLUS_EXPR:
> +      /* These are saved for the sake of warn_string_plus_int.  */
> +      orig_arg1 = arg1;
> +      orig_arg2 = arg2;
> +      break;
> +
>         /* =, ->, [], () must be non-static member functions.  */
>       case MODIFY_EXPR:
> +      if (code2 == PLUS_EXPR)
> +        {
> +      	  /* Like PLUS_EXPR.  */
> +      	  orig_arg1 = arg1;
> +	  orig_arg2 = arg2;
> +        }
>         if (code2 != NOP_EXPR)
>   	break;
>         /* FALLTHRU */
Formatting problems should be obvious :-)

I think Jason probably needs to chime in on this patch.  I'm not keen on 
the way we stash away the arguments into orig_argX.  Perhaps he's got a 
better suggestion.

Jeff

On 2017-04-28 14:34 -0600, Jeff Law wrote:

> Can you use TYPE_MAIN_VARIANT (type) here rather than building a new type?

I'll check the macros.

> Formatting problems should be obvious :-)

I'll follow the GCC formatting rules.

> I think Jason probably needs to chime in on this patch.  I'm not keen on 
> the way we stash away the arguments into orig_argX.  Perhaps he's got a 
> better suggestion.

In the recent version of Clang, this warning options has been seperated
into two options -Wstring-plus-int and -Wstring-plus-char.  Maybe I should
do the same.