Does this set of operators cause undefined behaviour?

Question

As far as I understand, things like ++i++ are not allowed in C/C++ because they break a rule that variables cannot be written to multiple times in a single statement. The result is undefined behaviour.

Could someone confirm if this also applies to my statement: ++i %= j?

I would like a for loop where I increment i around a circular buffer (size j) starting at an arbitrary point p until I get back to point p.

for(int i = p+1 ; i != p; ++i %= j  )
{
    if (buffer[i].ready()) 
    {
        buffer[i].do_something();
        p = i;
        break;
    }
}

I could do it in more lines of code, but would rather not.

Answer 1

In your code you say ++i % j, which means 'Increment i (and store the new value in i), then calculate the modulo of i and j. But that value is not stored anywhere.

To get a wrap around loop, you can use i = (i+1)%j

Answer 2

The behaviour is undefined pre C++17.

++i %= j is equivalent to i = ++i % j.

This is a dressed up version of i = ++i, and everyone knows that is UB.

Answer 3

Yes, your code is well defined (in C++17 anyway). Quoting the standard:

[expr.ass]

The assignment operator (=) and the compound assignment operators all group right-to-left. All require a modifiable lvalue as their left operand and return an lvalue referring to the left operand. The result in all cases is a bit-field if the left operand is a bit-field. In all cases, the assignment is sequenced after the value computation of the right and left operands, and before the value computation of the assignment expression. The right operand is sequenced before the left operand. With respect to an indeterminately-sequenced function call, the operation of a compound assignment is a single evaluation.

[expr.pre.incr]

The operand of prefix ++ is modified by adding 1. The operand shall be a modifiable lvalue. The type of the operand shall be an arithmetic type other than cv bool, or a pointer to a completely-defined object type. The result is the updated operand; it is an lvalue, and it is a bit-field if the operand is a bit-field. The expression ++x is equivalent to x+=1.

The text in bold means your code pretty much has the following semantics:

auto& __j = j;   // refer to j
auto& __i = ++i; // refer to i after the increment
__i %= __j;

If the expression looks fishy to you despite the standards reassurance, you can always sequence it yourself with the comma operator.

for(... ; ... ; (++i, i %= j))

Answer 4

The code ++i %= j is identical to the following code:

operator %= (++i, j);

In the standard (§1.9/15) it's stated that

The value computations of the operands of an operator are sequenced before the value computation of the result of the operator.

While "value computations" includes:

• value computations (including determining the identity of an object for glvalue evaluation and fetching a value previously assigned to an object for prvalue evaluation) and
• initiation of side effects.

So, here the compiler is forced to first calculate ++i and j (in any order), including the side-effect of ++, and call operator %= only after these calculations finished. So, it's a well-defined behavior since at least C++11.

Please see this answer for more details.

Answer 5

Others have already answered that your code is valid C++11 and later, but is undefined behavior for earlier versions of C++ and won't even compile in C (ignoring class member fcn calls, of course). All of which says you should probably use a more explicit form to achieve your desired end.

I also wanted to point out that you likely have a subtle bug in your code when p == j - 1 because you add one but don't mod when you initialize i, making you access beyond the end of the array in that case.

You also never process the element at index p. Is that really what you want? If you repeatedly executed this code and none of the other buffers were ready(), then you would keep skipping over checking if buffer[p].ready().

More correct, more universal code (that still doesn't check the element at index p):

int i;

for (i = (p + 1) % j; i != p && !buffer[i].ready(); ++i, i %= j);

if (i != p)
{
  buffer[i].do_something();
  p = i;
}

A version that starts by processing the element at index p, but stops after going over the array at most once:

int i = p;

while (!buffer[i].ready() && (++i, i %= j, i != p));

if (buffer[i].ready())
{
  buffer[i].do_something();
  p = i;
}

There is a subtle point about the above version. If !buffer[p].ready() when we first enter the loop, we go through the whole array, and none the other elements are ready() either, then we will exit the loop with i == p. We will then interrogate buffer[p] again asking if it is ready(). So, it is possible to ask buffer[p].ready() twice, which may or may not be important (e.g. - if it has side effects). Heads up!

Here's a version that avoids that issue, but checks buffer[p].ready() last:

int i = p;

while ((++i, i %= j, i != p) && !buffer[i].ready());

if (i != p || buffer[p].ready())
{
  buffer[i].do_something();
  p = i;
}