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
This commit is contained in:
parent
d0cf395a99
commit
65a550b46e
@ -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.
|
||||
|
@ -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;
|
||||
|
@ -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
|
||||
|
@ -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;
|
||||
|
Loading…
Reference in New Issue
Block a user