[PATCH] D149550: [clang][Interp] Fix compound assign operator evaluation order

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:683-685
+  // C++17 onwards require that we evaluate the RHS first.
+  // Compute RHS and save it in a temporary variable so we can
+  // load it again later.
----------------
tbaeder wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > In C, the evaluation of the operands are unsequenced. C doesn't currently have constexpr functions, only constexpr objects, but that eliminates mutating operations like compound assignment... for the moment. Perhaps a FIXME comment for figuring out how to handle C?
> > > 
> > > (The situation I'm worried about in C is with UB dealing with unsequenced operations, like rejecting: https://godbolt.org/z/W11jchrKc)
> > Could C make them sequenced when introducing constexpr functions? :)
> Since we already emit a warning for this, we could in the future just check if the statement is in a constexpr function and emit an error instead? We're emitting the warning for c++ pre-17 as well but we don't make it an error, I guess because it's not UB there?
If we're actually leaving the operations unsequenced before C++17, then we should reject that code because it is UB: http://eel.is/c++draft/basic#intro.execution-10

The wording in C++14 for assignment operations is:
> 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.

So the left and right operands are unsequenced relative to one another.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149550/new/

https://reviews.llvm.org/D149550