This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)
- From: Marek Polacek <polacek at redhat dot com>
- To: Martin Sebor <msebor at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>, Joseph Myers <joseph at codesourcery dot com>
- Date: Wed, 30 Sep 2015 16:47:57 +0200
- Subject: Re: [C/C++ PATCH] RFC: Implement -Wduplicated-cond (PR c/64249) (version 2)
- Authentication-results: sourceware.org; auth=none
- References: <20150918134434 dot GG27588 at redhat dot com>
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