[clang][bytecode][NFC] Don't push anything in OffsetHelper

[tbaederr]

Instead, return the new Pointer. This was weird before because some callers had to get the value from the stack again to perform further operations.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Instead, return the new Pointer. This was weird before because some callers had to get the value from the stack again to perform further operations.

---

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

1 Files Affected:

• (modified) clang/lib/AST/ByteCode/Interp.h (+60-45)

diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 85a07abb5ed01..9d17f96c97c85 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2066,34 +2066,32 @@ inline bool CastMemberPtrPtr(InterpState &S, CodePtr OpPC) {
 //===----------------------------------------------------------------------===//
 
 template <class T, ArithOp Op>
-bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset,
-                  const Pointer &Ptr, bool IsPointerArith = false) {
+std::optional<Pointer> OffsetHelper(InterpState &S, CodePtr OpPC,
+                                    const T &Offset, const Pointer &Ptr,
+                                    bool IsPointerArith = false) {
   // A zero offset does not change the pointer.
-  if (Offset.isZero()) {
-    S.Stk.push<Pointer>(Ptr);
-    return true;
-  }
+  if (Offset.isZero())
+    return Ptr;
 
   if (IsPointerArith && !CheckNull(S, OpPC, Ptr, CSK_ArrayIndex)) {
     // The CheckNull will have emitted a note already, but we only
     // abort in C++, since this is fine in C.
     if (S.getLangOpts().CPlusPlus)
-      return false;
+      return std::nullopt;
   }
 
   // Arrays of unknown bounds cannot have pointers into them.
   if (!CheckArray(S, OpPC, Ptr))
-    return false;
+    return std::nullopt;
 
   // This is much simpler for integral pointers, so handle them first.
   if (Ptr.isIntegralPointer()) {
     uint64_t V = Ptr.getIntegerRepresentation();
     uint64_t O = static_cast<uint64_t>(Offset) * Ptr.elemSize();
     if constexpr (Op == ArithOp::Add)
-      S.Stk.push<Pointer>(V + O, Ptr.asIntPointer().Desc);
+      return Pointer(V + O, Ptr.asIntPointer().Desc);
     else
-      S.Stk.push<Pointer>(V - O, Ptr.asIntPointer().Desc);
-    return true;
+      return Pointer(V - O, Ptr.asIntPointer().Desc);
   } else if (Ptr.isFunctionPointer()) {
     uint64_t O = static_cast<uint64_t>(Offset);
     uint64_t N;
@@ -2105,8 +2103,7 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset,
     if (N > 1)
       S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
           << N << /*non-array*/ true << 0;
-    S.Stk.push<Pointer>(Ptr.asFunctionPointer().getFunction(), N);
-    return true;
+    return Pointer(Ptr.asFunctionPointer().getFunction(), N);
   }
 
   assert(Ptr.isBlockPointer());
@@ -2156,7 +2153,7 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset,
   }
 
   if (Invalid && S.getLangOpts().CPlusPlus)
-    return false;
+    return std::nullopt;
 
   // Offset is valid - compute it on unsigned.
   int64_t WideIndex = static_cast<int64_t>(Index);
@@ -2172,15 +2169,11 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset,
   // we don't get here.
   if (Result == 0 && Ptr.isOnePastEnd()) {
     if (Ptr.getFieldDesc()->isArray())
-      S.Stk.push<Pointer>(Ptr.atIndex(0));
-    else
-      S.Stk.push<Pointer>(Ptr.asBlockPointer().Pointee,
-                          Ptr.asBlockPointer().Base);
-    return true;
+      return Ptr.atIndex(0);
+    return Pointer(Ptr.asBlockPointer().Pointee, Ptr.asBlockPointer().Base);
   }
 
-  S.Stk.push<Pointer>(Ptr.atIndex(static_cast<uint64_t>(Result)));
-  return true;
+  return Ptr.atIndex(static_cast<uint64_t>(Result));
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
@@ -2189,16 +2182,26 @@ bool AddOffset(InterpState &S, CodePtr OpPC) {
   Pointer Ptr = S.Stk.pop<Pointer>();
   if (Ptr.isBlockPointer())
     Ptr = Ptr.expand();
-  return OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr,
-                                       /*IsPointerArith=*/true);
+
+  if (std::optional<Pointer> Result = OffsetHelper<T, ArithOp::Add>(
+          S, OpPC, Offset, Ptr, /*IsPointerArith=*/true)) {
+    S.Stk.push<Pointer>(*Result);
+    return true;
+  }
+  return false;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool SubOffset(InterpState &S, CodePtr OpPC) {
   const T &Offset = S.Stk.pop<T>();
   const Pointer &Ptr = S.Stk.pop<Pointer>();
-  return OffsetHelper<T, ArithOp::Sub>(S, OpPC, Offset, Ptr,
-                                       /*IsPointerArith=*/true);
+
+  if (std::optional<Pointer> Result = OffsetHelper<T, ArithOp::Sub>(
+          S, OpPC, Offset, Ptr, /*IsPointerArith=*/true)) {
+    S.Stk.push<Pointer>(*Result);
+    return true;
+  }
+  return false;
 }
 
 template <ArithOp Op>
@@ -2218,12 +2221,13 @@ static inline bool IncDecPtrHelper(InterpState &S, CodePtr OpPC,
 
   // Now the current Ptr again and a constant 1.
   OneT One = OneT::from(1);
-  if (!OffsetHelper<OneT, Op>(S, OpPC, One, P, /*IsPointerArith=*/true))
-    return false;
-
-  // Store the new value.
-  Ptr.deref<Pointer>() = S.Stk.pop<Pointer>();
-  return true;
+  if (std::optional<Pointer> Result =
+          OffsetHelper<OneT, Op>(S, OpPC, One, P, /*IsPointerArith=*/true)) {
+    // Store the new value.
+    Ptr.deref<Pointer>() = *Result;
+    return true;
+  }
+  return false;
 }
 
 static inline bool IncPtr(InterpState &S, CodePtr OpPC) {
@@ -2925,16 +2929,22 @@ inline bool ArrayElemPtr(InterpState &S, CodePtr OpPC) {
 
   if (Offset.isZero()) {
     if (Ptr.getFieldDesc()->isArray() && Ptr.getIndex() == 0) {
-      S.Stk.push<Pointer>(Ptr.atIndex(0));
-    } else {
-      S.Stk.push<Pointer>(Ptr);
+      S.Stk.push<Pointer>(Ptr.atIndex(0).narrow());
+      return true;
     }
-  } else {
-    if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr))
-      return false;
+    S.Stk.push<Pointer>(Ptr);
+    return true;
+  }
+
+  assert(!Offset.isZero());
+
+  if (std::optional<Pointer> Result =
+          OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) {
+    S.Stk.push<Pointer>(Result->narrow());
+    return true;
   }
 
-  return NarrowPtr(S, OpPC);
+  return false;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
@@ -2949,16 +2959,21 @@ inline bool ArrayElemPtrPop(InterpState &S, CodePtr OpPC) {
 
   if (Offset.isZero()) {
     if (Ptr.getFieldDesc()->isArray() && Ptr.getIndex() == 0) {
-      S.Stk.push<Pointer>(Ptr.atIndex(0));
-    } else {
-      S.Stk.push<Pointer>(Ptr);
+      S.Stk.push<Pointer>(Ptr.atIndex(0).narrow());
+      return true;
     }
-  } else {
-    if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr))
-      return false;
+    S.Stk.push<Pointer>(Ptr);
+    return true;
   }
 
-  return NarrowPtr(S, OpPC);
+  assert(!Offset.isZero());
+
+  if (std::optional<Pointer> Result =
+          OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) {
+    S.Stk.push<Pointer>(Result->narrow());
+    return true;
+  }
+  return false;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>