Re: [PATCH] Warn for dangerous use of omitted middle operand in ?:

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, May 31, 2010 at 02:43:24PM +0200, Andi Kleen wrote:
> 
> Here's an updated patch.
> 
> -Andi

Some nitpicking:

> 2010-05-31  Andi Kleen  <ak@linux.intel.com>
> 
>         * gcc.dg/warn-omitted-condop.c: Add

Missing dot at the end of line.  Also note that testsuite/
has separate changelog, so these two lines shoudl go there.

>         * g++.dg/warn-omitted-condop.C: Add
>         * cp/parser.c: (cp_parser_question_colon_clause):
>         Switch to use cp_lexer_peek_token.

No colon after filename.  cp/ has separate changelog, so
cp/ path shouldn't appear and the entry should go into cp/ChangeLog.

>         Call warn_for_omitted_condop. Call pedwarn for omitted
>         middle operand.
>         * c-parser.c: (c_parser_conditional_expression)
>         Call warn_for_omitted_condop.
>         * c-common.c (warn_for_omitted_condop): Add.
>         * c-common.h (warn_for_omitted_condop): Add prototype.
>         * doc/invoke.texi: Document omitted condop warning.
> 
> diff --git a/gcc/c-common.c b/gcc/c-common.c
> index 115e302..fac42cd 100644
> --- a/gcc/c-common.c
> +++ b/gcc/c-common.c
> @@ -8588,6 +8588,20 @@ fold_offsetof (tree expr, tree stop_ref)
>    return convert (size_type_node, fold_offsetof_1 (expr, stop_ref));
>  }
>  
> +/* Warn for A ?: C expressions (with B omitted) where A is a boolean 
> +   expression, because B will always be true. */
> +
> +void
> +warn_for_omitted_condop (location_t location, tree cond) 
> +{ 
> +  if (truth_value_p (TREE_CODE (cond))) 
> +    {
> +      warning_at (location, OPT_Wparentheses, 
> +		"the omitted middle operand in ?: will always be %<true%>, "
> +		"suggest explicit middle operand");
> +    }    
> +} 

For single statement then/else bodies {} shouldn't be used around it.
Also there is trailing whitespace.

	Jakub