[clang][Interp] Support pointer arithmetic in binary operators

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

241

I am not sure if this is the right place to handle this but there are a bunch of other cases.

• nullptr can have 0 added or subtracted
• You can only do addition/subtraction from a pointer if the result in within bounds or one after the end
• You can subtract two pointers if they point to the same object.

godbolt: https://godbolt.org/z/5YTY93z8M

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

241

I will probably move this special case out of the function and into its own.
Thanks for the tests, I think this should all already work except for adding two pointers. I'll update the diff once I checked that out.

[tbaeder]

clang/lib/AST/Interp/Interp.h

970 ↗

(On Diff #468060)

This is also not being diagnosed (only rejected) by the current interpreter. But would be nice to have an error message.

[tbaeder]

Ping

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

241

You will also want to cover this case as well: https://eel.is/c++draft/expr.add#6

261

or neither left or right operand is a pointer type

273

If it is two pointers then it must be a subtraction.

302

306

310

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

241

FWIW I've looked at the link but can hardly wrap my head around what the text is talking about, let alone make some contant expression out of it that doesn't run into some sort of other error :(

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

241

nvm, I just realized it requires a reinterpret_cast

[shafik]

clang/test/AST/Interp/arrays.cpp

69

We should also have a test for addition out of bounds but subsequent addition bringing it back in bounds. Still UB but we should verify it: https://godbolt.org/z/c1MWjGdfT

Apparently gcc does not catch this one.

[shafik]

clang/lib/AST/Interp/Interp.h

970 ↗

(On Diff #468060)

The TODO is still present, will you tackle this as part of this PR or a follow-up?

[tbaeder]

clang/lib/AST/Interp/Interp.h

970 ↗

(On Diff #468060)

A follow-up would be better I think.

[shafik]

LGTM

clang/lib/AST/Interp/ByteCodeExprGen.cpp

case BO_Comma:
     return true;
   default:
     break;
   }
 
   // Typecheck the args.
   Optional<PrimType> LT = classify(LHS->getType());
   Optional<PrimType> RT = classify(RHS->getType());
-  if (!LT || !RT) {
+  Optional<PrimType> T = classify(BO->getType());
+  if (!LT || !RT || !T) {
     return this->bail(BO);
   }
 
-  if (Optional<PrimType> T = classify(BO->getType())) {
-    if (!visit(LHS))
-      return false;
-    if (!visit(RHS))
-      return false;
-
-    auto Discard = [this, T, BO](bool Result) {
-      if (!Result)
-        return false;
-      return DiscardResult ? this->emitPop(*T, BO) : true;
-    };
 
-    switch (BO->getOpcode()) {
-    case BO_EQ:
-      return Discard(this->emitEQ(*LT, BO));
-    case BO_NE:
-      return Discard(this->emitNE(*LT, BO));
-    case BO_LT:
-      return Discard(this->emitLT(*LT, BO));
-    case BO_LE:
-      return Discard(this->emitLE(*LT, BO));
-    case BO_GT:
-      return Discard(this->emitGT(*LT, BO));
-    case BO_GE:
-      return Discard(this->emitGE(*LT, BO));
-    case BO_Sub:
-      return Discard(this->emitSub(*T, BO));
-    case BO_Add:
-      return Discard(this->emitAdd(*T, BO));
-    case BO_Mul:
-      return Discard(this->emitMul(*T, BO));
-    case BO_Rem:
-      return Discard(this->emitRem(*T, BO));
-    case BO_Div:
-      return Discard(this->emitDiv(*T, BO));
-    case BO_Assign:
-      if (!this->emitStore(*T, BO))
-        return false;
-      return DiscardResult ? this->emitPopPtr(BO) : true;
-    case BO_And:
-      return Discard(this->emitBitAnd(*T, BO));
-    case BO_Or:
-      return Discard(this->emitBitOr(*T, BO));
-    case BO_LAnd:
-    case BO_LOr:
-    default:
-      return this->bail(BO);
-    }
-  }
+  // Pointer arithmethic special case. This is supported for one of
+  // LHS and RHS being a pointer type and the other being an integer type.
+  if (BO->getType()->isPointerType()) {
+    PrimType OffsetType;
+    if (LHS->getType()->isIntegerType() && RHS->getType()->isPointerType()) {
+      if (!visit(RHS) || !visit(LHS))
+        return false;
+      OffsetType = *LT;
+    } else {
+      if (!visit(LHS) || !visit(RHS))
+        return false;
+      OffsetType = *RT;
+    }
+
+    if (BO->getOpcode() == BO_Add)
+      return Discard(this->emitAddOffset(OffsetType, BO));
+    else if (BO->getOpcode() == BO_Sub)
+      return Discard(this->emitSubOffset(OffsetType, BO));
+    return this->bail(BO);
+  }
+
+  if (!visit(LHS) || !visit(RHS))
+    return false;
+
 
-  return this->bail(BO);
+  llvm_unreachable("Unhandled binary op");
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E) {
   if (Optional<PrimType> T = classify(E))
     return this->emitZero(*T, E);
 
   return false;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitArraySubscriptExpr(
     const ArraySubscriptExpr *E) {
   const Expr *Base = E->getBase();
   const Expr *Index = E->getIdx();
 
   // Take pointer of LHS, add offset from RHS, narrow result.

clang/test/AST/Interp/arrays.cpp

 
 constexpr int getElement(int i) {
   int values[] = {1, 4, 9, 16, 25, 36};
   return values[i];
 }
 static_assert(getElement(1) == 4, "");
 static_assert(getElement(5) == 36, "");
 
+constexpr int data[] = {5, 4, 3, 2, 1};
+constexpr int getElement(const int *Arr, int index) {
+  return *(Arr + index);
+}
+
+static_assert(getElement(data, 1) == 4, "");
+static_assert(getElement(data, 4) == 1, "");
+
+constexpr int getElementFromEnd(const int *Arr, int size, int index) {
+  return *(Arr + size - index - 1);
+}
+static_assert(getElementFromEnd(data, 5, 0) == 1, "");
+static_assert(getElementFromEnd(data, 5, 4) == 5, "");
+
 
 template<typename T>
 constexpr T getElementOf(T* array, int i) {
   return array[i];
 }
 static_assert(getElementOf(foo[0], 1) == &m, "");
 
 
 template <typename T, int N>
 constexpr T& getElementOfArray(T (&array)[N], int I) {
   return array[I];
 }
 static_assert(getElementOfArray(foo[2], 3) == &m, "");
 
 
-constexpr int data[] = {5, 4, 3, 2, 1};
 static_assert(data[0] == 4, ""); // expected-error{{failed}} \
                                  // expected-note{{5 == 4}} \
                                  // ref-error{{failed}} \
                                  // ref-note{{5 == 4}}
 
 
 constexpr int dynamic[] = {
   f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
 };