public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: Martin Sebor <msebor@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Jason Merrill <jason@redhat.com>,
	       Joseph Myers <joseph@codesourcery.com>
Subject: Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take
Date: Fri, 18 Sep 2015 13:46:00 -0000	[thread overview]
Message-ID: <20150918134434.GG27588@redhat.com> (raw)

2)
Reply-To: 
In-Reply-To: <20150918100606.GF27588@redhat.com>

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

             reply	other threads:[~2015-09-18 13:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-18 13:46 Marek Polacek [this message]
2015-09-18 16:50 ` Martin Sebor
2015-09-18 18:26   ` Manuel López-Ibáñez
2015-09-21 17:17     ` Marek Polacek
2015-09-21 17:11   ` Marek Polacek
2015-09-22 12:38     ` Marek Polacek
2015-09-22 22:49       ` Martin Sebor
2015-09-23 17:22         ` Marek Polacek
2015-09-23 18:23           ` Jeff Law
2015-09-23 18:31             ` Marek Polacek
2015-09-30 15:24 ` [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2) Marek Polacek
2015-09-30 15:55   ` Joseph Myers
2015-09-30 18:51     ` Jeff Law
2015-10-02 10:24       ` Marek Polacek
2015-10-02 15:43         ` H.J. Lu
2015-10-02 15:52           ` Marek Polacek
2015-10-03  7:07 ` [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (take Andreas Schwab
2015-10-03 17:00   ` Marek Polacek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150918134434.GG27588@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=msebor@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).