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
next 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).