The C++ sequence point rules for compound assignment (+= and such) are
slightly different from those for C: the value of the LHS is not loaded
until after the RHS value (and LHS location) are evaluated. Since we
don't have tree codes for compound assignment, I need to handle this in
C++ by preevaluating the RHS into a temporary. Doing this breaks
various gomp tests because the logic for recognizing compound assignment
gets confused by the COMPOUND_EXPR, so I've adjusted the appropriate omp
code appropriately.
Tested x86_64-pc-linux-gnu, OK for trunk?
commit cec1fed71c8cf44b26fbb80546b1c2dd3780ebe1
Author: Jason Merrill <ja...@redhat.com>
Date: Fri Jul 8 14:07:23 2011 -0400
PR c++/45437
gcc/
* gimplify.c (goa_stabilize_expr): Handle RHS preevaluation in
compound assignment.
gcc/c-family/
* c-omp.c (check_omp_for_incr_expr): Handle preevaluation.
gcc/cp/
* typeck.c (cp_build_modify_expr): Preevaluate RHS.
diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index 1ee0bf0..1c5415a 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -213,6 +213,29 @@ check_omp_for_incr_expr (location_t loc, tree exp, tree decl)
return fold_build2_loc (loc, PLUS_EXPR,
TREE_TYPE (exp), TREE_OPERAND (exp, 0), t);
break;
+ case COMPOUND_EXPR:
+ {
+ /* cp_build_modify_expr forces preevaluation of the RHS to make
+ sure that it is evaluated before the lvalue-rvalue conversion
+ is applied to the LHS. Reconstruct the original expression. */
+ tree op0 = TREE_OPERAND (exp, 0);
+ tree op1 = TREE_OPERAND (exp, 1);
+ if (TREE_CODE (op0) == CONVERT_EXPR)
+ op0 = TREE_OPERAND (op0, 0);
+ if (TREE_CODE (op0) == TARGET_EXPR
+ && !VOID_TYPE_P (TREE_TYPE (op0)))
+ {
+ tree temp = TARGET_EXPR_SLOT (op0);
+ if (TREE_CODE_CLASS (TREE_CODE (op1)) == tcc_binary
+ && TREE_OPERAND (op1, 1) == temp)
+ {
+ op1 = copy_node (op1);
+ TREE_OPERAND (op1, 1) = TARGET_EXPR_INITIAL (op0);
+ return check_omp_for_incr_expr (loc, op1, decl);
+ }
+ }
+ break;
+ }
default:
break;
}
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5febff5..1479092 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6663,6 +6663,8 @@ cp_build_modify_expr (tree lhs, enum tree_code modifycode, tree rhs,
}
else
{
+ tree init = NULL_TREE;
+
/* A binary op has been requested. Combine the old LHS
value with the RHS producing the value we should actually
store into the LHS. */
@@ -6670,7 +6672,17 @@ cp_build_modify_expr (tree lhs, enum tree_code modifycode, tree rhs,
&& MAYBE_CLASS_TYPE_P (TREE_TYPE (lhstype)))
|| MAYBE_CLASS_TYPE_P (lhstype)));
+ /* Preevaluate the RHS to make sure its evaluation is complete
+ before the lvalue-to-rvalue conversion of the LHS:
+
+ [expr.ass] With respect to an indeterminately-sequenced
+ function call, the operation of a compound assignment is a
+ single evaluation. [ Note: Therefore, a function call shall
+ not intervene between the lvalue-to-rvalue conversion and the
+ side effect associated with any single compound assignment
+ operator. -- end note ] */
lhs = stabilize_reference (lhs);
+ rhs = stabilize_expr (rhs, &init);
newrhs = cp_build_binary_op (input_location,
modifycode, lhs, rhs,
complain);
@@ -6682,6 +6694,9 @@ cp_build_modify_expr (tree lhs, enum tree_code modifycode, tree rhs,
return error_mark_node;
}
+ if (init)
+ newrhs = cp_build_compound_expr (init, newrhs, complain);
+
/* Now it looks like a plain assignment. */
modifycode = NOP_EXPR;
if (c_dialect_objc ())
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 4ff7e93..5a928be 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6451,6 +6451,13 @@ goa_stabilize_expr (tree *expr_p, gimple_seq *pre_p, tree lhs_addr,
saw_lhs |= goa_stabilize_expr (&TREE_OPERAND (expr, 0), pre_p,
lhs_addr, lhs_var);
break;
+ case COMPOUND_EXPR:
+ /* Break out any preevaluations from cp_build_modify_expr. */
+ for (; TREE_CODE (expr) == COMPOUND_EXPR;
+ expr = TREE_OPERAND (expr, 1))
+ gimplify_stmt (&TREE_OPERAND (expr, 0), pre_p);
+ *expr_p = expr;
+ return goa_stabilize_expr (expr_p, pre_p, lhs_addr, lhs_var);
default:
break;
}
diff --git a/gcc/testsuite/g++.dg/expr/compound-asn1.C b/gcc/testsuite/g++.dg/expr/compound-asn1.C
new file mode 100644
index 0000000..194235c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/compound-asn1.C
@@ -0,0 +1,15 @@
+// PR c++/45437
+// { dg-options -Wsequence-point }
+// { dg-do run }
+
+bool f(bool& b) {
+ b = true;
+ return false;
+}
+
+int main() {
+ bool b = false;
+ b |= f(b);
+ if (!b)
+ return 1;
+}
diff --git a/gcc/testsuite/g++.dg/warn/sequence-pt-1.C b/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
index 05eee82..6a98fd7 100644
--- a/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
+++ b/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
@@ -62,7 +62,7 @@ foo (int a, int b, int n, int p, int *ptr, struct s *sptr,
(a = a++) && b; /* { dg-warning "undefined" "sequence point warning" } */
b, (a = a++); /* { dg-warning "undefined" "sequence point warning" } */
(a = a++), b; /* { dg-warning "undefined" "sequence point warning" } */
- a ^= b ^= a ^= b; /* { dg-warning "undefined" "sequence point warning" } */
+ a ^= b ^= a ^= b; /* { dg-bogus "undefined" "sequence point warning" } */
a = a; /* { dg-bogus "undefined" "bogus sequence point warning" } */
a = (a++ && 4); /* { dg-bogus "undefined" "bogus sequence point warning" } */