Sequence Points and Method Chaining reloaded

Question

I've read this:

• Undefined behavior and sequence points
• Undefined behavior and sequence points reloaded
• Sequence Points and Method Chaining
• GCC bug? Chaining methods, broken sequence point

...but I'm still not sure how this should behave:

int should_be_zero
    = stream.seek(4).read_integer()
    - stream.seek(4).read_integer();

Stream::seek() returns *this and seek/read_integer() respectively call fseek/fread on some FILE*.

This should return 0 like this:

1. stream.seek(4)
2. stream.read_integer() (at position 4, returns X, stream position advanced to 8)
3. stream.seek(4)
4. stream.read_integer() (at position 4, returns Y == X)
5. X - Y == 0

This worked well for me on gcc, MinGW and MinGW-w64. But when I decided to extend compiler support for MSVC, I discovered that this doesn't work anymore and returns garbage values. Here's what actually happens on MSVC:

1. stream.seek(4)
2. stream.seek(4) (again)
3. stream.read_integer() (at position 4, returns X, stream position advanced to 8)
4. stream.read_integer() (at position 8, returns Y != X)
5. X - Y != 0

Is such execution order well defined? If not, how can I protect myself against shooting myself in the foot like this in the future?

(Wrapping the calls with brackets doesn't seem to do anything.)

Answer 1

The internal order of execution within an expression is not defined. Only the obvious behavior of operator precedence is defined.

So, in this case, the compiler is obliged to call stream.seek(4) twice [unless the compiler figures out that it's "the same result either way"] and stream.read_integer() twice. But the order of those calls is undetermined (or whatever the term is in the C++ standard) - in other words, the compiler can order those four calls any way it likes.

Your code would be even more risky if you did something like:

 int x
    = stream.seek(4).read_integer()
    - stream.read_integer();  

since it's not well defined which of the two reads happen in which order now - it could call the second read_integer first (at offset 0) or after the seek and read at offset 8. Nobody knows which, and the compiler may even re-arrange them if you make subtle changes to the code (e.g. it decides to do things in a diffferent order because you added another variable that uses another register -> re-arrange code to use registers better...)

The solution is to introduce intermediate variables:

int a = stream.seek(4).read_integer();
int b = stream.seek(4).read_integer();

int should_be_zero = a - b; // Or b - a, if that's what you want... :)

This should be done in every piece of code where the order of execution is important for the correctness of the code - and bear in mind that "side-effects" (such as reading input, writing output, modifying state) are definitely dependent on order of execution.