2017-01-30 Jakub Jelinek PR c++/79232 * typeck.c (cp_build_modify_expr): Handle properly COMPOUND_EXPRs on lhs that have {PRE{DEC,INC}REMENT,MODIFY,MIN,MAX,COND}_EXPR in the rightmost operand. * g++.dg/cpp1z/eval-order4.C: New test. * g++.dg/other/pr79232.C: New test. --- gcc/cp/typeck.c.jj 2017-01-30 09:31:43.076595640 +0100 +++ gcc/cp/typeck.c 2017-01-30 15:56:33.601002577 +0100 @@ -7568,16 +7568,26 @@ tree cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode, tree rhs, tsubst_flags_t complain) { - tree result; + tree result = NULL_TREE; tree newrhs = rhs; tree lhstype = TREE_TYPE (lhs); + tree olhs = lhs; tree olhstype = lhstype; bool plain_assign = (modifycode == NOP_EXPR); + bool compound_side_effects_p = false; + tree preeval = NULL_TREE; /* Avoid duplicate error messages from operands that had errors. */ if (error_operand_p (lhs) || error_operand_p (rhs)) return error_mark_node; + while (TREE_CODE (lhs) == COMPOUND_EXPR) + { + if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0))) + compound_side_effects_p = true; + lhs = TREE_OPERAND (lhs, 1); + } + /* Handle control structure constructs used as "lvalues". Note that we leave COMPOUND_EXPR on the LHS because it is sequenced after the RHS. */ switch (TREE_CODE (lhs)) @@ -7585,20 +7595,57 @@ cp_build_modify_expr (location_t loc, tr /* Handle --foo = 5; as these are valid constructs in C++. */ case PREDECREMENT_EXPR: case PREINCREMENT_EXPR: + if (compound_side_effects_p) + { + if (VOID_TYPE_P (TREE_TYPE (rhs))) + { + if (complain & tf_error) + error ("void value not ignored as it ought to be"); + return error_mark_node; + } + newrhs = rhs = stabilize_expr (rhs, &preeval); + } if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0))) lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs), cp_stabilize_reference (TREE_OPERAND (lhs, 0)), TREE_OPERAND (lhs, 1)); lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0)); + maybe_add_compound: + /* If we had (bar, --foo) = 5; or (bar, (baz, --foo)) = 5; + and looked through the COMPOUND_EXPRs, readd them now around + the resulting lhs. */ + if (TREE_CODE (olhs) == COMPOUND_EXPR) + { + lhs = build2 (COMPOUND_EXPR, lhstype, TREE_OPERAND (olhs, 0), lhs); + tree *ptr = &TREE_OPERAND (lhs, 1); + for (olhs = TREE_OPERAND (olhs, 1); + TREE_CODE (olhs) == COMPOUND_EXPR; + olhs = TREE_OPERAND (olhs, 1)) + { + *ptr = build2 (COMPOUND_EXPR, lhstype, + TREE_OPERAND (olhs, 0), *ptr); + ptr = &TREE_OPERAND (*ptr, 1); + } + } break; case MODIFY_EXPR: + if (compound_side_effects_p) + { + if (VOID_TYPE_P (TREE_TYPE (rhs))) + { + if (complain & tf_error) + error ("void value not ignored as it ought to be"); + return error_mark_node; + } + newrhs = rhs = stabilize_expr (rhs, &preeval); + } if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0))) lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs), cp_stabilize_reference (TREE_OPERAND (lhs, 0)), TREE_OPERAND (lhs, 1)); lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0)); - break; + goto maybe_add_compound; case MIN_EXPR: case MAX_EXPR: @@ -7626,7 +7673,6 @@ cp_build_modify_expr (location_t loc, tr except that the RHS goes through a save-expr so the code to compute it is only emitted once. */ tree cond; - tree preeval = NULL_TREE; if (VOID_TYPE_P (TREE_TYPE (rhs))) { @@ -7652,14 +7698,31 @@ cp_build_modify_expr (location_t loc, tr if (cond == error_mark_node) return cond; + /* If we had (e, (a ? b : c)) = d; or (e, (f, (a ? b : c))) = d; + and looked through the COMPOUND_EXPRs, readd them now around + the resulting cond before adding the preevaluated rhs. */ + if (TREE_CODE (olhs) == COMPOUND_EXPR) + { + cond = build2 (COMPOUND_EXPR, TREE_TYPE (cond), + TREE_OPERAND (olhs, 0), cond); + tree *ptr = &TREE_OPERAND (cond, 1); + for (olhs = TREE_OPERAND (olhs, 1); + TREE_CODE (olhs) == COMPOUND_EXPR; + olhs = TREE_OPERAND (olhs, 1)) + { + *ptr = build2 (COMPOUND_EXPR, TREE_TYPE (cond), + TREE_OPERAND (olhs, 0), *ptr); + ptr = &TREE_OPERAND (*ptr, 1); + } + } /* Make sure the code to compute the rhs comes out before the split. */ - if (preeval) - cond = build2 (COMPOUND_EXPR, TREE_TYPE (lhs), preeval, cond); - return cond; + result = cond; + goto ret; } default: + lhs = olhs; break; } @@ -7675,7 +7738,7 @@ cp_build_modify_expr (location_t loc, tr rhs = convert (lhstype, rhs); result = build2 (INIT_EXPR, lhstype, lhs, rhs); TREE_SIDE_EFFECTS (result) = 1; - return result; + goto ret; } else if (! MAYBE_CLASS_TYPE_P (lhstype)) /* Do the default thing. */; @@ -7688,7 +7751,7 @@ cp_build_modify_expr (location_t loc, tr release_tree_vector (rhs_vec); if (result == NULL_TREE) return error_mark_node; - return result; + goto ret; } } else @@ -7703,7 +7766,7 @@ cp_build_modify_expr (location_t loc, tr { result = objc_maybe_build_modify_expr (lhs, rhs); if (result) - return result; + goto ret; } /* `operator=' is not an inheritable operator. */ @@ -7717,7 +7780,7 @@ cp_build_modify_expr (location_t loc, tr complain); if (result == NULL_TREE) return error_mark_node; - return result; + goto ret; } lhstype = olhstype; } @@ -7762,7 +7825,7 @@ cp_build_modify_expr (location_t loc, tr { result = objc_maybe_build_modify_expr (lhs, newrhs); if (result) - return result; + goto ret; } } gcc_assert (TREE_CODE (lhstype) != REFERENCE_TYPE); @@ -7858,9 +7921,10 @@ cp_build_modify_expr (location_t loc, tr from_array = TREE_CODE (TREE_TYPE (newrhs)) == ARRAY_TYPE ? 1 + (modifycode != INIT_EXPR): 0; - return build_vec_init (lhs, NULL_TREE, newrhs, - /*explicit_value_init_p=*/false, - from_array, complain); + result = build_vec_init (lhs, NULL_TREE, newrhs, + /*explicit_value_init_p=*/false, + from_array, complain); + goto ret; } if (modifycode == INIT_EXPR) @@ -7899,7 +7963,7 @@ cp_build_modify_expr (location_t loc, tr result = objc_generate_write_barrier (lhs, modifycode, newrhs); if (result) - return result; + goto ret; } result = build2 (modifycode == NOP_EXPR ? MODIFY_EXPR : INIT_EXPR, @@ -7909,6 +7973,9 @@ cp_build_modify_expr (location_t loc, tr if (!plain_assign) TREE_NO_WARNING (result) = 1; + ret: + if (preeval) + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), preeval, result); return result; } --- gcc/testsuite/g++.dg/cpp1z/eval-order4.C.jj 2017-01-30 16:08:22.195641383 +0100 +++ gcc/testsuite/g++.dg/cpp1z/eval-order4.C 2017-01-30 16:08:09.000000000 +0100 @@ -0,0 +1,80 @@ +// PR c++/79232 +// { dg-do run } +// { dg-options "-fstrong-eval-order" } + +int last = 0; + +int +foo (int i) +{ + if (i != last + 1) + __builtin_abort (); + last = i; + return i; +} + +char a, b; +int c; + +char & +bar (int i, int j) +{ + foo (i); + return j ? a : b; +} + +int +main () +{ + (foo (2) ? bar (3, 0) : bar (3, 1)) = foo (1); + if (last != 3) + __builtin_abort (); + last = 0; + (foo (2), foo (3) ? bar (4, 0) : bar (4, 1)) = foo (1); + if (last != 4) + __builtin_abort (); + last = 0; + (foo (2), (foo (3) ? bar (4, 0) : bar (4, 1))) = foo (1); + if (last != 4) + __builtin_abort (); + last = 0; + (foo (2), foo (3), foo (4) ? bar (5, 0) : bar (5, 1)) = foo (1); + if (last != 5) + __builtin_abort (); + last = 0; + (foo (2), (foo (3), (foo (4) ? bar (5, 0) : bar (5, 1)))) = foo (1); + if (last != 5) + __builtin_abort (); + last = 0; + --c = foo (1); + if (c != 1) + __builtin_abort (); + last = 0; + (foo (2), --c) = foo (1); + if (last != 2 || c != 1) + __builtin_abort (); + last = 0; + (foo (2), foo (3), --c) = foo (1); + if (last != 3 || c != 1) + __builtin_abort (); + last = 0; + (foo (2), (foo (3), --c)) = foo (1); + if (last != 3 || c != 1) + __builtin_abort (); + last = 0; + bar (2, 0) = foo (1); + if (last != 2) + __builtin_abort (); + last = 0; + (foo (2), bar (3, 0)) = foo (1); + if (last != 3) + __builtin_abort (); + last = 0; + (foo (2), foo (3), bar (4, 0)) = foo (1); + if (last != 4) + __builtin_abort (); + last = 0; + (foo (2), (foo (3), bar (4, 0))) = foo (1); + if (last != 4) + __builtin_abort (); +} --- gcc/testsuite/g++.dg/other/pr79232.C.jj 2017-01-30 13:37:32.095090643 +0100 +++ gcc/testsuite/g++.dg/other/pr79232.C 2017-01-30 13:35:17.000000000 +0100 @@ -0,0 +1,12 @@ +// PR c++/79232 +// { dg-do compile } + +extern char a[]; +int b; +char c, e; + +void +foo (long d) +{ + (0, b ? &c : a)[d] = e; +}