[C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)

Marek Polacek polacek@redhat.com
Wed Sep 30 15:24:00 GMT 2015


Ping.

On Fri, Sep 18, 2015 at 03:44:34PM +0200, Marek Polacek wrote:
> On Fri, Sep 18, 2015 at 12:06:06PM +0200, Marek Polacek wrote:
> > > Since we don't know bar's side-effects we must assume they change
> > > the value of a and so we must avoid diagnosing the third if.
> > 
> > Ok, I'm convinced now.  We have something similar in the codebase:
> > libsupc++/eh_catch.cc has
> > 
> >   int count = header->handlerCount;
> >   if (count < 0)
> >     {   
> >       // This exception was rethrown.  Decrement the (inverted) catch
> >       // count and remove it from the chain when it reaches zero.
> >       if (++count == 0)
> >         globals->caughtExceptions = header->nextException;
> >     }   
> >   else if (--count == 0)
> >     {   
> >       // Handling for this exception is complete.  Destroy the object.
> >       globals->caughtExceptions = header->nextException;
> >       _Unwind_DeleteException (&header->unwindHeader);
> >       return;
> >     }   
> >   else if (count < 0)
> >     // A bug in the exception handling library or compiler.
> >     std::terminate ();
> > 
> > Here all arms are reachable.  I guess I need to kill the chain of conditions
> > when we find something with side-effects, exactly as you suggested.
> 
> Done in the below.  This version actually bootstraps, because I've added
> -Wno-duplicated-cond for insn-dfatab.o and insn-latencytab.o (don't know
> how to fix these) + I've tweaked a condition in genemit.c.  The problem
> here is that we have
> 
>       if (INTVAL (x) == 0)
>         printf ("const0_rtx");
>       else if (INTVAL (x) == 1)
>         printf ("const1_rtx");
>       else if (INTVAL (x) == -1) 
>         printf ("constm1_rtx");
>       // ...
>       else if (INTVAL (x) == STORE_FLAG_VALUE)
>         printf ("const_true_rtx");
> 
> and STORE_FLAG_VALUE happens to be 1, so we have two same conditions.
> STORE_FLAG_VALUE is 1 or -1, but according to the documentation it can
> also be some other number so we should keep this if statement.  I've
> avoided the warning by adding STORE_FLAG_VALUE > 1 check.
> 
> How does this look like now?
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2015-09-18  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c/64249
> 	* c-common.c (warn_duplicated_cond_add_or_warn): New function.
> 	* c-common.h (warn_duplicated_cond_add_or_warn): Declare.
> 	* c.opt (Wduplicated-cond): New option.
> 
> 	* c-parser.c (c_parser_statement_after_labels): Add CHAIN parameter
> 	and pass it down to c_parser_if_statement.
> 	(c_parser_else_body): Add CHAIN parameter and pass it down to
> 	c_parser_statement_after_labels.
> 	(c_parser_if_statement): Add CHAIN parameter.  Add code to warn about
> 	duplicated if-else-if conditions.
> 
> 	* parser.c (cp_parser_statement): Add CHAIN parameter and pass it
> 	down to cp_parser_selection_statement.
> 	(cp_parser_selection_statement): Add CHAIN parameter.  Add code to
> 	warn about duplicated if-else-if conditions.
> 	(cp_parser_implicitly_scoped_statement): Add CHAIN parameter and pass
> 	it down to cp_parser_statement.
> 
> 	* doc/invoke.texi: Document -Wduplicated-cond.
> 	* Makefile.in (insn-latencytab.o): Use -Wno-duplicated-cond.
> 	(insn-dfatab.o): Likewise.
> 	* genemit.c (gen_exp): Rewrite condition to avoid -Wduplicated-cond
> 	warning.
> 
> 	* c-c++-common/Wduplicated-cond-1.c: New test.
> 	* c-c++-common/Wduplicated-cond-2.c: New test.
> 	* c-c++-common/Wduplicated-cond-3.c: New test.
> 	* c-c++-common/Wduplicated-cond-4.c: New test.
> 	* c-c++-common/Wmisleading-indentation.c (fn_37): Avoid
> 	-Wduplicated-cond warning.
> 
> diff --git gcc/Makefile.in gcc/Makefile.in
> index c2df21d..d7caa76 100644
> --- gcc/Makefile.in
> +++ gcc/Makefile.in
> @@ -217,6 +217,8 @@ libgcov-merge-tool.o-warn = -Wno-error
>  gimple-match.o-warn = -Wno-unused
>  generic-match.o-warn = -Wno-unused
>  dfp.o-warn = -Wno-strict-aliasing
> +insn-latencytab.o-warn = -Wno-duplicated-cond
> +insn-dfatab.o-warn = -Wno-duplicated-cond
>  
>  # All warnings have to be shut off in stage1 if the compiler used then
>  # isn't gcc; configure determines that.  WARN_CFLAGS will be either
> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
> index 4b922bf..8991215 100644
> --- gcc/c-family/c-common.c
> +++ gcc/c-family/c-common.c
> @@ -12919,4 +12919,45 @@ reject_gcc_builtin (const_tree expr, location_t loc /* = UNKNOWN_LOCATION */)
>    return false;
>  }
>  
> +/* If we're creating an if-else-if condition chain, first see if we
> +   already have this COND in the CHAIN.  If so, warn and don't add COND
> +   into the vector, otherwise add the COND there.  LOC is the location
> +   of COND.  */
> +
> +void
> +warn_duplicated_cond_add_or_warn (location_t loc, tree cond, vec<tree> **chain)
> +{
> +  /* No chain has been created yet.  Do nothing.  */
> +  if (*chain == NULL)
> +    return;
> +
> +  if (TREE_SIDE_EFFECTS (cond))
> +    {
> +      /* Uh-oh!  This condition has a side-effect, thus invalidates
> +	 the whole chain.  */
> +      delete *chain;
> +      *chain = NULL;
> +      return;
> +    }
> +
> +  unsigned int ix;
> +  tree t;
> +  bool found = false;
> +  FOR_EACH_VEC_ELT (**chain, ix, t)
> +    if (operand_equal_p (cond, t, 0))
> +      {
> +	if (warning_at (loc, OPT_Wduplicated_cond,
> +			"duplicated %<if%> condition"))
> +	  inform (EXPR_LOCATION (t), "previously used here");
> +	found = true;
> +	break;
> +      }
> +
> +  if (!found
> +      && !CONSTANT_CLASS_P (cond)
> +      /* Don't infinitely grow the chain.  */
> +      && (*chain)->length () < 512)
> +    (*chain)->safe_push (cond);
> +}
> +
>  #include "gt-c-family-c-common.h"
> diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
> index 74d1bc1..7957df5 100644
> --- gcc/c-family/c-common.h
> +++ gcc/c-family/c-common.h
> @@ -1441,5 +1441,6 @@ extern tree cilk_for_number_of_iterations (tree);
>  extern bool check_no_cilk (tree, const char *, const char *,
>  		           location_t loc = UNKNOWN_LOCATION);
>  extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);
> +extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec<tree> **);
>  
>  #endif /* ! GCC_C_COMMON_H */
> diff --git gcc/c-family/c.opt gcc/c-family/c.opt
> index 47ba070..f318044 100644
> --- gcc/c-family/c.opt
> +++ gcc/c-family/c.opt
> @@ -406,6 +406,10 @@ Wdiv-by-zero
>  C ObjC C++ ObjC++ Var(warn_div_by_zero) Init(1) Warning
>  Warn about compile-time integer division by zero
>  
> +Wduplicated-cond
> +C ObjC C++ ObjC++ Var(warn_duplicated_cond) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
> +Warn about duplicated conditions in an if-else-if chain
> +
>  Weffc++
>  C++ ObjC++ Var(warn_ecpp) Warning
>  Warn about violations of Effective C++ style rules
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index d5de102..9468aff 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -1198,8 +1198,8 @@ static tree c_parser_compound_statement (c_parser *);
>  static void c_parser_compound_statement_nostart (c_parser *);
>  static void c_parser_label (c_parser *);
>  static void c_parser_statement (c_parser *);
> -static void c_parser_statement_after_labels (c_parser *);
> -static void c_parser_if_statement (c_parser *);
> +static void c_parser_statement_after_labels (c_parser *, vec<tree> * = NULL);
> +static void c_parser_if_statement (c_parser *, vec<tree> *);
>  static void c_parser_switch_statement (c_parser *);
>  static void c_parser_while_statement (c_parser *, bool);
>  static void c_parser_do_statement (c_parser *, bool);
> @@ -4961,10 +4961,11 @@ c_parser_statement (c_parser *parser)
>    c_parser_statement_after_labels (parser);
>  }
>  
> -/* Parse a statement, other than a labeled statement.  */
> +/* Parse a statement, other than a labeled statement.  CHAIN is a vector
> +   of if-else-if conditions.  */
>  
>  static void
> -c_parser_statement_after_labels (c_parser *parser)
> +c_parser_statement_after_labels (c_parser *parser, vec<tree> *chain)
>  {
>    location_t loc = c_parser_peek_token (parser)->location;
>    tree stmt = NULL_TREE;
> @@ -4979,7 +4980,7 @@ c_parser_statement_after_labels (c_parser *parser)
>        switch (c_parser_peek_token (parser)->keyword)
>  	{
>  	case RID_IF:
> -	  c_parser_if_statement (parser);
> +	  c_parser_if_statement (parser, chain);
>  	  break;
>  	case RID_SWITCH:
>  	  c_parser_switch_statement (parser);
> @@ -5230,10 +5231,12 @@ c_parser_if_body (c_parser *parser, bool *if_p,
>  
>  /* Parse the else body of an if statement.  This is just parsing a
>     statement but (a) it is a block in C99, (b) we handle an empty body
> -   specially for the sake of -Wempty-body warnings.  */
> +   specially for the sake of -Wempty-body warnings.  CHAIN is a vector
> +   of if-else-if conditions.  */
>  
>  static tree
> -c_parser_else_body (c_parser *parser, const token_indent_info &else_tinfo)
> +c_parser_else_body (c_parser *parser, const token_indent_info &else_tinfo,
> +		    vec<tree> *chain)
>  {
>    location_t body_loc = c_parser_peek_token (parser)->location;
>    tree block = c_begin_compound_stmt (flag_isoc99);
> @@ -5251,7 +5254,7 @@ c_parser_else_body (c_parser *parser, const token_indent_info &else_tinfo)
>        c_parser_consume_token (parser);
>      }
>    else
> -    c_parser_statement_after_labels (parser);
> +    c_parser_statement_after_labels (parser, chain);
>  
>    token_indent_info next_tinfo
>      = get_token_indent_info (c_parser_peek_token (parser));
> @@ -5265,10 +5268,11 @@ c_parser_else_body (c_parser *parser, const token_indent_info &else_tinfo)
>     if-statement:
>       if ( expression ) statement
>       if ( expression ) statement else statement
> -*/
> +
> +  CHAIN is a vector of if-else-if conditions.  */
>  
>  static void
> -c_parser_if_statement (c_parser *parser)
> +c_parser_if_statement (c_parser *parser, vec<tree> *chain)
>  {
>    tree block;
>    location_t loc;
> @@ -5294,15 +5298,47 @@ c_parser_if_statement (c_parser *parser)
>    parser->in_if_block = true;
>    first_body = c_parser_if_body (parser, &first_if, if_tinfo);
>    parser->in_if_block = in_if_block;
> +
> +  if (warn_duplicated_cond)
> +    warn_duplicated_cond_add_or_warn (EXPR_LOCATION (cond), cond, &chain);
> +
>    if (c_parser_next_token_is_keyword (parser, RID_ELSE))
>      {
>        token_indent_info else_tinfo
>  	= get_token_indent_info (c_parser_peek_token (parser));
>        c_parser_consume_token (parser);
> -      second_body = c_parser_else_body (parser, else_tinfo);
> +      if (warn_duplicated_cond)
> +	{
> +	  if (c_parser_next_token_is_keyword (parser, RID_IF)
> +	      && chain == NULL)
> +	    {
> +	      /* We've got "if (COND) else if (COND2)".  Start the
> +		 condition chain and add COND as the first element.  */
> +	      chain = new vec<tree> ();
> +	      if (!CONSTANT_CLASS_P (cond) && !TREE_SIDE_EFFECTS (cond))
> +		chain->safe_push (cond);
> +	    }
> +	  else if (!c_parser_next_token_is_keyword (parser, RID_IF))
> +	    {
> +	      /* This is if-else without subsequent if.  Zap the condition
> +		 chain; we would have already warned at this point.  */
> +	      delete chain;
> +	      chain = NULL;
> +	    }
> +	}
> +      second_body = c_parser_else_body (parser, else_tinfo, chain);
>      }
>    else
> -    second_body = NULL_TREE;
> +    {
> +      second_body = NULL_TREE;
> +      if (warn_duplicated_cond)
> +	{
> +	  /* This if statement does not have an else clause.  We don't
> +	     need the condition chain anymore.  */
> +	  delete chain;
> +	  chain = NULL;
> +	}
> +    }
>    c_finish_if_stmt (loc, cond, first_body, second_body, first_if);
>    if_stmt = c_end_compound_stmt (loc, block, flag_isoc99);
>  
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 4f424b6..c82a5c0 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -2023,7 +2023,7 @@ static void cp_parser_lambda_body
>  /* Statements [gram.stmt.stmt]  */
>  
>  static void cp_parser_statement
> -  (cp_parser *, tree, bool, bool *);
> +  (cp_parser *, tree, bool, bool *, vec<tree> * = NULL);
>  static void cp_parser_label_for_labeled_statement
>  (cp_parser *, tree);
>  static tree cp_parser_expression_statement
> @@ -2033,7 +2033,7 @@ static tree cp_parser_compound_statement
>  static void cp_parser_statement_seq_opt
>    (cp_parser *, tree);
>  static tree cp_parser_selection_statement
> -  (cp_parser *, bool *);
> +  (cp_parser *, bool *, vec<tree> *);
>  static tree cp_parser_condition
>    (cp_parser *);
>  static tree cp_parser_iteration_statement
> @@ -2058,7 +2058,7 @@ static void cp_parser_declaration_statement
>    (cp_parser *);
>  
>  static tree cp_parser_implicitly_scoped_statement
> -  (cp_parser *, bool *, const token_indent_info &);
> +  (cp_parser *, bool *, const token_indent_info &, vec<tree> * = NULL);
>  static void cp_parser_already_scoped_statement
>    (cp_parser *, const token_indent_info &);
>  
> @@ -9923,11 +9923,13 @@ cp_parser_lambda_body (cp_parser* parser, tree lambda_expr)
>  
>    If IF_P is not NULL, *IF_P is set to indicate whether the statement
>    is a (possibly labeled) if statement which is not enclosed in braces
> -  and has an else clause.  This is used to implement -Wparentheses.  */
> +  and has an else clause.  This is used to implement -Wparentheses.
> +
> +  CHAIN is a vector of if-else-if conditions.  */
>  
>  static void
>  cp_parser_statement (cp_parser* parser, tree in_statement_expr,
> -		     bool in_compound, bool *if_p)
> +		     bool in_compound, bool *if_p, vec<tree> *chain)
>  {
>    tree statement, std_attrs = NULL_TREE;
>    cp_token *token;
> @@ -9975,7 +9977,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>  
>  	case RID_IF:
>  	case RID_SWITCH:
> -	  statement = cp_parser_selection_statement (parser, if_p);
> +	  statement = cp_parser_selection_statement (parser, if_p, chain);
>  	  break;
>  
>  	case RID_WHILE:
> @@ -10404,10 +10406,14 @@ cp_parser_statement_seq_opt (cp_parser* parser, tree in_statement_expr)
>     If IF_P is not NULL, *IF_P is set to indicate whether the statement
>     is a (possibly labeled) if statement which is not enclosed in
>     braces and has an else clause.  This is used to implement
> -   -Wparentheses.  */
> +   -Wparentheses.
> +
> +   CHAIN is a vector of if-else-if conditions.  This is used to implement
> +   -Wduplicated-cond.  */
>  
>  static tree
> -cp_parser_selection_statement (cp_parser* parser, bool *if_p)
> +cp_parser_selection_statement (cp_parser* parser, bool *if_p,
> +			       vec<tree> *chain)
>  {
>    cp_token *token;
>    enum rid keyword;
> @@ -10458,6 +10464,10 @@ cp_parser_selection_statement (cp_parser* parser, bool *if_p)
>  	    /* Add the condition.  */
>  	    finish_if_stmt_cond (condition, statement);
>  
> +	    if (warn_duplicated_cond)
> +	      warn_duplicated_cond_add_or_warn (token->location, condition,
> +						&chain);
> +
>  	    /* Parse the then-clause.  */
>  	    in_statement = parser->in_statement;
>  	    parser->in_statement |= IN_IF_STMT;
> @@ -10475,10 +10485,41 @@ cp_parser_selection_statement (cp_parser* parser, bool *if_p)
>  		  = get_token_indent_info (cp_lexer_peek_token (parser->lexer));
>  		/* Consume the `else' keyword.  */
>  		cp_lexer_consume_token (parser->lexer);
> +		if (warn_duplicated_cond)
> +		  {
> +		    if (cp_lexer_next_token_is_keyword (parser->lexer,
> +							RID_IF)
> +			&& chain == NULL)
> +		      {
> +			/* We've got "if (COND) else if (COND2)".  Start
> +			   the condition chain and add COND as the first
> +			   element.  */
> +			chain = new vec<tree> ();
> +			if (!CONSTANT_CLASS_P (condition)
> +			    && !TREE_SIDE_EFFECTS (condition))
> +			{
> +			  /* Wrap it in a NOP_EXPR so that we can set the
> +			     location of the condition.  */
> +			  tree e = build1 (NOP_EXPR, TREE_TYPE (condition),
> +					   condition);
> +			  SET_EXPR_LOCATION (e, token->location);
> +			  chain->safe_push (e);
> +			}
> +		      }
> +		    else if (!cp_lexer_next_token_is_keyword (parser->lexer,
> +							      RID_IF))
> +		      {
> +			/* This is if-else without subsequent if.  Zap the
> +			   condition chain; we would have already warned at
> +			   this point.  */
> +			delete chain;
> +			chain = NULL;
> +		      }
> +		  }
>  		begin_else_clause (statement);
>  		/* Parse the else-clause.  */
>  		cp_parser_implicitly_scoped_statement (parser, NULL,
> -						       guard_tinfo);
> +						       guard_tinfo, chain);
>  
>  		finish_else_clause (statement);
>  
> @@ -10500,6 +10541,12 @@ cp_parser_selection_statement (cp_parser* parser, bool *if_p)
>  		  warning_at (EXPR_LOCATION (statement), OPT_Wparentheses,
>  			      "suggest explicit braces to avoid ambiguous"
>  			      " %<else%>");
> +		if (warn_duplicated_cond)
> +		  {
> +		    /* We don't need the condition chain anymore.  */
> +		    delete chain;
> +		    chain = NULL;
> +		  }
>  	      }
>  
>  	    /* Now we're all done with the if-statement.  */
> @@ -11410,11 +11457,15 @@ cp_parser_declaration_statement (cp_parser* parser)
>     braces and has an else clause.  This is used to implement
>     -Wparentheses.
>  
> +   CHAIN is a vector of if-else-if conditions.  This is used to implement
> +   -Wduplicated-cond.
> +
>     Returns the new statement.  */
>  
>  static tree
>  cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
> -				       const token_indent_info &guard_tinfo)
> +				       const token_indent_info &guard_tinfo,
> +				       vec<tree> *chain)
>  {
>    tree statement;
>    location_t body_loc = cp_lexer_peek_token (parser->lexer)->location;
> @@ -11447,7 +11498,7 @@ cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
>        /* Create a compound-statement.  */
>        statement = begin_compound_stmt (0);
>        /* Parse the dependent-statement.  */
> -      cp_parser_statement (parser, NULL_TREE, false, if_p);
> +      cp_parser_statement (parser, NULL_TREE, false, if_p, chain);
>        /* Finish the dummy compound-statement.  */
>        finish_compound_stmt (statement);
>      }
> diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> index 547ee2d..d6a4973 100644
> --- gcc/doc/invoke.texi
> +++ gcc/doc/invoke.texi
> @@ -241,7 +241,7 @@ Objective-C and Objective-C++ Dialects}.
>  -pedantic-errors @gol
>  -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
>  -Waggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
> --Wbool-compare -Wframe-address @gol
> +-Wbool-compare -Wduplicated-cond -Wframe-address @gol
>  -Wno-attributes -Wno-builtin-macro-redefined @gol
>  -Wc90-c99-compat -Wc99-c11-compat @gol
>  -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
> @@ -3458,6 +3458,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
>  -Wimplicit-int @r{(C and Objective-C only)} @gol
>  -Wimplicit-function-declaration @r{(C and Objective-C only)} @gol
>  -Wbool-compare  @gol
> +-Wduplicated-cond  @gol
>  -Wcomment  @gol
>  -Wformat   @gol
>  -Wmain @r{(only for C/ObjC and unless} @option{-ffreestanding}@r{)}  @gol
> @@ -4489,6 +4490,17 @@ if ((n > 1) == 2) @{ @dots{} @}
>  @end smallexample
>  This warning is enabled by @option{-Wall}.
>  
> +@item -Wduplicated-cond
> +@opindex Wno-duplicated-cond
> +@opindex Wduplicated-cond
> +Warn about duplicated conditions in an if-else-if chain.  For instance,
> +warn for the following code:
> +@smallexample
> +if (p->q != NULL) @{ @dots{} @}
> +else if (p->q != NULL) @{ @dots{} @}
> +@end smallexample
> +This warning is enabled by @option{-Wall}.
> +
>  @item -Wframe-address
>  @opindex Wno-frame-address
>  @opindex Wframe-address
> diff --git gcc/genemit.c gcc/genemit.c
> index a2c474d..13f9119 100644
> --- gcc/genemit.c
> +++ gcc/genemit.c
> @@ -179,10 +179,10 @@ gen_exp (rtx x, enum rtx_code subroutine_type, char *used)
>        else if (INTVAL (x) == -1)
>  	printf ("constm1_rtx");
>        else if (-MAX_SAVED_CONST_INT <= INTVAL (x)
> -	  && INTVAL (x) <= MAX_SAVED_CONST_INT)
> +	       && INTVAL (x) <= MAX_SAVED_CONST_INT)
>  	printf ("const_int_rtx[MAX_SAVED_CONST_INT + (%d)]",
>  		(int) INTVAL (x));
> -      else if (INTVAL (x) == STORE_FLAG_VALUE)
> +      else if (STORE_FLAG_VALUE > 1 && INTVAL (x) == STORE_FLAG_VALUE)
>  	printf ("const_true_rtx");
>        else
>  	{
> diff --git gcc/testsuite/c-c++-common/Wduplicated-cond-1.c gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
> index e69de29..4763a84 100644
> --- gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
> +++ gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
> @@ -0,0 +1,200 @@
> +/* PR c/64249 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wduplicated-cond" } */
> +
> +#ifndef __cplusplus
> +# define bool _Bool
> +# define true 1
> +# define false 0
> +#endif
> +
> +extern int foo (void);
> +
> +int
> +fn1 (int n)
> +{
> +  if (n == 1) /* { dg-message "previously used here" } */
> +    return -1;
> +  else if (n == 2)
> +    return 0;
> +  else if (n == 1) /* { dg-warning "duplicated .if. condition" } */
> +    return 1;
> +  return 0;
> +}
> +
> +int
> +fn2 (void)
> +{
> +  if (4)
> +    return 1;
> +  else if (4)
> +    return 2;
> +
> +#define N 10
> +  if (N)
> +    return 3;
> +  else if (N)
> +    return 4;
> +}
> +
> +int
> +fn3 (int n)
> +{
> +  if (n == 42)
> +    return 1;
> +  if (n == 42)
> +    return 2;
> +
> +  if (n)
> +    if (n)
> +      if (n)
> +	if (n)
> +	  return 42;
> +
> +  if (!n)
> +    return 10;
> +  else
> +    return 11;
> +}
> +
> +int
> +fn4 (int n)
> +{
> +  if (n > 0)
> +    {
> +      if (n == 1) /* { dg-message "previously used here" } */
> +	return 1;
> +      else if (n == 1) /* { dg-warning "duplicated .if. condition" } */
> +	return 2;
> +    }
> +  else if (n < 0)
> +    {
> +      if (n < -1)
> +	return 6;
> +      else if (n < -2)
> +	{
> +	  if (n == -10) /* { dg-message "previously used here" } */
> +	    return 3;
> +	  else if (n == -10) /* { dg-warning "duplicated .if. condition" } */
> +	    return 4;
> +	}
> +    }
> +  else
> +    return 7;
> +  return 0;
> +}
> +
> +struct S { long p, q; };
> +
> +int
> +fn5 (struct S *s)
> +{
> +  if (!s->p) /* { dg-message "previously used here" } */
> +    return 12345;
> +  else if (!s->p) /* { dg-warning "duplicated .if. condition" } */
> +    return 1234;
> +  return 0;
> +}
> +
> +int
> +fn6 (int n)
> +{
> +  if (n) /* { dg-message "previously used here" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  return 0;
> +}
> +
> +int
> +fn7 (int n)
> +{
> +  if (n == 0) /* { dg-message "previously used here" } */
> +    return 10;
> +  else if (n == 1) /* { dg-message "previously used here" } */
> +    return 11;
> +  else if (n == 2) /* { dg-message "previously used here" } */
> +    return 12;
> +  else if (n == 3) /* { dg-message "previously used here" } */
> +    return 13;
> +  else if (n == 4) /* { dg-message "previously used here" } */
> +    return 14;
> +  else if (n == 5) /* { dg-message "previously used here" } */
> +    return 15;
> +  else if (n == 6) /* { dg-message "previously used here" } */
> +    return 16;
> +  else if (n == 7) /* { dg-message "previously used here" } */
> +    return 17;
> +  else if (n == 0) /* { dg-warning "duplicated .if. condition" } */
> +    return 100;
> +  else if (n == 1) /* { dg-warning "duplicated .if. condition" } */
> +    return 101;
> +  else if (n == 2) /* { dg-warning "duplicated .if. condition" } */
> +    return 102;
> +  else if (n == 3) /* { dg-warning "duplicated .if. condition" } */
> +    return 103;
> +  else if (n == 4) /* { dg-warning "duplicated .if. condition" } */
> +    return 104;
> +  else if (n == 5) /* { dg-warning "duplicated .if. condition" } */
> +    return 105;
> +  else if (n == 6) /* { dg-warning "duplicated .if. condition" } */
> +    return 106;
> +  else if (n == 7) /* { dg-warning "duplicated .if. condition" } */
> +    return 107;
> +  return 0;
> +}
> +
> +int
> +fn8 (bool b)
> +{
> +  if (!b) /* { dg-message "previously used here" } */
> +    return 16;
> +  else if (!b) /* { dg-warning "duplicated .if. condition" } */
> +    return 27;
> +  else
> +    return 64;
> +}
> +
> +int
> +fn9 (int i, int j, int k)
> +{
> +  if (i > 0 && j > 0 && k > 0) /* { dg-message "previously used here" } */
> +    return -999;
> +  else
> +  if (i > 0 && j > 0 && k > 0) /* { dg-warning "duplicated .if. condition" } */
> +    return 999;
> +  else
> +    return 0;
> +}
> +
> +int
> +fn10 (void)
> +{
> +  if (foo ())
> +    return 1732984;
> +  else if (foo ())
> +    return 18409;
> +  return 0;
> +}
> +
> +int
> +fn11 (int n)
> +{
> +  if (++n == 10)
> +    return 666;
> +  else if (++n == 10)
> +    return 9;
> +  return 0;
> +}
> diff --git gcc/testsuite/c-c++-common/Wduplicated-cond-2.c gcc/testsuite/c-c++-common/Wduplicated-cond-2.c
> index e69de29..90a8663 100644
> --- gcc/testsuite/c-c++-common/Wduplicated-cond-2.c
> +++ gcc/testsuite/c-c++-common/Wduplicated-cond-2.c
> @@ -0,0 +1,200 @@
> +/* PR c/64249 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall" } */
> +
> +#ifndef __cplusplus
> +# define bool _Bool
> +# define true 1
> +# define false 0
> +#endif
> +
> +extern int foo (void);
> +
> +int
> +fn1 (int n)
> +{
> +  if (n == 1) /* { dg-message "previously used here" } */
> +    return -1;
> +  else if (n == 2)
> +    return 0;
> +  else if (n == 1) /* { dg-warning "duplicated .if. condition" } */
> +    return 1;
> +  return 0;
> +}
> +
> +int
> +fn2 (void)
> +{
> +  if (4)
> +    return 1;
> +  else if (4)
> +    return 2;
> +
> +#define N 10
> +  if (N)
> +    return 3;
> +  else if (N)
> +    return 4;
> +}
> +
> +int
> +fn3 (int n)
> +{
> +  if (n == 42)
> +    return 1;
> +  if (n == 42)
> +    return 2;
> +
> +  if (n)
> +    if (n)
> +      if (n)
> +	if (n)
> +	  return 42;
> +
> +  if (!n)
> +    return 10;
> +  else
> +    return 11;
> +}
> +
> +int
> +fn4 (int n)
> +{
> +  if (n > 0)
> +    {
> +      if (n == 1) /* { dg-message "previously used here" } */
> +	return 1;
> +      else if (n == 1) /* { dg-warning "duplicated .if. condition" } */
> +	return 2;
> +    }
> +  else if (n < 0)
> +    {
> +      if (n < -1)
> +	return 6;
> +      else if (n < -2)
> +	{
> +	  if (n == -10) /* { dg-message "previously used here" } */
> +	    return 3;
> +	  else if (n == -10) /* { dg-warning "duplicated .if. condition" } */
> +	    return 4;
> +	}
> +    }
> +  else
> +    return 7;
> +  return 0;
> +}
> +
> +struct S { long p, q; };
> +
> +int
> +fn5 (struct S *s)
> +{
> +  if (!s->p) /* { dg-message "previously used here" } */
> +    return 12345;
> +  else if (!s->p) /* { dg-warning "duplicated .if. condition" } */
> +    return 1234;
> +  return 0;
> +}
> +
> +int
> +fn6 (int n)
> +{
> +  if (n) /* { dg-message "previously used here" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  else if (n) /* { dg-warning "duplicated .if. condition" } */
> +    return n;
> +  return 0;
> +}
> +
> +int
> +fn7 (int n)
> +{
> +  if (n == 0) /* { dg-message "previously used here" } */
> +    return 10;
> +  else if (n == 1) /* { dg-message "previously used here" } */
> +    return 11;
> +  else if (n == 2) /* { dg-message "previously used here" } */
> +    return 12;
> +  else if (n == 3) /* { dg-message "previously used here" } */
> +    return 13;
> +  else if (n == 4) /* { dg-message "previously used here" } */
> +    return 14;
> +  else if (n == 5) /* { dg-message "previously used here" } */
> +    return 15;
> +  else if (n == 6) /* { dg-message "previously used here" } */
> +    return 16;
> +  else if (n == 7) /* { dg-message "previously used here" } */
> +    return 17;
> +  else if (n == 0) /* { dg-warning "duplicated .if. condition" } */
> +    return 100;
> +  else if (n == 1) /* { dg-warning "duplicated .if. condition" } */
> +    return 101;
> +  else if (n == 2) /* { dg-warning "duplicated .if. condition" } */
> +    return 102;
> +  else if (n == 3) /* { dg-warning "duplicated .if. condition" } */
> +    return 103;
> +  else if (n == 4) /* { dg-warning "duplicated .if. condition" } */
> +    return 104;
> +  else if (n == 5) /* { dg-warning "duplicated .if. condition" } */
> +    return 105;
> +  else if (n == 6) /* { dg-warning "duplicated .if. condition" } */
> +    return 106;
> +  else if (n == 7) /* { dg-warning "duplicated .if. condition" } */
> +    return 107;
> +  return 0;
> +}
> +
> +int
> +fn8 (bool b)
> +{
> +  if (!b) /* { dg-message "previously used here" } */
> +    return 16;
> +  else if (!b) /* { dg-warning "duplicated .if. condition" } */
> +    return 27;
> +  else
> +    return 64;
> +}
> +
> +int
> +fn9 (int i, int j, int k)
> +{
> +  if (i > 0 && j > 0 && k > 0) /* { dg-message "previously used here" } */
> +    return -999;
> +  else
> +  if (i > 0 && j > 0 && k > 0) /* { dg-warning "duplicated .if. condition" } */
> +    return 999;
> +  else
> +    return 0;
> +}
> +
> +int
> +fn10 (void)
> +{
> +  if (foo ())
> +    return 1732984;
> +  else if (foo ())
> +    return 18409;
> +  return 0;
> +}
> +
> +int
> +fn11 (int n)
> +{
> +  if (++n == 10)
> +    return 666;
> +  else if (++n == 10)
> +    return 9;
> +  return 0;
> +}
> diff --git gcc/testsuite/c-c++-common/Wduplicated-cond-3.c gcc/testsuite/c-c++-common/Wduplicated-cond-3.c
> index e69de29..e3b5ac0 100644
> --- gcc/testsuite/c-c++-common/Wduplicated-cond-3.c
> +++ gcc/testsuite/c-c++-common/Wduplicated-cond-3.c
> @@ -0,0 +1,204 @@
> +/* PR c/64249 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -Wno-duplicated-cond" } */
> +
> +#ifndef __cplusplus
> +# define bool _Bool
> +# define true 1
> +# define false 0
> +#endif
> +
> +extern int foo (void);
> +
> +int
> +fn1 (int n)
> +{
> +  if (n == 1)
> +    return -1;
> +  else if (n == 2)
> +    return 0;
> +  else if (n == 1)
> +    return 1;
> +  return 0;
> +}
> +
> +int
> +fn2 (void)
> +{
> +  if (4)
> +    return 1;
> +  else if (4)
> +    return 2;
> +
> +#define N 10
> +  if (N)
> +    return 3;
> +  else if (N)
> +    return 4;
> +}
> +
> +int
> +fn3 (int n)
> +{
> +  if (n == 42)
> +    return 1;
> +  if (n == 42)
> +    return 2;
> +
> +  if (n)
> +    if (n)
> +      if (n)
> +	if (n)
> +	  return 42;
> +
> +  if (!n)
> +    return 10;
> +  else
> +    return 11;
> +}
> +
> +int
> +fn4 (int n)
> +{
> +  if (n > 0)
> +    {
> +      if (n == 1)
> +	return 1;
> +      else if (n == 1)
> +	return 2;
> +    }
> +  else if (n < 0)
> +    {
> +      if (n < -1)
> +	return 6;
> +      else if (n < -2)
> +	{
> +	  if (n == -10)
> +	    return 3;
> +	  else if (n == -10)
> +	    return 4;
> +	}
> +    }
> +  else
> +    return 7;
> +  return 0;
> +}
> +
> +struct S { long p, q; };
> +
> +int
> +fn5 (struct S *s)
> +{
> +  if (!s->p)
> +    return 12345;
> +  else if (!s->p)
> +    return 1234;
> +  return 0;
> +}
> +
> +int
> +fn6 (int n)
> +{
> +  if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  else if (n)
> +    return n;
> +  return 0;
> +}
> +
> +int
> +fn7 (int n)
> +{
> +  if (n == 0)
> +    return 10;
> +  else if (n == 1)
> +    return 11;
> +  else if (n == 2)
> +    return 12;
> +  else if (n == 3)
> +    return 13;
> +  else if (n == 4)
> +    return 14;
> +  else if (n == 5)
> +    return 15;
> +  else if (n == 6)
> +    return 16;
> +  else if (n == 7)
> +    return 17;
> +  else if (n == 0)
> +    return 100;
> +  else if (n == 1)
> +    return 101;
> +  else if (n == 2)
> +    return 102;
> +  else if (n == 3)
> +    return 103;
> +  else if (n == 4)
> +    return 104;
> +  else if (n == 5)
> +    return 105;
> +  else if (n == 6)
> +    return 106;
> +  else if (n == 7)
> +    return 107;
> +  return 0;
> +}
> +
> +int
> +fn8 (bool b)
> +{
> +  if (!b)
> +    return 16;
> +  else if (!b)
> +    return 27;
> +  else
> +    return 64;
> +}
> +
> +int
> +fn9 (int i, int j, int k)
> +{
> +  if ((i > 0 && j > 0 && k > 0)
> +      && ((i > 11 && j == 76 && k < 10)
> +	  || (i < 0 && j == 99 && k > 103)))
> +    return -999;
> +  else
> +  if ((i > 0 && j > 0 && k > 0)
> +      && ((i > 11 && j == 76 && k < 10)
> +	  || (i < 0 && j == 99 && k > 103)))
> +    return 999;
> +  else
> +    return 0;
> +}
> +
> +int
> +fn10 (void)
> +{
> +  if (foo ())
> +    return 1732984;
> +  else if (foo ())
> +    return 18409;
> +  return 0;
> +}
> +
> +int
> +fn11 (int n)
> +{
> +  if (++n == 10)
> +    return 666;
> +  else if (++n == 10)
> +    return 9;
> +  return 0;
> +}
> diff --git gcc/testsuite/c-c++-common/Wduplicated-cond-4.c gcc/testsuite/c-c++-common/Wduplicated-cond-4.c
> index e69de29..4fb7e17 100644
> --- gcc/testsuite/c-c++-common/Wduplicated-cond-4.c
> +++ gcc/testsuite/c-c++-common/Wduplicated-cond-4.c
> @@ -0,0 +1,32 @@
> +/* PR c/64249 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wduplicated-cond" } */
> +/* Test we don't warn if a condition in an if-else-if chain
> +   has a side-effect.  E.g. __cxxabiv1::__cxa_end_catch ()
> +   uses such a construction.  */
> +
> +extern int a, bar (void);
> +
> +int
> +fn1 (void)
> +{
> +  if (a)
> +    return 1;
> +  else if (bar ())
> +    return 2;
> +  else if (a)
> +    return 3;
> +  return 0;
> +}
> +
> +int
> +fn2 (int c)
> +{
> +  if (c < 0)
> +    return 1;
> +  else if (--c == 0)
> +    return 2;
> +  else if (c < 0)
> +    return 3;
> +  return 0;
> +}
> diff --git gcc/testsuite/c-c++-common/Wmisleading-indentation.c gcc/testsuite/c-c++-common/Wmisleading-indentation.c
> index 0d6d8d2..ca13141 100644
> --- gcc/testsuite/c-c++-common/Wmisleading-indentation.c
> +++ gcc/testsuite/c-c++-common/Wmisleading-indentation.c
> @@ -708,7 +708,7 @@ fn_37 (void)
>  
>    if (flagA)
>      ;
> -  else if (flagA); /* { dg-message "8: ...this 'if' clause" } */
> +  else if (flagB); /* { dg-message "8: ...this 'if' clause" } */
>      foo (0); /* { dg-warning "statement is indented as if" } */
>    while (flagA) /* { dg-message "3: ...this 'while' clause" } */
>      /* blah */;
> @@ -716,13 +716,13 @@ fn_37 (void)
>  
>    if (flagA)
>      ;
> -  else if (flagA) /* { dg-message "8: ...this 'if' clause" } */
> +  else if (flagB) /* { dg-message "8: ...this 'if' clause" } */
>      foo (1);
>      foo (2); /* { dg-warning "statement is indented as if" } */
>  
>    if (flagA)
>      foo (1);
> -  else if (flagA) /* { dg-message "8: ...this 'if' clause" } */
> +  else if (flagB) /* { dg-message "8: ...this 'if' clause" } */
>      foo (2);
>      foo (3); /* { dg-warning "statement is indented as if" } */
>  
> 
> 	Marek

	Marek



More information about the Gcc-patches mailing list