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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
