Re: Implement -Wduplicated-branches (PR c/64279) (v3)
This patch introduced a new testsuite failure: FAIL: g++.dg/warn/Wduplicated-branches1.C -std=gnu++98 (test for excess errors) Excess errors: /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/warn/Wduplicated-branches1.C:9:3: warning: this condition has identical branches [-Wduplicated-branches] /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/warn/Wduplicated-branches1.C:9:3: warning: this condition has identical branches [-Wduplicated-branches] Thanks, David
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Fri, Jan 20, 2017 at 10:05:22AM -0500, David Edelsohn wrote: > This patch introduced a new testsuite failure: > > FAIL: g++.dg/warn/Wduplicated-branches1.C -std=gnu++98 (test for excess > errors) > Excess errors: > /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/warn/Wduplicated-branches1.C:9:3: > warning: this condition has identical branches [-Wduplicated-branches] > /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/warn/Wduplicated-branches1.C:9:3: > warning: this condition has identical branches [-Wduplicated-branches] Sorry about that. I'll investigate. Marek
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Jan 16, 2017 at 03:32:59PM -0700, Jeff Law wrote:
> s/indentical/identical in the doc/invoke.texi changes.
Fixed.
> > diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> > index 96e7351..ed8ffe4 100644
> > --- gcc/c/c-typeck.c
> > +++ gcc/c/c-typeck.c
> > @@ -5193,6 +5193,15 @@ build_conditional_expr (location_t colon_loc, tree
> > ifexp, bool ifexp_bcp,
> > ret = build1 (EXCESS_PRECISION_EXPR, semantic_result_type, ret);
> >
> >protected_set_expr_location (ret, colon_loc);
> > +
> > + /* If the OP1 and OP2 are the same and don't have side-effects,
> > + warn here, because the COND_EXPR will be turned into OP1. */
> > + if (warn_duplicated_branches
> > + && TREE_CODE (ret) == COND_EXPR
> > + && (op1 == op2 || operand_equal_p (op1, op2, 0)))
> Did you want OEP_LEXICOGRAPHIC here, or in this context do we not have to
> worry about the more complex forms? What about a statement expressions?
> Have we lowered those at this point already?
I think we do not want OEP_LEXICOGRAPHIC here, because if the op0 or op1
of ?: have side-effects, they'll survive until c_genericize, so the warning
will warn. With OEP_LEXICOGRAPHIC, it'd warn twice.
Simple statement expressions are handled (there are a few tests). More
complicated ({})s are represented as BIND_EXPRs and those aren't handled.
> > diff --git gcc/cp/call.c gcc/cp/call.c
> > index e431221..84931fb 100644
> > --- gcc/cp/call.c
> > +++ gcc/cp/call.c
> > @@ -5302,6 +5302,13 @@ build_conditional_expr_1 (location_t loc, tree arg1,
> > tree arg2, tree arg3,
> > valid_operands:
> >result = build3_loc (loc, COND_EXPR, result_type, arg1, arg2, arg3);
> >
> > + /* If the ARG2 and ARG3 are the same and don't have side-effects,
> > + warn here, because the COND_EXPR will be turned into ARG2. */
> > + if (warn_duplicated_branches
> > + && (arg2 == arg3 || operand_equal_p (arg2, arg3, 0)))
> Likewise.
Same as above.
> So, typo fix in invoke.texi and change to use OEP_LEXICOGRAPHIC in those two
> spots if needed and then OK for the trunk.
Thanks!
This is the patch with the typo fixed. I'll commit it today after regtesting.
2017-01-20 Marek Polacek
PR c/64279
* c-common.h (do_warn_duplicated_branches_r): Declare.
* c-gimplify.c (c_genericize): Walk the function tree calling
do_warn_duplicated_branches_r.
* c-warn.c (expr_from_macro_expansion_r): New.
(do_warn_duplicated_branches): New.
(do_warn_duplicated_branches_r): New.
* c.opt (Wduplicated-branches): New option.
* c-typeck.c (build_conditional_expr): Warn about duplicated branches.
* call.c (build_conditional_expr_1): Warn about duplicated branches.
* semantics.c (finish_expr_stmt): Build statement using the proper
location.
* doc/invoke.texi: Document -Wduplicated-branches.
* fold-const.c (operand_equal_p): Handle MODIFY_EXPR, INIT_EXPR,
COMPOUND_EXPR, PREDECREMENT_EXPR, PREINCREMENT_EXPR,
POSTDECREMENT_EXPR, POSTINCREMENT_EXPR, CLEANUP_POINT_EXPR, EXPR_STMT,
STATEMENT_LIST, and RETURN_EXPR. For non-pure non-const functions
return 0 only when not OEP_LEXICOGRAPHIC.
(fold_build_cleanup_point_expr): Use the expression
location when building CLEANUP_POINT_EXPR.
* tree-core.h (enum operand_equal_flag): Add OEP_LEXICOGRAPHIC.
* tree.c (add_expr): Handle error_mark_node.
* c-c++-common/Wduplicated-branches-1.c: New test.
* c-c++-common/Wduplicated-branches-10.c: New test.
* c-c++-common/Wduplicated-branches-11.c: New test.
* c-c++-common/Wduplicated-branches-12.c: New test.
* c-c++-common/Wduplicated-branches-2.c: New test.
* c-c++-common/Wduplicated-branches-3.c: New test.
* c-c++-common/Wduplicated-branches-4.c: New test.
* c-c++-common/Wduplicated-branches-5.c: New test.
* c-c++-common/Wduplicated-branches-6.c: New test.
* c-c++-common/Wduplicated-branches-7.c: New test.
* c-c++-common/Wduplicated-branches-8.c: New test.
* c-c++-common/Wduplicated-branches-9.c: New test.
* c-c++-common/Wimplicit-fallthrough-7.c: Coalesce dg-warning.
* g++.dg/cpp0x/lambda/lambda-switch.C: Move dg-warning.
* g++.dg/ext/builtin-object-size3.C: Likewise.
* g++.dg/gomp/loop-1.C: Likewise.
* g++.dg/warn/Wduplicated-branches1.C: New test.
* g++.dg/warn/Wduplicated-branches2.C: New test.
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index b838869..06918db 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1537,6 +1537,7 @@ extern void maybe_warn_bool_compare (location_t, enum
tree_code, tree, tree);
extern bool maybe_warn_shift_overflow (location_t, tree, tree);
extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec **);
extern bool diagnose_mismatched_attributes (tree, tree);
+extern tree do_warn_duplicated_branch
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Thu, Jan 19, 2017 at 05:52:14PM +0100, Marek Polacek wrote: > > I agree that not warning for > > if (foo) > > return NULL; > > else > > return NULL; > > is bad. But how can I compare those expressions side-by-side? I'm not > > finding > > anything. :( > > Seems like ENOTIME to address this; will you be ok with the patch as-is > (modulo Jeff comments), if I open a PR about the above test case? Yeah. Jakub
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Jan 09, 2017 at 02:39:30PM +0100, Marek Polacek wrote:
> On Mon, Jan 09, 2017 at 12:18:01PM +0100, Jakub Jelinek wrote:
> > On Mon, Jan 09, 2017 at 10:21:47AM +0100, Marek Polacek wrote:
> > > +/* Callback function to determine whether an expression TP or one of its
> > > + subexpressions comes from macro expansion. Used to suppress bogus
> > > + warnings. */
> > > +
> > > +static tree
> > > +expr_from_macro_expansion_r (tree *tp, int *, void *)
> > > +{
> > > + if (CAN_HAVE_LOCATION_P (*tp)
> > > + && from_macro_expansion_at (EXPR_LOCATION (*tp)))
> > > +return integer_zero_node;
> > > +
> > > + return NULL_TREE;
> > > +}
> >
> > I know this is hard issue, but won't it disable the warning way too often?
> >
> > Perhaps it is good enough for the initial version (GCC 7), but doesn't it
> > stop
> > whenever one uses NULL in the branches, or some other trivial macros like
> > that? Perhaps we want to do the analysis if there is anything from macro
> > expansion side-by-side on both the expressions and if you find something
> > from a macro expansion, then still warn if both corresponding expressions
> > are from the same macro expansion (either only non-function like one, or
> > perhaps also function-like one with the same arguments, if it is possible
> > to figure out those somehow)? And perhaps it would be nice to choose
> > warning level, whether you want to warn only under these rules (no macros
> > or something smarter if implemented) vs. some certainly non-default more
> > aggressive mode that will just warn no matter what macros there are.
>
> I agree that not warning for
> if (foo)
> return NULL;
> else
> return NULL;
> is bad. But how can I compare those expressions side-by-side? I'm not
> finding
> anything. :(
Seems like ENOTIME to address this; will you be ok with the patch as-is
(modulo Jeff comments), if I open a PR about the above test case?
Thanks,
Marek
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On 01/09/2017 02:21 AM, Marek Polacek wrote: On Thu, Jan 05, 2017 at 04:41:28PM +0100, Jakub Jelinek wrote: On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote: Coming back to this... Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) or so (the exact last operand needs to be figured out). OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the same thing. 0 is a tiny bit better, but still it will give up on e.g. pure and other calls. OEP_PURE_SAME is tiny bit better than that, but still calls with the same arguments to the same function will not be considered equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. So maybe we need another OEP_* mode for this. Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning doesn't trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ mode for this (names? OEP_EXTENDED?) and then in operand_equal_p in case tcc_expression do case MODIFY_EXPR: if (flags & OEP_EXTENDED) // compare LHS and RHS of both ? Yeah. Not sure what is the best name for that. Maybe Richi has some clever ideas. Here it is. The changes in operand_equal_p should only trigger with the new OEP_LEXICOGRAPHIC, and given the macro location issue, the warning isn't yet enabled by neither -Wall nor -Wextra, so this all should be safe. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2017-01-09 Marek Polacek PR c/64279 * c-common.h (do_warn_duplicated_branches_r): Declare. * c-gimplify.c (c_genericize): Walk the function tree calling do_warn_duplicated_branches_r. * c-warn.c (expr_from_macro_expansion_r): New. (do_warn_duplicated_branches): New. (do_warn_duplicated_branches_r): New. * c.opt (Wduplicated-branches): New option. * c-typeck.c (build_conditional_expr): Warn about duplicated branches. * call.c (build_conditional_expr_1): Warn about duplicated branches. * semantics.c (finish_expr_stmt): Build statement using the proper location. * doc/invoke.texi: Document -Wduplicated-branches. * fold-const.c (operand_equal_p): Handle MODIFY_EXPR, INIT_EXPR, COMPOUND_EXPR, PREDECREMENT_EXPR, PREINCREMENT_EXPR, POSTDECREMENT_EXPR, POSTINCREMENT_EXPR, CLEANUP_POINT_EXPR, EXPR_STMT, STATEMENT_LIST, and RETURN_EXPR. For non-pure non-const functions return 0 only when not OEP_LEXICOGRAPHIC. (fold_build_cleanup_point_expr): Use the expression location when building CLEANUP_POINT_EXPR. * tree-core.h (enum operand_equal_flag): Add OEP_LEXICOGRAPHIC. * tree.c (add_expr): Handle error_mark_node. * c-c++-common/Wduplicated-branches-1.c: New test. * c-c++-common/Wduplicated-branches-10.c: New test. * c-c++-common/Wduplicated-branches-11.c: New test. * c-c++-common/Wduplicated-branches-12.c: New test. * c-c++-common/Wduplicated-branches-2.c: New test. * c-c++-common/Wduplicated-branches-3.c: New test. * c-c++-common/Wduplicated-branches-4.c: New test. * c-c++-common/Wduplicated-branches-5.c: New test. * c-c++-common/Wduplicated-branches-6.c: New test. * c-c++-common/Wduplicated-branches-7.c: New test. * c-c++-common/Wduplicated-branches-8.c: New test. * c-c++-common/Wduplicated-branches-9.c: New test. * c-c++-common/Wimplicit-fallthrough-7.c: Coalesce dg-warning. * g++.dg/cpp0x/lambda/lambda-switch.C: Move dg-warning. * g++.dg/ext/builtin-object-size3.C: Likewise. * g++.dg/gomp/loop-1.C: Likewise. * g++.dg/warn/Wduplicated-branches1.C: New test. * g++.dg/warn/Wduplicated-branches2.C: New test. s/indentical/identical in the doc/invoke.texi changes. diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 96e7351..ed8ffe4 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -5193,6 +5193,15 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp, ret = build1 (EXCESS_PRECISION_EXPR, semantic_result_type, ret); protected_set_expr_location (ret, colon_loc); + + /* If the OP1 and OP2 are the same and don't have side-effects, + warn here, because the COND_EXPR will be turned into OP1. */ + if (warn_duplicated_branches + && TREE_CODE (ret) == COND_EXPR + && (op1 == op2 || operand_equal_p (op1, op2, 0))) Did you want OEP_LEXICOGRAPHIC here, or in this context do we not have to worry about the more complex forms? What about a statement expressions? Have we lowered those at this point already? diff --git gcc/cp/call.c gcc/cp/call.c index e431221..84931fb 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -5302,6 +5302,13 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3, valid_operands: result = build3_loc (loc, COND_EXPR, result_typ
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Jan 09, 2017 at 12:18:01PM +0100, Jakub Jelinek wrote:
> On Mon, Jan 09, 2017 at 10:21:47AM +0100, Marek Polacek wrote:
> > +/* Callback function to determine whether an expression TP or one of its
> > + subexpressions comes from macro expansion. Used to suppress bogus
> > + warnings. */
> > +
> > +static tree
> > +expr_from_macro_expansion_r (tree *tp, int *, void *)
> > +{
> > + if (CAN_HAVE_LOCATION_P (*tp)
> > + && from_macro_expansion_at (EXPR_LOCATION (*tp)))
> > +return integer_zero_node;
> > +
> > + return NULL_TREE;
> > +}
>
> I know this is hard issue, but won't it disable the warning way too often?
>
> Perhaps it is good enough for the initial version (GCC 7), but doesn't it stop
> whenever one uses NULL in the branches, or some other trivial macros like
> that? Perhaps we want to do the analysis if there is anything from macro
> expansion side-by-side on both the expressions and if you find something
> from a macro expansion, then still warn if both corresponding expressions
> are from the same macro expansion (either only non-function like one, or
> perhaps also function-like one with the same arguments, if it is possible
> to figure out those somehow)? And perhaps it would be nice to choose
> warning level, whether you want to warn only under these rules (no macros
> or something smarter if implemented) vs. some certainly non-default more
> aggressive mode that will just warn no matter what macros there are.
I agree that not warning for
if (foo)
return NULL;
else
return NULL;
is bad. But how can I compare those expressions side-by-side? I'm not finding
anything. :(
As for the idea of multiple levels, sure, I could do that, although I'd prefer
to get the initial version in first.
Marek
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Jan 09, 2017 at 10:21:47AM +0100, Marek Polacek wrote:
> +/* Callback function to determine whether an expression TP or one of its
> + subexpressions comes from macro expansion. Used to suppress bogus
> + warnings. */
> +
> +static tree
> +expr_from_macro_expansion_r (tree *tp, int *, void *)
> +{
> + if (CAN_HAVE_LOCATION_P (*tp)
> + && from_macro_expansion_at (EXPR_LOCATION (*tp)))
> +return integer_zero_node;
> +
> + return NULL_TREE;
> +}
I know this is hard issue, but won't it disable the warning way too often?
Perhaps it is good enough for the initial version (GCC 7), but doesn't it stop
whenever one uses NULL in the branches, or some other trivial macros like
that? Perhaps we want to do the analysis if there is anything from macro
expansion side-by-side on both the expressions and if you find something
from a macro expansion, then still warn if both corresponding expressions
are from the same macro expansion (either only non-function like one, or
perhaps also function-like one with the same arguments, if it is possible
to figure out those somehow)? And perhaps it would be nice to choose
warning level, whether you want to warn only under these rules (no macros
or something smarter if implemented) vs. some certainly non-default more
aggressive mode that will just warn no matter what macros there are.
Jakub
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Jan 09, 2017 at 11:57:48AM +0100, Richard Biener wrote: > On Mon, 9 Jan 2017, Marek Polacek wrote: > > > On Thu, Jan 05, 2017 at 04:41:28PM +0100, Jakub Jelinek wrote: > > > On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote: > > > > Coming back to this... > > > > > > > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) > > > > > or so (the exact last operand needs to be figured out). > > > > > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean > > > > > the > > > > > same thing. 0 is a tiny bit better, but still it will give up on > > > > > e.g. pure > > > > > and other calls. OEP_PURE_SAME is tiny bit better than that, but > > > > > still > > > > > calls with the same arguments to the same function will not be > > > > > considered > > > > > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. > > > > > So maybe we need another OEP_* mode for this. > > > > > > > > Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning > > > > doesn't > > > > trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably > > > > STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ > > > > mode for > > > > this (names? OEP_EXTENDED?) and then in operand_equal_p in case > > > > tcc_expression > > > > do > > > > > > > > case MODIFY_EXPR: > > > > if (flags & OEP_EXTENDED) > > > > // compare LHS and RHS of both > > > > > > > > ? > > > > > > Yeah. Not sure what is the best name for that. Maybe Richi has some > > > clever > > > ideas. > > > > Here it is. The changes in operand_equal_p should only trigger with the new > > OEP_LEXICOGRAPHIC, and given the macro location issue, the warning isn't yet > > enabled by neither -Wall nor -Wextra, so this all should be safe. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > @@ -2722,6 +2722,9 @@ combine_comparisons (location_t loc, > If OEP_ADDRESS_OF is set, we are actually comparing addresses of > objects, > not values of expressions. > > + If OEP_LEXICOGRAPHIC is set, then also handle expressions such as > + MODIFY_EXPR, RETURN_EXPR, as well as STATEMENT_LISTs. > + > > I'd say "also handle expressions with side-effects such as ..." > > otherwise the middle-end changes look good to me - I'll defer to > C FE maintainers for the rest. Thanks, I'll fix it up. Marek
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, 9 Jan 2017, Marek Polacek wrote: > On Thu, Jan 05, 2017 at 04:41:28PM +0100, Jakub Jelinek wrote: > > On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote: > > > Coming back to this... > > > > > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) > > > > or so (the exact last operand needs to be figured out). > > > > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean > > > > the > > > > same thing. 0 is a tiny bit better, but still it will give up on e.g. > > > > pure > > > > and other calls. OEP_PURE_SAME is tiny bit better than that, but still > > > > calls with the same arguments to the same function will not be > > > > considered > > > > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. > > > > So maybe we need another OEP_* mode for this. > > > > > > Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning > > > doesn't > > > trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably > > > STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ > > > mode for > > > this (names? OEP_EXTENDED?) and then in operand_equal_p in case > > > tcc_expression > > > do > > > > > > case MODIFY_EXPR: > > > if (flags & OEP_EXTENDED) > > > // compare LHS and RHS of both > > > > > > ? > > > > Yeah. Not sure what is the best name for that. Maybe Richi has some clever > > ideas. > > Here it is. The changes in operand_equal_p should only trigger with the new > OEP_LEXICOGRAPHIC, and given the macro location issue, the warning isn't yet > enabled by neither -Wall nor -Wextra, so this all should be safe. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? @@ -2722,6 +2722,9 @@ combine_comparisons (location_t loc, If OEP_ADDRESS_OF is set, we are actually comparing addresses of objects, not values of expressions. + If OEP_LEXICOGRAPHIC is set, then also handle expressions such as + MODIFY_EXPR, RETURN_EXPR, as well as STATEMENT_LISTs. + I'd say "also handle expressions with side-effects such as ..." otherwise the middle-end changes look good to me - I'll defer to C FE maintainers for the rest. Thanks, Richard. > 2017-01-09 Marek Polacek > > PR c/64279 > * c-common.h (do_warn_duplicated_branches_r): Declare. > * c-gimplify.c (c_genericize): Walk the function tree calling > do_warn_duplicated_branches_r. > * c-warn.c (expr_from_macro_expansion_r): New. > (do_warn_duplicated_branches): New. > (do_warn_duplicated_branches_r): New. > * c.opt (Wduplicated-branches): New option. > > * c-typeck.c (build_conditional_expr): Warn about duplicated branches. > > * call.c (build_conditional_expr_1): Warn about duplicated branches. > * semantics.c (finish_expr_stmt): Build statement using the proper > location. > > * doc/invoke.texi: Document -Wduplicated-branches. > * fold-const.c (operand_equal_p): Handle MODIFY_EXPR, INIT_EXPR, > COMPOUND_EXPR, PREDECREMENT_EXPR, PREINCREMENT_EXPR, > POSTDECREMENT_EXPR, POSTINCREMENT_EXPR, CLEANUP_POINT_EXPR, EXPR_STMT, > STATEMENT_LIST, and RETURN_EXPR. For non-pure non-const functions > return 0 only when not OEP_LEXICOGRAPHIC. > (fold_build_cleanup_point_expr): Use the expression > location when building CLEANUP_POINT_EXPR. > * tree-core.h (enum operand_equal_flag): Add OEP_LEXICOGRAPHIC. > * tree.c (add_expr): Handle error_mark_node. > > * c-c++-common/Wduplicated-branches-1.c: New test. > * c-c++-common/Wduplicated-branches-10.c: New test. > * c-c++-common/Wduplicated-branches-11.c: New test. > * c-c++-common/Wduplicated-branches-12.c: New test. > * c-c++-common/Wduplicated-branches-2.c: New test. > * c-c++-common/Wduplicated-branches-3.c: New test. > * c-c++-common/Wduplicated-branches-4.c: New test. > * c-c++-common/Wduplicated-branches-5.c: New test. > * c-c++-common/Wduplicated-branches-6.c: New test. > * c-c++-common/Wduplicated-branches-7.c: New test. > * c-c++-common/Wduplicated-branches-8.c: New test. > * c-c++-common/Wduplicated-branches-9.c: New test. > * c-c++-common/Wimplicit-fallthrough-7.c: Coalesce dg-warning. > * g++.dg/cpp0x/lambda/lambda-switch.C: Move dg-warning. > * g++.dg/ext/builtin-object-size3.C: Likewise. > * g++.dg/gomp/loop-1.C: Likewise. > * g++.dg/warn/Wduplicated-branches1.C: New test. > * g++.dg/warn/Wduplicated-branches2.C: New test. > > diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h > index b838869..06918db 100644 > --- gcc/c-family/c-common.h > +++ gcc/c-family/c-common.h > @@ -1537,6 +1537,7 @@ extern void maybe_warn_bool_compare (location_t, enum > tree_code, tree, tree); > extern bool maybe_warn_shift_overflow (location_t, tree, tree); > extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec > **); > extern bool
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Thu, Jan 05, 2017 at 04:41:28PM +0100, Jakub Jelinek wrote: > On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote: > > Coming back to this... > > > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) > > > or so (the exact last operand needs to be figured out). > > > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the > > > same thing. 0 is a tiny bit better, but still it will give up on e.g. > > > pure > > > and other calls. OEP_PURE_SAME is tiny bit better than that, but still > > > calls with the same arguments to the same function will not be considered > > > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. > > > So maybe we need another OEP_* mode for this. > > > > Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning > > doesn't > > trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably > > STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ mode > > for > > this (names? OEP_EXTENDED?) and then in operand_equal_p in case > > tcc_expression > > do > > > > case MODIFY_EXPR: > > if (flags & OEP_EXTENDED) > > // compare LHS and RHS of both > > > > ? > > Yeah. Not sure what is the best name for that. Maybe Richi has some clever > ideas. Here it is. The changes in operand_equal_p should only trigger with the new OEP_LEXICOGRAPHIC, and given the macro location issue, the warning isn't yet enabled by neither -Wall nor -Wextra, so this all should be safe. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2017-01-09 Marek Polacek PR c/64279 * c-common.h (do_warn_duplicated_branches_r): Declare. * c-gimplify.c (c_genericize): Walk the function tree calling do_warn_duplicated_branches_r. * c-warn.c (expr_from_macro_expansion_r): New. (do_warn_duplicated_branches): New. (do_warn_duplicated_branches_r): New. * c.opt (Wduplicated-branches): New option. * c-typeck.c (build_conditional_expr): Warn about duplicated branches. * call.c (build_conditional_expr_1): Warn about duplicated branches. * semantics.c (finish_expr_stmt): Build statement using the proper location. * doc/invoke.texi: Document -Wduplicated-branches. * fold-const.c (operand_equal_p): Handle MODIFY_EXPR, INIT_EXPR, COMPOUND_EXPR, PREDECREMENT_EXPR, PREINCREMENT_EXPR, POSTDECREMENT_EXPR, POSTINCREMENT_EXPR, CLEANUP_POINT_EXPR, EXPR_STMT, STATEMENT_LIST, and RETURN_EXPR. For non-pure non-const functions return 0 only when not OEP_LEXICOGRAPHIC. (fold_build_cleanup_point_expr): Use the expression location when building CLEANUP_POINT_EXPR. * tree-core.h (enum operand_equal_flag): Add OEP_LEXICOGRAPHIC. * tree.c (add_expr): Handle error_mark_node. * c-c++-common/Wduplicated-branches-1.c: New test. * c-c++-common/Wduplicated-branches-10.c: New test. * c-c++-common/Wduplicated-branches-11.c: New test. * c-c++-common/Wduplicated-branches-12.c: New test. * c-c++-common/Wduplicated-branches-2.c: New test. * c-c++-common/Wduplicated-branches-3.c: New test. * c-c++-common/Wduplicated-branches-4.c: New test. * c-c++-common/Wduplicated-branches-5.c: New test. * c-c++-common/Wduplicated-branches-6.c: New test. * c-c++-common/Wduplicated-branches-7.c: New test. * c-c++-common/Wduplicated-branches-8.c: New test. * c-c++-common/Wduplicated-branches-9.c: New test. * c-c++-common/Wimplicit-fallthrough-7.c: Coalesce dg-warning. * g++.dg/cpp0x/lambda/lambda-switch.C: Move dg-warning. * g++.dg/ext/builtin-object-size3.C: Likewise. * g++.dg/gomp/loop-1.C: Likewise. * g++.dg/warn/Wduplicated-branches1.C: New test. * g++.dg/warn/Wduplicated-branches2.C: New test. diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index b838869..06918db 100644 --- gcc/c-family/c-common.h +++ gcc/c-family/c-common.h @@ -1537,6 +1537,7 @@ extern void maybe_warn_bool_compare (location_t, enum tree_code, tree, tree); extern bool maybe_warn_shift_overflow (location_t, tree, tree); extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec **); extern bool diagnose_mismatched_attributes (tree, tree); +extern tree do_warn_duplicated_branches_r (tree *, int *, void *); /* In c-attribs.c. */ extern bool attribute_takes_identifier_p (const_tree); diff --git gcc/c-family/c-gimplify.c gcc/c-family/c-gimplify.c index c327ca7..57edb41 100644 --- gcc/c-family/c-gimplify.c +++ gcc/c-family/c-gimplify.c @@ -125,6 +125,10 @@ c_genericize (tree fndecl) &pset); } + if (warn_duplicated_branches) +walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl), + do_warn_duplicated_branches_r, NULL); + /* Dump the C-specific tree IR. */ dump_orig
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On January 5, 2017 4:41:28 PM GMT+01:00, Jakub Jelinek wrote: >On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote: >> Coming back to this... > >> > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, >0) >> > or so (the exact last operand needs to be figured out). >> > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to >mean the >> > same thing. 0 is a tiny bit better, but still it will give up on >e.g. pure >> > and other calls. OEP_PURE_SAME is tiny bit better than that, but >still >> > calls with the same arguments to the same function will not be >considered >> > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST >etc. >> > So maybe we need another OEP_* mode for this. >> >> Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this >warning doesn't >> trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably >> STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ >mode for >> this (names? OEP_EXTENDED?) and then in operand_equal_p in case >tcc_expression >> do >> >> case MODIFY_EXPR: >> if (flags & OEP_EXTENDED) >> // compare LHS and RHS of both >> >> ? > >Yeah. Not sure what is the best name for that. Maybe Richi has some >clever >ideas. OEP_LEXICOGRAPHIC? Richard. > > Jakub
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote: > Coming back to this... > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) > > or so (the exact last operand needs to be figured out). > > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the > > same thing. 0 is a tiny bit better, but still it will give up on e.g. pure > > and other calls. OEP_PURE_SAME is tiny bit better than that, but still > > calls with the same arguments to the same function will not be considered > > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. > > So maybe we need another OEP_* mode for this. > > Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning doesn't > trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably > STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ mode for > this (names? OEP_EXTENDED?) and then in operand_equal_p in case > tcc_expression > do > > case MODIFY_EXPR: > if (flags & OEP_EXTENDED) > // compare LHS and RHS of both > > ? Yeah. Not sure what is the best name for that. Maybe Richi has some clever ideas. Jakub
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
Coming back to this... On Thu, Nov 03, 2016 at 02:38:50PM +0100, Jakub Jelinek wrote: > On Thu, Nov 03, 2016 at 09:27:55AM -0400, Jason Merrill wrote: > > On Thu, Nov 3, 2016 at 7:24 AM, Marek Polacek wrote: > > > On Tue, Nov 01, 2016 at 02:53:58PM +0100, Jakub Jelinek wrote: > > >> On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote: > > >> > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek > > >> > wrote: > > >> > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: > > >> > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: > > >> > >> > I found a problem with this patch--we can't call > > >> > >> > do_warn_duplicated_branches in > > >> > >> > build_conditional_expr, because that way the C++-specific codes > > >> > >> > might leak into > > >> > >> > the hasher. Instead, I should use operand_equal_p, I think. Let > > >> > >> > me rework > > >> > >> > that part of the patch. > > >> > > > >> > Hmm, is there a reason not to use operand_equal_p for > > >> > do_warn_duplicated_branches as well? I'm concerned about hash > > >> > collisions leading to false positives. > > >> > > >> If the hashing function is iterative_hash_expr / inchash::add_expr, then > > >> that is supposed to pair together with operand_equal_p, we even have > > >> checking verification of that. > > > > Yes, but there could still be hash collisions; we can't guarantee that > > everything that compares unequal also hashes unequal. > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) > or so (the exact last operand needs to be figured out). > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the > same thing. 0 is a tiny bit better, but still it will give up on e.g. pure > and other calls. OEP_PURE_SAME is tiny bit better than that, but still > calls with the same arguments to the same function will not be considered > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. > So maybe we need another OEP_* mode for this. Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning doesn't trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably STATEMENT_LIST and others. So I suppose I could introduce a new OEP_ mode for this (names? OEP_EXTENDED?) and then in operand_equal_p in case tcc_expression do case MODIFY_EXPR: if (flags & OEP_EXTENDED) // compare LHS and RHS of both ? Marek
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On 11/3/16, Jakub Jelinek wrote: > On Thu, Nov 03, 2016 at 09:27:55AM -0400, Jason Merrill wrote: >> On Thu, Nov 3, 2016 at 7:24 AM, Marek Polacek wrote: >> > On Tue, Nov 01, 2016 at 02:53:58PM +0100, Jakub Jelinek wrote: >> >> On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote: >> >> > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek >> >> > wrote: >> >> > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: >> >> > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: >> >> > >> > I found a problem with this patch--we can't call >> >> > >> > do_warn_duplicated_branches in >> >> > >> > build_conditional_expr, because that way the C++-specific codes >> >> > >> > might leak into >> >> > >> > the hasher. Instead, I should use operand_equal_p, I think. >> >> > >> > Let me rework >> >> > >> > that part of the patch. >> >> > >> >> > Hmm, is there a reason not to use operand_equal_p for >> >> > do_warn_duplicated_branches as well? I'm concerned about hash >> >> > collisions leading to false positives. >> >> >> >> If the hashing function is iterative_hash_expr / inchash::add_expr, >> >> then >> >> that is supposed to pair together with operand_equal_p, we even have >> >> checking verification of that. >> >> Yes, but there could still be hash collisions; we can't guarantee that >> everything that compares unequal also hashes unequal. > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) > or so (the exact last operand needs to be figured out). > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the > same thing. 0 is a tiny bit better, but still it will give up on e.g. pure > and other calls. OEP_PURE_SAME is tiny bit better than that, but still > calls with the same arguments to the same function will not be considered > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. > So maybe we need another OEP_* mode for this. > > Jakub > Pinging this conversation for the new year. Any chances of -Wduplicated-branches making it in in time for GCC 7? Thanks, Eric
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Thu, Nov 03, 2016 at 09:27:55AM -0400, Jason Merrill wrote: > On Thu, Nov 3, 2016 at 7:24 AM, Marek Polacek wrote: > > On Tue, Nov 01, 2016 at 02:53:58PM +0100, Jakub Jelinek wrote: > >> On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote: > >> > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek > >> > wrote: > >> > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: > >> > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: > >> > >> > I found a problem with this patch--we can't call > >> > >> > do_warn_duplicated_branches in > >> > >> > build_conditional_expr, because that way the C++-specific codes > >> > >> > might leak into > >> > >> > the hasher. Instead, I should use operand_equal_p, I think. Let > >> > >> > me rework > >> > >> > that part of the patch. > >> > > >> > Hmm, is there a reason not to use operand_equal_p for > >> > do_warn_duplicated_branches as well? I'm concerned about hash > >> > collisions leading to false positives. > >> > >> If the hashing function is iterative_hash_expr / inchash::add_expr, then > >> that is supposed to pair together with operand_equal_p, we even have > >> checking verification of that. > > Yes, but there could still be hash collisions; we can't guarantee that > everything that compares unequal also hashes unequal. Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0) or so (the exact last operand needs to be figured out). OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean the same thing. 0 is a tiny bit better, but still it will give up on e.g. pure and other calls. OEP_PURE_SAME is tiny bit better than that, but still calls with the same arguments to the same function will not be considered equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc. So maybe we need another OEP_* mode for this. Jakub
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Thu, Nov 3, 2016 at 7:24 AM, Marek Polacek wrote: > On Tue, Nov 01, 2016 at 02:53:58PM +0100, Jakub Jelinek wrote: >> On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote: >> > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek wrote: >> > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: >> > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: >> > >> > I found a problem with this patch--we can't call >> > >> > do_warn_duplicated_branches in >> > >> > build_conditional_expr, because that way the C++-specific codes might >> > >> > leak into >> > >> > the hasher. Instead, I should use operand_equal_p, I think. Let me >> > >> > rework >> > >> > that part of the patch. >> > >> > Hmm, is there a reason not to use operand_equal_p for >> > do_warn_duplicated_branches as well? I'm concerned about hash >> > collisions leading to false positives. >> >> If the hashing function is iterative_hash_expr / inchash::add_expr, then >> that is supposed to pair together with operand_equal_p, we even have >> checking verification of that. Yes, but there could still be hash collisions; we can't guarantee that everything that compares unequal also hashes unequal. Jason
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Tue, Nov 01, 2016 at 02:53:58PM +0100, Jakub Jelinek wrote: > On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote: > > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek wrote: > > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: > > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: > > >> > I found a problem with this patch--we can't call > > >> > do_warn_duplicated_branches in > > >> > build_conditional_expr, because that way the C++-specific codes might > > >> > leak into > > >> > the hasher. Instead, I should use operand_equal_p, I think. Let me > > >> > rework > > >> > that part of the patch. > > > > Hmm, is there a reason not to use operand_equal_p for > > do_warn_duplicated_branches as well? I'm concerned about hash > > collisions leading to false positives. > > If the hashing function is iterative_hash_expr / inchash::add_expr, then > that is supposed to pair together with operand_equal_p, we even have > checking verification of that. Yes, I use inchash::add_expr. So any opinions as to what to do with this patch? Marek
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Tue, Nov 01, 2016 at 09:41:20AM -0400, Jason Merrill wrote: > On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek wrote: > > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: > >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: > >> > I found a problem with this patch--we can't call > >> > do_warn_duplicated_branches in > >> > build_conditional_expr, because that way the C++-specific codes might > >> > leak into > >> > the hasher. Instead, I should use operand_equal_p, I think. Let me > >> > rework > >> > that part of the patch. > > Hmm, is there a reason not to use operand_equal_p for > do_warn_duplicated_branches as well? I'm concerned about hash > collisions leading to false positives. If the hashing function is iterative_hash_expr / inchash::add_expr, then that is supposed to pair together with operand_equal_p, we even have checking verification of that. Jakub
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Tue, Oct 25, 2016 at 9:59 AM, Marek Polacek wrote: > On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote: >> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote: >> > I found a problem with this patch--we can't call >> > do_warn_duplicated_branches in >> > build_conditional_expr, because that way the C++-specific codes might leak >> > into >> > the hasher. Instead, I should use operand_equal_p, I think. Let me rework >> > that part of the patch. Hmm, is there a reason not to use operand_equal_p for do_warn_duplicated_branches as well? I'm concerned about hash collisions leading to false positives. Jason
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote:
> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote:
> > I found a problem with this patch--we can't call
> > do_warn_duplicated_branches in
> > build_conditional_expr, because that way the C++-specific codes might leak
> > into
> > the hasher. Instead, I should use operand_equal_p, I think. Let me rework
> > that part of the patch.
>
> Thus:
And one more thing, let's not warn for if { } else { }, either.
Thanks Tobias for testing.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-10-25 Marek Polacek
PR c/64279
* c-common.h (do_warn_duplicated_branches_r,
do_warn_duplicated_branches): Declare.
* c-gimplify.c (c_genericize): Walk the function tree calling
do_warn_duplicated_branches_r.
* c-warn.c (expr_from_macro_expansion_r): New.
(do_warn_duplicated_branches): New.
(do_warn_duplicated_branches_r): New.
* c.opt (Wduplicated-branches): New option.
* c-typeck.c (build_conditional_expr): Warn about duplicated branches.
* call.c (build_conditional_expr_1): Warn about duplicated branches.
* semantics.c (finish_expr_stmt): Build statement using the proper
location.
* doc/invoke.texi: Document -Wduplicated-branches.
* fold-const.c (fold_build_cleanup_point_expr): Use the expression
location when building CLEANUP_POINT_EXPR.
* tree.c (add_expr): Handle error_mark_node.
* c-c++-common/Wduplicated-branches-1.c: New test.
* c-c++-common/Wduplicated-branches-2.c: New test.
* c-c++-common/Wduplicated-branches-3.c: New test.
* c-c++-common/Wduplicated-branches-4.c: New test.
* c-c++-common/Wduplicated-branches-5.c: New test.
* c-c++-common/Wduplicated-branches-6.c: New test.
* c-c++-common/Wduplicated-branches-7.c: New test.
* c-c++-common/Wduplicated-branches-8.c: New test.
* c-c++-common/Wduplicated-branches-9.c: New test.
* c-c++-common/Wduplicated-branches-10.c: New test.
* c-c++-common/Wimplicit-fallthrough-7.c: Coalesce dg-warning.
* g++.dg/cpp0x/lambda/lambda-switch.C: Move dg-warning.
* g++.dg/ext/builtin-object-size3.C (bar): Likewise.
* g++.dg/gomp/loop-1.C: Likewise.
* g++.dg/warn/Wduplicated-branches1.C: New test.
* g++.dg/warn/Wduplicated-branches2.C: New test.
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index 547bab2..46e9d2e 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1530,6 +1530,7 @@ extern void maybe_warn_bool_compare (location_t, enum
tree_code, tree, tree);
extern bool maybe_warn_shift_overflow (location_t, tree, tree);
extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec **);
extern bool diagnose_mismatched_attributes (tree, tree);
+extern tree do_warn_duplicated_branches_r (tree *, int *, void *);
/* In c-attribs.c. */
extern bool attribute_takes_identifier_p (const_tree);
diff --git gcc/c-family/c-gimplify.c gcc/c-family/c-gimplify.c
index c18b057..3ed2da8 100644
--- gcc/c-family/c-gimplify.c
+++ gcc/c-family/c-gimplify.c
@@ -125,6 +125,10 @@ c_genericize (tree fndecl)
&pset);
}
+ if (warn_duplicated_branches)
+walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl),
+ do_warn_duplicated_branches_r, NULL);
+
/* Dump the C-specific tree IR. */
dump_orig = get_dump_info (TDI_original, &local_dump_flags);
if (dump_orig)
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 904f6d3..433f5c3 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -2154,3 +2154,72 @@ maybe_warn_bool_compare (location_t loc, enum tree_code
code, tree op0,
"with boolean expression is always false", cst);
}
}
+
+/* Callback function to determine whether an expression TP or one of its
+ subexpressions comes from macro expansion. Used to suppress bogus
+ warnings. */
+
+static tree
+expr_from_macro_expansion_r (tree *tp, int *, void *)
+{
+ if (CAN_HAVE_LOCATION_P (*tp)
+ && from_macro_expansion_at (EXPR_LOCATION (*tp)))
+return integer_zero_node;
+
+ return NULL_TREE;
+}
+
+/* Possibly warn when an if-else has identical branches. */
+
+static void
+do_warn_duplicated_branches (tree expr)
+{
+ tree thenb = COND_EXPR_THEN (expr);
+ tree elseb = COND_EXPR_ELSE (expr);
+
+ /* Don't bother if there's no else branch. */
+ if (elseb == NULL_TREE)
+return;
+
+ /* And don't warn for empty statements. */
+ if (TREE_CODE (thenb) == NOP_EXPR
+ && TREE_TYPE (thenb) == void_type_node
+ && TREE_OPERAND (thenb, 0) == size_zero_node)
+return;
+
+ /* ... or empty branches. */
+ if (TREE_CODE (thenb) == STATEMENT_LIST
+ && STATEMENT_LIST_HEAD (thenb) == NULL)
+return;
+
+ /* Compute the hash of the then branch. */
+ inchash::hash hstate0 (0);
+ inchash::ad
