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 <pola...@redhat.com>
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_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.
So, typo fix in invoke.texi and change to use OEP_LEXICOGRAPHIC in those
two spots if needed and then OK for the trunk.
jeff
