Skip to content

Commit 0dcada9

Browse files
authored
[clang][Interp] Fix array subscript eval order (#101804)
Always evaluate LHS first, then RHS.
1 parent 79caa06 commit 0dcada9

File tree

4 files changed

+41
-12
lines changed

4 files changed

+41
-12
lines changed

clang/lib/AST/Interp/Compiler.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,21 +1282,31 @@ bool Compiler<Emitter>::VisitImplicitValueInitExpr(
12821282

12831283
template <class Emitter>
12841284
bool Compiler<Emitter>::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
1285-
const Expr *Base = E->getBase();
1285+
const Expr *LHS = E->getLHS();
1286+
const Expr *RHS = E->getRHS();
12861287
const Expr *Index = E->getIdx();
12871288

12881289
if (DiscardResult)
1289-
return this->discard(Base) && this->discard(Index);
1290+
return this->discard(LHS) && this->discard(RHS);
12901291

1291-
// Take pointer of LHS, add offset from RHS.
1292-
// What's left on the stack after this is a pointer.
1293-
if (!this->visit(Base))
1294-
return false;
1292+
// C++17's rules require us to evaluate the LHS first, regardless of which
1293+
// side is the base.
1294+
bool Success = true;
1295+
for (const Expr *SubExpr : {LHS, RHS}) {
1296+
if (!this->visit(SubExpr))
1297+
Success = false;
1298+
}
12951299

1296-
if (!this->visit(Index))
1300+
if (!Success)
12971301
return false;
12981302

12991303
PrimType IndexT = classifyPrim(Index->getType());
1304+
// If the index is first, we need to change that.
1305+
if (LHS == Index) {
1306+
if (!this->emitFlip(PT_Ptr, IndexT, E))
1307+
return false;
1308+
}
1309+
13001310
return this->emitArrayElemPtrPop(IndexT, E);
13011311
}
13021312

clang/lib/AST/Interp/Interp.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1158,6 +1158,21 @@ bool Pop(InterpState &S, CodePtr OpPC) {
11581158
return true;
11591159
}
11601160

1161+
/// [Value1, Value2] -> [Value2, Value1]
1162+
template <PrimType TopName, PrimType BottomName>
1163+
bool Flip(InterpState &S, CodePtr OpPC) {
1164+
using TopT = typename PrimConv<TopName>::T;
1165+
using BottomT = typename PrimConv<BottomName>::T;
1166+
1167+
const auto &Top = S.Stk.pop<TopT>();
1168+
const auto &Bottom = S.Stk.pop<BottomT>();
1169+
1170+
S.Stk.push<TopT>(Top);
1171+
S.Stk.push<BottomT>(Bottom);
1172+
1173+
return true;
1174+
}
1175+
11611176
//===----------------------------------------------------------------------===//
11621177
// Const
11631178
//===----------------------------------------------------------------------===//

clang/lib/AST/Interp/Opcodes.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,11 @@ def Dup : Opcode {
729729
let HasGroup = 1;
730730
}
731731

732+
def Flip : Opcode {
733+
let Types = [AllTypeClass, AllTypeClass];
734+
let HasGroup = 1;
735+
}
736+
732737
// [] -> []
733738
def Invalid : Opcode {}
734739
def Unsupported : Opcode {}

clang/test/AST/Interp/eval-order.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ namespace EvalOrder {
4545
}
4646
template <typename T> constexpr T &&b(T &&v) {
4747
if (!done_a)
48-
throw "wrong"; // expected-note 7{{not valid}}
48+
throw "wrong"; // expected-note 5{{not valid}}
4949
done_b = true;
5050
return (T &&)v;
5151
}
@@ -95,10 +95,9 @@ namespace EvalOrder {
9595
constexpr int arr[3] = {};
9696
SEQ(A(arr)[B(0)]);
9797
SEQ(A(+arr)[B(0)]);
98-
SEQ(A(0)[B(arr)]); // expected-error {{not an integral constant expression}} FIXME \
99-
// expected-note 2{{in call to}}
100-
SEQ(A(0)[B(+arr)]); // expected-error {{not an integral constant expression}} FIXME \
101-
// expected-note 2{{in call to}}
98+
SEQ(A(0)[B(arr)]);
99+
SEQ(A(0)[B(+arr)]);
100+
102101
SEQ(A(ud)[B(0)]);
103102

104103
// Rule 7: a << b

0 commit comments

Comments
 (0)