[clang][Interp] Fix assignment operator call eval order

[tbaederr]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/101845.diff

2 Files Affected:

• (modified) clang/lib/AST/Interp/Compiler.cpp (+24-2)
• (modified) clang/test/AST/Interp/eval-order.cpp (+5-16)

diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index d9db1c788314c..bd2b0f74b34c5 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -3977,7 +3977,19 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
     }
   }
 
-  auto Args = llvm::ArrayRef(E->getArgs(), E->getNumArgs());
+  SmallVector<const Expr *, 8> Args(
+      llvm::ArrayRef(E->getArgs(), E->getNumArgs()));
+
+  bool IsAssignmentOperatorCall = false;
+  if (const auto *OCE = dyn_cast<CXXOperatorCallExpr>(E);
+      OCE && OCE->isAssignmentOp()) {
+    // Just like with regular assignments, we need to special-case assignment
+    // operators here and evaluate the RHS (the second arg) before the LHS (the
+    // first arg. We fix this by using a Flip op later.
+    assert(Args.size() == 2);
+    IsAssignmentOperatorCall = true;
+    std::reverse(Args.begin(), Args.end());
+  }
   // Calling a static operator will still
   // pass the instance, but we don't need it.
   // Discard it here.
@@ -3986,7 +3998,8 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
         MD && MD->isStatic()) {
       if (!this->discard(E->getArg(0)))
         return false;
-      Args = Args.drop_front();
+      // Drop first arg.
+      Args.erase(Args.begin());
     }
   }
 
@@ -4038,6 +4051,15 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
     ++ArgIndex;
   }
 
+  // Undo the argument reversal we did earlier.
+  if (IsAssignmentOperatorCall) {
+    assert(Args.size() == 2);
+    PrimType Arg1T = classify(Args[0]).value_or(PT_Ptr);
+    PrimType Arg2T = classify(Args[1]).value_or(PT_Ptr);
+    if (!this->emitFlip(Arg2T, Arg1T, E))
+      return false;
+  }
+
   if (FuncDecl) {
     const Function *Func = getFunction(FuncDecl);
     if (!Func)
diff --git a/clang/test/AST/Interp/eval-order.cpp b/clang/test/AST/Interp/eval-order.cpp
index c78c5061a08f2..213ef209a1c04 100644
--- a/clang/test/AST/Interp/eval-order.cpp
+++ b/clang/test/AST/Interp/eval-order.cpp
@@ -1,13 +1,7 @@
 // RUN: %clang_cc1 -std=c++1z -verify=ref,both %s -fcxx-exceptions -triple=x86_64-linux-gnu
 // RUN: %clang_cc1 -std=c++1z -verify=expected,both %s -fcxx-exceptions -triple=x86_64-linux-gnu -fexperimental-new-constant-interpreter
 
-// ref-no-diagnostics
-
-/// Check that assignment operators evaluate their operands right-to-left.
-/// Copied from test/SemaCXX/constant-expression-cxx1z.cpp
-///
-/// As you can see from the FIXME comments, some of these are not yet working correctly
-/// in the new interpreter.
+// both-no-diagnostics
 namespace EvalOrder {
   template<typename T> struct lvalue {
     T t;
@@ -45,7 +39,7 @@ namespace EvalOrder {
     }
     template <typename T> constexpr T &&b(T &&v) {
       if (!done_a)
-        throw "wrong"; // expected-note 3{{not valid}}
+        throw "wrong";
       done_b = true;
       return (T &&)v;
     }
@@ -79,15 +73,10 @@ namespace EvalOrder {
 
   // Rule 5: b = a, b @= a
   SEQ(B(lvalue<int>().get()) = A(0));
-  SEQ(B(lvalue<UserDefined>().get()) = A(ud)); // expected-error {{not an integral constant expression}} FIXME \
-                                               // expected-note 2{{in call to}}
+  SEQ(B(lvalue<UserDefined>().get()) = A(ud));
   SEQ(B(lvalue<int>().get()) += A(0));
-  SEQ(B(lvalue<UserDefined>().get()) += A(ud)); // expected-error {{not an integral constant expression}} FIXME \
-                                                // expected-note 2{{in call to}}
-
-  SEQ(B(lvalue<NonMember>().get()) += A(nm)); // expected-error {{not an integral constant expression}} FIXME \
-                                              // expected-note 2{{in call to}}
-
+  SEQ(B(lvalue<UserDefined>().get()) += A(ud));
+  SEQ(B(lvalue<NonMember>().get()) += A(nm));
 
   // Rule 6: a[b]
   constexpr int arr[3] = {};

[tschuett]

No review?

[tbaederr]

I have post-review powers for changes in Interp/.