Does using postincrement within a fold-expression give unsequenced behaviour?

166 Views Asked by At

Recently I've came up with an idea how enumerate elements in a parameter pack (Pulses) and I've been happy with the solution for a while:

lastValue = 0.0; i = -1;
lastValue += (... + ((time <= endtimes[++i] && time >= delays[i]) ? static_cast<Pulses*>(fields[i])->operator()(time - delays[i]) : 0.0));

however now as I've tried to compile with Clang instead of GCC, I got the following warning

warning: multiple unsequenced modifications to 'i' [-Wunsequenced]

I've read a couple of posts about unsequenced modifications, but in their examples the modification either happens two times in an expression or on left side of the "=" operator.

On the other hand my understanding is that parameter packs are evaluated sequentially, so... no unsequenced behaviour right? Or am I dead wrong here and the compiler is always right?

2

There are 2 best solutions below

2
On BEST ANSWER

Clang is right, the code causes UB.

Evaluation rules for fold expressions are exactly the same as for normal expressions.

Evaluations of the two operands of + (and of most binary operators) is unsequenced relative to one another (i.e. can happen in any order, possibly interleaved), and unsequenced changes of scalar variables cause UB.

The solution is simple: use operator , for folding, instead of +. For ,, the first operand is fully evaluated before the second one, so there's a clear evaluation order and no UB.

lastValue = 0.0; i = -1;
((lastValue += ((time <= endtimes[++i] && time >= delays[i]) ? static_cast<Pulses*>(fields[i])->operator()(time - delays[i]) : 0.0)), ...);

Or, use a lambda to make it less ugly:

lastValue = 0.0; i = 0;
([&]{
    if (time <= endtimes[i] && time >= delays[i])
        lastValue += static_cast<Pulses*>(fields[i])->operator()(time - delays[i]);
    i++;
}(), ...);
1
On

Well, the compiler is always right, even when it’s wrong…

Unless i is used in the … that you removed, I can’t see what’s wrong. On the other hand, the expression is overly complex and could do with being split up, even just to make debugging easier.