[clang][Interp] Support inc/dec operators on pointers

[tbaeder]

Summary

I think this is pretty uncommon so I was trying to implement it without adding new opcodes, but looks like that's not possible.

[aaron.ballman]

clang/lib/AST/Interp/Opcodes.td

404–408

Don't both of these result in a Pointer? And should they have a Types field?

clang/test/AST/Interp/arrays.cpp

225–279

I'd like test cases where the pointer arithmetic has run off the beginning/end of the object (forming the invalid pointer is UB per http://eel.is/c++draft/expr.add#4.3 even if you never dereference the pointer).

[tbaeder]

clang/lib/AST/Interp/Opcodes.td

404–408

Ah yes, they do. They don't need a type field, we know we're operating on a pointer.

[aaron.ballman]

clang/test/AST/Interp/arrays.cpp

225–279

constexpr int bad1() {
  const int *e =  E + 3;
  e++; // This is fine because it's a one-past-the-end pointer
  return *e; // This is UB
}

constexpr int bad2() {
  const int *e = E + 4;
  e++; // This is UB
  return *e; // This is UB as well
}

constexpr int bad3() {
  const int *e = E;
  --e; // This is UB
  return *e; // This is UB as well
}

[tbaeder]

clang/test/AST/Interp/arrays.cpp

225–279

Interesting side-effect of removing the Run call in isPotentialConstantExpression(): The new constant interpreter doesn't diagnose the "never produces a constant expression` case. The function must be called for any diagnostics to be printed.

[tbaeder]

Ping

[aaron.ballman]

LGTM!

[shafik]

LGTM

clang/lib/AST/Interp/ByteCodeExprGen.cpp

bool ByteCodeExprGen<Emitter>::VisitCXXThisExpr(const CXXThisExpr *E) {
   return this->emitThis(E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitUnaryOperator(const UnaryOperator *E) {
   const Expr *SubExpr = E->getSubExpr();
   Optional<PrimType> T = classify(SubExpr->getType());
 
-  // TODO: Support pointers for inc/dec operators.
   switch (E->getOpcode()) {
   case UO_PostInc: { // x++
     if (!this->visit(SubExpr))
       return false;
 
+    if (T == PT_Ptr) {
+      if (!this->emitIncPtr(E))
+        return false;
+
+      return DiscardResult ? this->emitPopPtr(E) : true;
+    }
+
     return DiscardResult ? this->emitIncPop(*T, E) : this->emitInc(*T, E);
   }
   case UO_PostDec: { // x--
     if (!this->visit(SubExpr))
       return false;
 
+    if (T == PT_Ptr) {
+      if (!this->emitDecPtr(E))
+        return false;
+
+      return DiscardResult ? this->emitPopPtr(E) : true;
+    }
+
     return DiscardResult ? this->emitDecPop(*T, E) : this->emitDec(*T, E);
   }
   case UO_PreInc: { // ++x
     if (!this->visit(SubExpr))
       return false;
 
+    if (T == PT_Ptr) {
+      this->emitLoadPtr(E);
+      this->emitConstUint8(1, E);
+      this->emitAddOffsetUint8(E);
+      return DiscardResult ? this->emitStorePopPtr(E) : this->emitStorePtr(E);
+    }
+
     // Post-inc and pre-inc are the same if the value is to be discarded.
     if (DiscardResult)
       return this->emitIncPop(*T, E);
 
     this->emitLoad(*T, E);
     this->emitConst(1, E);
     this->emitAdd(*T, E);
     return this->emitStore(*T, E);
   }
   case UO_PreDec: { // --x
     if (!this->visit(SubExpr))
       return false;
 
+    if (T == PT_Ptr) {
+      this->emitLoadPtr(E);
+      this->emitConstUint8(1, E);
+      this->emitSubOffsetUint8(E);
+      return DiscardResult ? this->emitStorePopPtr(E) : this->emitStorePtr(E);
+    }
+
     // Post-dec and pre-dec are the same if the value is to be discarded.
     if (DiscardResult)
       return this->emitDecPop(*T, E);
 
     this->emitLoad(*T, E);
     this->emitConst(1, E);
     this->emitSub(*T, E);
     return this->emitStore(*T, E);

clang/lib/AST/Interp/Interp.h

 
 /// Checks if Div/Rem operation on LHS and RHS is valid.
 template <typename T>
 bool CheckDivRem(InterpState &S, CodePtr OpPC, const T &LHS, const T &RHS);
 
 /// Interpreter entry point.
 bool Interpret(InterpState &S, APValue &Result);
 
+enum class ArithOp { Add, Sub };
+
 //===----------------------------------------------------------------------===//
 // Add, Sub, Mul
 //===----------------------------------------------------------------------===//
 
 template <typename T, bool (*OpFW)(T, T, unsigned, T *),
           template <typename U> class OpAP>
 bool AddSubMulHelper(InterpState &S, CodePtr OpPC, unsigned Bits, const T &LHS,
                      const T &RHS) {
bool AddOffset(InterpState &S, CodePtr OpPC) {
   return OffsetHelper<T, true>(S, OpPC);
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool SubOffset(InterpState &S, CodePtr OpPC) {
   return OffsetHelper<T, false>(S, OpPC);
 }
 
+template <ArithOp Op>
+static inline bool IncDecPtrHelper(InterpState &S, CodePtr OpPC) {
+  using OneT = Integral<8, false>;
+  const Pointer &Ptr = S.Stk.pop<Pointer>();
+
+  // Get the current value on the stack.
+  S.Stk.push<Pointer>(Ptr.deref<Pointer>());
+
+  // Now the current Ptr again and a constant 1.
+  // FIXME: We shouldn't have to push these two on the stack.
+  S.Stk.push<Pointer>(Ptr.deref<Pointer>());
+  S.Stk.push<OneT>(OneT::from(1));
+  if (!OffsetHelper<OneT, Op == ArithOp::Add>(S, OpPC))
+    return false;
+
+  // Store the new value.
+  Ptr.deref<Pointer>() = S.Stk.pop<Pointer>();
+  return true;
+}
+
+static inline bool IncPtr(InterpState &S, CodePtr OpPC) {
+  return IncDecPtrHelper<ArithOp::Add>(S, OpPC);
+}
+
+static inline bool DecPtr(InterpState &S, CodePtr OpPC) {
+  return IncDecPtrHelper<ArithOp::Sub>(S, OpPC);
+}
+
 /// 1) Pops a Pointer from the stack.
 /// 2) Pops another Pointer from the stack.
 /// 3) Pushes the different of the indices of the two pointers on the stack.
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 inline bool SubPtr(InterpState &S, CodePtr OpPC) {
   const Pointer &LHS = S.Stk.pop<Pointer>();
   const Pointer &RHS = S.Stk.pop<Pointer>();
 

clang/lib/AST/Interp/Opcodes.td

 // Pointer arithmetic.
 //===----------------------------------------------------------------------===//
 
 // [Pointer, Integral] -> [Pointer]
 def AddOffset : AluOpcode;
 // [Pointer, Integral] -> [Pointer]
 def SubOffset : AluOpcode;
 
-// Pointer, Pointer] - [Integral]
+// [Pointer, Pointer] -> [Integral]
 def SubPtr : Opcode {
   let Types = [IntegerTypeClass];
   let HasGroup = 1;
 }
 
+// [Pointer] -> [Pointer]
+def IncPtr : Opcode {
+  let HasGroup = 0;
+}
+// [Pointer] -> [Pointer]
+def DecPtr : Opcode {
+  let HasGroup = 0;
+}
+
 //===----------------------------------------------------------------------===//
 // Binary operators.
 //===----------------------------------------------------------------------===//
 
 // [Real, Real] -> [Real]
 def Sub : AluOpcode;
 def Add : AluOpcode;
 def Mul : AluOpcode;

clang/test/AST/Interp/arrays.cpp

public:
   A a[2];
   constexpr B() {}
 };
 constexpr B b;
 static_assert(b.a[0].a == 12, "");
 static_assert(b.a[1].a == 12, "");
 
 namespace IncDec {
-  // FIXME: Pointer arithmethic needs to be supported in inc/dec
-  //   unary operators
-#if 0
   constexpr int getNextElem(const int *A, int I) {
     const int *B = (A + I);
     ++B;
     return *B;
   }
   constexpr int E[] = {1,2,3,4};
 
-  static_assert(getNextElem(E, 1) == 3);
-#endif
+  static_assert(getNextElem(E, 1) == 3, "");
+
+  constexpr int getFirst() {
+    const int *e = E;
+    return *(e++);
+  }
+  static_assert(getFirst() == 1, "");
+
+  constexpr int getFirst2() {
+    const int *e = E;
+    e++;
+    return *e;
+  }
+  static_assert(getFirst2() == 2, "");
+
+  constexpr int getSecond() {
+    const int *e = E;
+    return *(++e);
+  }
+  static_assert(getSecond() == 2, "");
+
+  constexpr int getSecond2() {
+    const int *e = E;
+    ++e;
+    return *e;
+  }
+  static_assert(getSecond2() == 2, "");
+
+  constexpr int getLast() {
+    const int *e = E + 3;
+    return *(e--);
+  }
+  static_assert(getLast() == 4, "");
+
+  constexpr int getLast2() {
+    const int *e = E + 3;
+    e--;
+    return *e;
+  }
+  static_assert(getLast2() == 3, "");
+
+  constexpr int getSecondToLast() {
+    const int *e = E + 3;
+    return *(--e);
+  }
+  static_assert(getSecondToLast() == 3, "");
+
+  constexpr int getSecondToLast2() {
+    const int *e = E + 3;
+    --e;
+    return *e;
+  }
+  static_assert(getSecondToLast2() == 3, "");
+
+
 };