C++ expression evaluation order

Question

i ran into a curious problem regarding evaluation of expressions:

reference operator()(size_type i, size_type j) {
  return by_index(i, j, index)(i, j); // return matrix index reference with changed i, j
}

matrix& by_index(size_type &i, size_type &j, index_vector &index) {
  size_type a = position(i, index); // find position of i using std::upper_bound
  size_type b = position(j, index);
  i -= index[a];
  j -= index[b];
  return matrix_(a,b); // returns matrix reference stored in 2-D array
}

I have thought matrix(i,j) will be evaluated after the call to buy_index, so that i, j will be updated. this appears to be correct, i verified in debugger. however, for some types of matrix, specifically those which have to cast size_type the something else, for example int, the update in by_index is lost. modifying code slightly removes the problem:

reference operator()(size_type i, size_type j) {
  matrix &m = by_index(i, j, index);
  return m(i, j); 
}

do you know why the first operator misbehaves? thanks

prototypes which work and which do not

inline reference operator () (size_t i, size_t j); // ublas, size_type is std::size_t
reference operator () (int i, int j); // other prototype, size_type is int

in debugger backtrace stack looks like this:

i = 1 upon entry to operator() //okay
i = 0 after finish from by_index //okay
i = 1 upon entry to matrix:: operator() //not right, should be 0

Can you give an example of some code that has to cast the size_type. I'm a little confused, but I think that is where your problem lies. (Something with casting messing up your passes-by-reference.) — SoapBox, Jan 21 '10 at 04:05
what is the definition of matrix_(a,b) ? I presume it returns a matrix&, but where does it get it from? — John Weldon, Jan 21 '10 at 04:06
for that matter, what is the definition of position(i, index)? — John Weldon, Jan 21 '10 at 04:08
I put comments explaining functions, they take arguments by value or using const reference. they should not have side effects. — Anycorn, Jan 21 '10 at 04:18

score 3 · Accepted Answer · answered Jan 21 '10 at 05:25

3

In my opinion, this boils down to order of evaluation.

The standard says -

(5.4) Except where noted, the order of evaluation of operands of individual operators and subexpressions of individual expressions, and the order in which side effects take place, is unspecified.

Which fits the bill exactly. The values of i and j may be evaluated before the call to by_index(), or after it. You can't tell - this is unspecified.

I will add that the form that solves your problem is far more readable in my eyes, and I would have used it regardless of correctness of the first form...

answered Jan 21 '10 at 05:25

Hexagon

6,845
2
24
16

okay, that is what i started thinking after the problem.before that I have thought the order was guaranteed. – Anycorn Jan 21 '10 at 05:33
@jk, Actually, I doubt sequence points are relevant here. There *is* a sequence point right after returning from the by_index() function call. But the evaluation of the parameters themselves happen regardless of sequence points. – Hexagon Jan 22 '10 at 09:28

score 2 · Answer 2 · edited May 23 '17 at 11:46

2

I suspect that casting a reference to a different type breaks strict aliasing rules that your compiler uses to optimize more efficiently. You have two variables/references of different type and the compiler assumes that they don't refer to the same memory (but which they in fact do). The compiler then optimizes the code under that wrong assumption which produces wrong results.

You can try to compile with -fno-strict-aliasing (or equivalent) to disable these optimizations and see if it improves the situation.

edited May 23 '17 at 11:46

Community

1
1

answered Jan 21 '10 at 04:31

sth

222,467
53
283
367

have been compiling using g++ -O0 -Wall. I have thought that disables all potential dangers optimizations. will try your suggestion – Anycorn Jan 21 '10 at 04:57
Hmmm, -O0 should have turned it off, so probably this is not the solution. Would have fit the symptoms, though. – sth Jan 21 '10 at 05:03

sth · Answer 3 · 2010-01-21T15:44:19.143

Finally I found where in the standard this is specified (n1905 draft):

(5.2.2-8) - The order of evaluation of arguments is unspeciﬁed. All side effects of argument expression evaluations take effect before the function is entered. The order of evaluation of the postﬁx expression and the argument expression list is unspeciﬁed.

The postfix expression mentioned is the part to the left of (). So in the "outer" function call it is not specified if by_index(i, j, index) or it's arguments (i, j) are evaluated first.

There is a sequence point after a function returns, so when by_index(i, j, index) returns all side effects are complete, but the (i, j) parameters might already have been evaluated (and the values been stored in a register or sth.) before that function even go called.

score 0 · Answer 4 · answered Jan 21 '10 at 05:24

Passing data through argument lists like that is very, very, unclear. Relying on implicit casts between reference types to different-sized bases is very, very unsafe.

Anyway, if you're calling by_index(size_t &,… with an int argument, you're taking the reference of a temporary. This should be a warning, but maybe you're on an older compiler. Try an explicit cast.

reinterpret_cast< size_t & >( i ) /* modify i before next fn call */

But of course that's not guaranteed to do the right thing. Really, you need to sort out unsigned from signed and not assume sizeof(int) == sizeof(size_t) because often it isn't.

score 0 · Answer 5 · answered Jul 14 '11 at 16:58

As an additional remark, this is the typical case where been concise overrules clarity, something Brian Kernighan strongly advises us to avoid (He wrote an excellent book on these matters, "The Practice of Programming"). The evaluation order is not well defined in such code, what leads to the "side effect" of unpredictable results. The change you have made is the recommended approach to situations like this one.

C++ expression evaluation order

5 Answers5