[clang] [clang][Interp] Fix array subscript eval order (PR #101804)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

Always evaluate LHS first, then RHS.

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


4 Files Affected:

- (modified) clang/lib/AST/Interp/Compiler.cpp (+17-7) 
- (modified) clang/lib/AST/Interp/Interp.h (+15) 
- (modified) clang/lib/AST/Interp/Opcodes.td (+5) 
- (modified) clang/test/AST/Interp/eval-order.cpp (+4-5) 


``````````diff
diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index f600d9b5b80f8..e1fa0eb1eacb3 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -1282,21 +1282,31 @@ bool Compiler<Emitter>::VisitImplicitValueInitExpr(
 
 template <class Emitter>
 bool Compiler<Emitter>::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
-  const Expr *Base = E->getBase();
+  const Expr *LHS = E->getLHS();
+  const Expr *RHS = E->getRHS();
   const Expr *Index = E->getIdx();
 
   if (DiscardResult)
-    return this->discard(Base) && this->discard(Index);
+    return this->discard(LHS) && this->discard(RHS);
 
-  // Take pointer of LHS, add offset from RHS.
-  // What's left on the stack after this is a pointer.
-  if (!this->visit(Base))
-    return false;
+  // C++17's rules require us to evaluate the LHS first, regardless of which
+  // side is the base.
+  bool Success = true;
+  for (const Expr *SubExpr : {LHS, RHS}) {
+    if (!this->visit(SubExpr))
+      Success = false;
+  }
 
-  if (!this->visit(Index))
+  if (!Success)
     return false;
 
   PrimType IndexT = classifyPrim(Index->getType());
+  // If the index is first, we need to change that.
+  if (LHS == Index) {
+    if (!this->emitFlip(PT_Ptr, IndexT, E))
+      return false;
+  }
+
   return this->emitArrayElemPtrPop(IndexT, E);
 }
 
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index b54635b9988e2..a3f81e2de466b 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1158,6 +1158,21 @@ bool Pop(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+/// [Value1, Value2] -> [Value2, Value1]
+template <PrimType TopName, PrimType BottomName>
+bool Flip(InterpState &S, CodePtr OpPC) {
+  using TopT = typename PrimConv<TopName>::T;
+  using BottomT = typename PrimConv<BottomName>::T;
+
+  const auto &Top = S.Stk.pop<TopT>();
+  const auto &Bottom = S.Stk.pop<BottomT>();
+
+  S.Stk.push<TopT>(Top);
+  S.Stk.push<BottomT>(Bottom);
+
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Const
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 3e830f89754dc..70d06bdfdc21c 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -729,6 +729,11 @@ def Dup : Opcode {
   let HasGroup = 1;
 }
 
+def Flip : Opcode {
+  let Types = [AllTypeClass, AllTypeClass];
+  let HasGroup = 1;
+}
+
 // [] -> []
 def Invalid : Opcode {}
 def Unsupported : Opcode {}
diff --git a/clang/test/AST/Interp/eval-order.cpp b/clang/test/AST/Interp/eval-order.cpp
index 7a7ce6a714601..77f50831f4f47 100644
--- a/clang/test/AST/Interp/eval-order.cpp
+++ b/clang/test/AST/Interp/eval-order.cpp
@@ -45,7 +45,7 @@ namespace EvalOrder {
     }
     template <typename T> constexpr T &&b(T &&v) {
       if (!done_a)
-        throw "wrong"; // expected-note 7{{not valid}}
+        throw "wrong"; // expected-note 5{{not valid}}
       done_b = true;
       return (T &&)v;
     }
@@ -95,10 +95,9 @@ namespace EvalOrder {
   constexpr int arr[3] = {};
   SEQ(A(arr)[B(0)]);
   SEQ(A(+arr)[B(0)]);
-  SEQ(A(0)[B(arr)]); // expected-error {{not an integral constant expression}} FIXME \
-                     // expected-note 2{{in call to}}
-  SEQ(A(0)[B(+arr)]); // expected-error {{not an integral constant expression}} FIXME \
-                      // expected-note 2{{in call to}}
+  SEQ(A(0)[B(arr)]);
+  SEQ(A(0)[B(+arr)]);
+
   SEQ(A(ud)[B(0)]);
 
   // Rule 7: a << b

``````````

</details>


https://github.com/llvm/llvm-project/pull/101804