37

I keep finding more idioms that lend themselves to std::exchange.

Today I found myself writing this in an answer:

do {
    path.push_front(v);
} while (v != std::exchange(v, pmap[v]));

I like it a lot more than, say

do {
    path.push_front(v);
    if (v == pmap[v])
        break;
    v= pmap[v];
} while (true);

Hopefully for obvious reasons.

However, I'm not big on standardese and I can't help but worry that lhs != rhs doesn't guarantee that the right-hand side expression isn't fully evaluated before the left-hand-side. That would make it a tautologous comparison - which would by definition return true.

The code, however, does run correctly, apparently evaluating lhs first.

Does anyone know

  • whether the standard guarantees this evaluation order
  • if it has changed in recent standards, which standard version first specified it?

PS. I realize that this is a special case of f(a,b) where f is operator!=. I've tried to answer my own query using the information found here but have failed to reach a conclusion to date:

Vadim Beskrovnov
  • 941
  • 6
  • 18
sehe
  • 374,641
  • 47
  • 450
  • 633
  • @SebastianRedl I agree. But only IFF that evaluation order isn't specified. As an off-topic prompt, do you have an elegant alternative suggestion? The reason that I came up with this version is that all other incarnations of this kind of loop that I've seen over many years are significantly worse. – sehe Nov 28 '22 at 14:06
  • 1
    Mmm... might be well formed if `operator !=` is a member https://timsong-cpp.github.io/cppwp/n4868/expr.call#8 - It's certainly not more complex than the standard's own example – StoryTeller - Unslander Monica Nov 28 '22 at 14:08
  • 1
    I do not see any wording that mandates that the left side of `!=` be sequenced before the right. C++17 added sequencing for some operations, but `!=` does not appear to be among them. – Raymond Chen Nov 28 '22 at 14:10
  • *Hopefully for obvious reasons.* No way! You can never generalise, but clearer is always better (and especially safer). Ah, OK! Unless you meant the double `pmap[v]` :) – rturrado Nov 28 '22 at 14:29
  • 1
    @rturrado I like to think that the loop is a lot clearer with the "atomic" (as in, single-statement) exchange. But yeah, it appears to be safer without. Which IMHO is the non-obvious part. The only reason I had the danger sensors going off is because I've learned painful C++ lessons in the past. But I expect the average programmer to have the same, completely intuitive expectation of what that code should do. – sehe Nov 28 '22 at 14:49
  • 2
    @rturrado And yes, the double `pmap[v]` can be circumvented by adding a named variable. Alas, there is no way to constrain the scope of said variable. Re-writing as a for-loop (which is the usual) requires either making the push operation a side effect of the condition (which is objectively worse because that operation doesn't already exist with well-known semantics, unlike `std::exchange`) or ... duplicating that outside of the loop body... It's catch-22 – sehe Nov 28 '22 at 14:49
  • 2
    [FWIW](https://godbolt.org/z/9bPr8Koza). – n. m. could be an AI Nov 28 '22 at 15:41
  • Thanks @n.m. I did do some checks myself, however the question is indeed focused on the standard guarantees. I [fixed the implementation defined behaviour in the answer that prompted this](https://stackoverflow.com/posts/74600853/revisions) – sehe Nov 28 '22 at 15:46
  • Maybe `std::exchange` is the wrong tool, here. Have you considered something like: https://godbolt.org/z/MjqcrKWz3 ? – Bob__ Nov 28 '22 at 16:19
  • @Bob__ Yeah. The appeal `std::exchange` is that it is standardized, with _well-known semantics_. Although one can come up with bespoke primitives, they won't really be as generally applicable - or it will never be as intuitive. (FP ecosystems like Haskell are bigger here, largely because immutability everywhere makes it easier to have "obvious" semantics). For comparison the iterations that I posted here: https://fosstodon.org/@sehe/109422130042322033 – sehe Nov 28 '22 at 16:33
  • Isn't that `do { path.push_front(std::exchange(v, pmap[v])) while (v != pmap[v])`? – Jeff Garrett Nov 28 '22 at 19:52
  • @JeffGarrett Gosh. I think it might be indeed. Downside is that `pmap[v]` is evaluated twice, which might be significant. But the elegance is wort a lot. It's subtle in that it's the condition seems "too late" (the exchange has already been performed). This is why I stopped looking at this solution direction. However, it's actually fine, because when `v == pmap[v]` it becomes idempotent by definition. Not bad! – sehe Nov 28 '22 at 22:23
  • @JeffGarrett Sadly it can't work out ([_obviously_](https://stackoverflow.com/questions/74601619/order-of-evaluation-in-v-stdexchangev-predecessorv/74607694#74607694:~:text=requires%20either%20making%20the%20push%20operation%20a%20side%20effect%20of%20the%20condition%20(which%20is%20objectively%20worse%20because%20that%20operation%20doesn%27t%20already%20exist%20with%20well%2Dknown%20semantics%2C%20unlike%20std%3A%3Aexchange)%20or%20...%20duplicating%20that%20outside%20of%20the%20loop%20body)) because it omits a node from the path sometimes: http://coliru.stacked-crooked.com/a/d1d91fb0f13b7df2 – sehe Nov 29 '22 at 02:23
  • Indeed. Compared to next instead of previous. Then I don't have a better one. :) – Jeff Garrett Nov 29 '22 at 03:14

3 Answers3

20

C++17 introduced rules on sequences. What was UB before is now well defined. This applies to arguments to function calls as well as a select assortment of operators:

sequenced before is an asymmetric, transitive, pair-wise relationship between evaluations within the same thread.

  • If A is sequenced before B (or, equivalently, B is sequenced after A), then evaluation of A will be complete before evaluation of B begins.

The built-in != however is not sequenced (see link above). A function call would be sequenced but the order of evaluation is not guaranteed:

  1. In a function call, value computations and side effects of the initialization of every parameter are indeterminately sequenced with respect to value computations and side effects of any other parameter.

(emphasis added)

To my reading, even if you wrote a wrapper function, your compiler would not be required to evaluate v first, then std::exchange(v, pmap[v]) and finally equal(..). And reversing the evaluation order, I believe, would change semantics in your example.

So sadly, as nice as std::exchange is, in this case, it is not guaranteed to do what you need it to.

bitmask
  • 32,434
  • 14
  • 99
  • 159
  • Edit: Sorry I misread a point. Corrected answer. – bitmask Nov 28 '22 at 14:17
  • 1
    Ha. I had [that quote](https://en.cppreference.com/w/cpp/language/eval_order#:~:text=In%20a%20function%20call%2C%20value%20computations%20and%20side%20effects%20of%20the%20initialization%20of%20every%20parameter%20are%20indeterminately%20sequenced%20with%20respect%20to%20value%20computations%20and%20side%20effects%20of%20any%20other%20parameter.) copied to comment just before you realized and started editing. Kinda sad. It looks like this is the state of affairs ¯\\_(ツ)_/¯ – sehe Nov 28 '22 at 14:18
  • @sehe Yes, my brain somehow parsed the word "indeterminately" as "determinately". :) – bitmask Nov 28 '22 at 14:19
  • It's unclear to me why the committee didn't follow through with sequencing and make it mandatory to be done in order. What possible optimisation could there be in allowing the compiler to choose evaluation order? Oh well, I'm not a compiler dev, so what do I know. – bitmask Nov 28 '22 at 14:21
  • 3
    @bitmask different calling conventions have different preferences for order of evaluation. One of the historically most popular is stack-based cdecl, which prefers right to left. – Raymond Chen Nov 28 '22 at 14:23
  • 1
    @bitmask A long time ago, [Hyman Rosen](https://stackoverflow.com/users/197411/hyman-rosen) wrote a [paper](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0431r0.htm) to mandate a strict right-to-left order of evaluation. That paper unfortunately went nowhere. – ForeverLearning Nov 28 '22 at 15:52
16

If != calls an overloaded comparison operator that takes the left argument (v) by reference, the order of evaluation here does not really matter: binding a reference (directly) does not access the object denoted by the initializer. The actual comparison (in the body of the operator function) takes place after both operands are evaluated ([intro.execution]/11):

When calling a function [...], every value computation and side effect associated with any argument expression [...] is sequenced before execution of every expression or statement in the body of the called function.

In other words, the comparison is guaranteed behave as you expect here.


This does not hold if evaluating the left side involves reading the value of v, as in the case of the built-in != operator, or an overload taking it by value (or requiring a conversion to bind a reference).

In that case, it becomes significant that the two operands are unsequenced ([intro.execution]/10):

Except where noted, evaluations of operands of individual operators and of subexpressions of individual expressions are unsequenced.

(The != operator does not have any special sequencing properties.)

There is potential for UB here:

If a side effect on a memory location is unsequenced relative to either another side effect on the same memory location or a value computation using the value of any object in the same memory location, and they are not potentially concurrent, the behavior is undefined.

However, [intro.execution]/11 applies:

For each function invocation or evaluation of an await-expression F, each evaluation that does not occur within F but is evaluated on the same thread and as part of the same signal handler (if any) is either sequenced before all evaluations that occur within F or sequenced after all evaluations that occur within F.

Summarized by a footnote:

In other words, function executions do not interleave with each other.

Which means the read of v on the left and its modification on the right (which occurs within std::exchange) can't conflict, and are effectively indeterminately sequenced: there's no UB, but no guarantee of consistent results either.


C++17 made some changes to the rules of evaluation order, but none are relevant here: this answer holds pre-C++17 as well.

duck
  • 1,455
  • 2
  • 8
  • Nice sources. It looks like c++17 made it unspecified rather than UB. That's an important difference. Is there a reason you call it UB? – sehe Nov 29 '22 at 11:27
2

Regardless of whether or not this works, it is almost completely unreadable code -- and remember that we write code mostly for humans, not compilers!

As a general rule, using conditionals that have side effects are often hard to read and understand because our brains are not equipped to do two things at once: Understand whether the condition is true or false, and also keep track of what changes. Do one thing at a time if you want others to easily understand what is happening. In your case, you are adding a third dimension to it: when something changes; using conditionals in which you have a function call that changes something, and comparing the changed result against something else is just impossible to read :-)

If you absolutely must use this idiom, separate the three parts:

do {
    path.push_front(v);
} while ([&v,&pmap]() mutable
         {
           auto old_v = v;  
           v = pmap[v];
           return v != old_v;
         } ());

This code is equivalent, of course, but I think it is far easier to read. Although I will claim that it is still harder to read than your alternative code without using std::exchange() :-)

  • Thank you +1. I came up with [several versions not dissimilar to this one](https://fosstodon.org/@sehe/109422130042322033) (though I like them all a bit better than the form presented here). Love that you unironically wrote an immediately invoked lambda in the condition of a do-while loop in c++ and called it "easier to read" :=P My version was a Very Bad Idea in C++. Readability wasn't the bad part. You can see [which version I went with](https://stackoverflow.com/questions/74601619/order-of-evaluation-in-v-stdexchangev-predecessorv/74607694#comment131684717_74601619). – sehe Nov 29 '22 at 02:02
  • (By the way I like your reasoning about what makes code understandable. I'm *not* sure what you refer to with "if you absolutely must use _this idiom_" - it seems you kept only the requirements/behavior, and yeah, that's a given of course) – sehe Nov 29 '22 at 02:04
  • @sehe "this idiom" = "the `do`...while loop in which you do some work in the `while` condition". I wished that that could be done using a compound statement rather than having to declare then immediately invoke a lambda, but that's just how it is :-) – Wolfgang Bangerth Nov 29 '22 at 03:45
  • 2
    I also wished that the condition in the trailing `while` would be in scope of the body of the loop, because then one could have moved the declaration of `old_v` into the body of the loop and referenced it from the `while` condition. That would also have avoided the lambda function. But that too just is what it is :-) – Wolfgang Bangerth Nov 29 '22 at 03:46
  • 1
    `[&]{ auto old_v = std::exchange(v, pmap[v]); return v != old_v; }()` is a bit terser. – user3840170 Nov 29 '22 at 10:51