P0145R2: Refining Expression Order for C++ (assignment 2).

	* cp-gimplify.c (lvalue_has_side_effects): New.
	(cp_gimplify_expr): Implement assignment ordering.

From-SVN: r238177

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 13f80f9103f1c134229c954f0fa57ce4322f28ff..0d6ce94a4db98fda4d185f8281ee0e76f268fd03 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,6 +1,8 @@
 2016-07-08  Jason Merrill  <jason@redhat.com>
 
        P0145R2: Refining Expression Order for C++.
+       * cp-gimplify.c (lvalue_has_side_effects): New.
+       (cp_gimplify_expr): Implement assignment ordering.
        * call.c (op_is_ordered, build_over_call): Adjust for
        -fargs-in-order renaming to -fstrong-eval-order.
        * cp-gimplify.c (cp_gimplify_expr): Likewise.

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index c04368fc189cbf853bee0331cf11d71d236c7909..8496d7cfee70ac2436e0876b860adda7a352f3c5 100644 (file)
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -559,6 +559,33 @@ simple_empty_class_p (tree type, tree op)
     && is_really_empty_class (type);
 }
 
+/* Returns true if evaluating E as an lvalue has side-effects;
+   specifically, a volatile lvalue has TREE_SIDE_EFFECTS, but it doesn't really
+   have side-effects until there is a read or write through it.  */
+
+static bool
+lvalue_has_side_effects (tree e)
+{
+  if (!TREE_SIDE_EFFECTS (e))
+    return false;
+  while (handled_component_p (e))
+    {
+      if (TREE_CODE (e) == ARRAY_REF
+         && TREE_SIDE_EFFECTS (TREE_OPERAND (e, 1)))
+       return true;
+      e = TREE_OPERAND (e, 0);
+    }
+  if (DECL_P (e))
+    /* Just naming a variable has no side-effects.  */
+    return false;
+  else if (INDIRECT_REF_P (e))
+    /* Similarly, indirection has no side-effects.  */
+    return TREE_SIDE_EFFECTS (TREE_OPERAND (e, 0));
+  else
+    /* For anything else, trust TREE_SIDE_EFFECTS.  */
+    return TREE_SIDE_EFFECTS (e);
+}
+
 /* Do C++-specific gimplification.  Args are as for gimplify_expr.  */
 
 int
@@ -659,8 +686,6 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
            /* Remove any copies of empty classes.  Also drop volatile
               variables on the RHS to avoid infinite recursion from
               gimplify_expr trying to load the value.  */
-           gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
-                          is_gimple_lvalue, fb_lvalue);
            if (TREE_SIDE_EFFECTS (op1))
              {
                if (TREE_THIS_VOLATILE (op1)
@@ -669,8 +694,29 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
                gimplify_and_add (op1, pre_p);
              }
+           gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+                          is_gimple_lvalue, fb_lvalue);
            *expr_p = TREE_OPERAND (*expr_p, 0);
          }
+       /* P0145 says that the RHS is sequenced before the LHS.
+          gimplify_modify_expr gimplifies the RHS before the LHS, but that
+          isn't quite strong enough in two cases:
+
+          1) gimplify.c wants to leave a CALL_EXPR on the RHS, which would
+          mean it's evaluated after the LHS.
+
+          2) the value calculation of the RHS is also sequenced before the
+          LHS, so for scalar assignment we need to preevaluate if the
+          RHS could be affected by LHS side-effects even if it has no
+          side-effects of its own.  We don't need this for classes because
+          class assignment takes its RHS by reference.  */
+       else if (flag_strong_eval_order > 1
+                && TREE_CODE (*expr_p) == MODIFY_EXPR
+                && lvalue_has_side_effects (op0)
+               && (TREE_CODE (op1) == CALL_EXPR
+                   || (SCALAR_TYPE_P (TREE_TYPE (op1))
+                       && !TREE_CONSTANT (op1))))
+        TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (op1, pre_p);
       }
       ret = GS_OK;
       break;

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b7e9d2d1109a90c27f39f69c848133f8afa0e34d..ca8c1b43002c3f0659a32998af7e2673509d3f1d 100644 (file)
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4080,6 +4080,13 @@ diagnosed by this option, and it may give an occasional false positive
 result, but in general it has been found fairly effective at detecting
 this sort of problem in programs.
 
+The C++17 standard will define the order of evaluation of operands in
+more cases: in particular it requires that the right-hand side of an
+assignment be evaluated before the left-hand side, so the above
+examples are no longer undefined.  But this warning will still warn
+about them, to help people avoid writing code that is undefined in C
+and earlier revisions of C++.
+
 The standard is worded confusingly, therefore there is some debate
 over the precise meaning of the sequence point rules in subtle cases.
 Links to discussions of the problem, including proposed formal

diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
index d382d073aa6cdb85b3c5a39db4951548e38cca6c..e87dce4006c2a2b9a5393de9257baafd3e8867aa 100644 (file)
--- a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
+++ b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C
@@ -31,6 +31,13 @@ struct A
   A& operator+=(int i) { return *this; }
 };
 
+struct B
+{
+  int _i;
+  B(): _i(0) {}
+  B(int i): _i(f(i)) { }
+};
+
 int operator>>(A&, int i) { }
 
 A a(0);
@@ -46,6 +53,18 @@ A& aref(int i)
   return a;
 }
 
+B b;
+B bval(int i)
+{
+  return B(i);
+}
+
+B& bref(int i)
+{
+  f(i);
+  return b;
+}
+
 static int si;
 int* ip (int i)
 {
@@ -84,10 +103,18 @@ template <class T> void f()
 
   // b @= a
   aref(19)=A(18);
-  //iref(21)=f(20);
+  iref(21)=f(20);
   aref(23)+=f(22);
+  bref(25)=bval(24);
+  (f(27), b) = bval(26);
   last = 0;
 
+  int ar[4] = {};
+  int i = 0;
+  ar[i++] = i;
+  if (ar[0] != 0)
+    __builtin_abort();
+
   // a[b]
   afn(20)[f(21)-21].memfn(f(22),23);
   ip(24)[f(25)-25] = 0;
@@ -123,10 +150,18 @@ void g()
 
   // b @= a
   aref(19)=A(18);
-  //iref(21)=f(20);
+  iref(21)=f(20);
   aref(23)+=f(22);
+  bref(25)=bval(24);
+  (f(27), b) = bval(26);
   last = 0;
 
+  int ar[4] = {};
+  int i = 0;
+  ar[i++] = i;
+  if (ar[0] != 0)
+    __builtin_abort();
+
   // a[b]
   afn(20)[f(21)-21].memfn(f(22),23);
   ip(24)[f(25)-25] = 0;