[clang][Interp] Implement logical and/or operators

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

371

[aaron.ballman]

Assuming my understanding is correct, this LGTM!

clang/lib/AST/Interp/ByteCodeExprGen.cpp

360

Am I correct in understanding that the reason we don't need to emit a const bool for false is because visiting the RHS already pushes either true or false onto the stack because it's already been converted to a bool result?

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

360

Both LHS and RHS push a bool result to the stack, but the jumpTruewill pop() the result of the LHS, so we need to push a true again if we want to use it later as the result of the binary operator.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

360

Thank you for the confirmation, that makes sense to me. LGTM!

clang/lib/AST/Interp/ByteCodeExprGen.h

ByteCodeExprGen(Context &Ctx, Program &P, Tys &&... Args)
       : Emitter(Ctx, P, Args...), Ctx(Ctx), P(P) {}
 
   // Expression visitors - result returned on interp stack.
   bool VisitCastExpr(const CastExpr *E);
   bool VisitIntegerLiteral(const IntegerLiteral *E);
   bool VisitFloatingLiteral(const FloatingLiteral *E);
   bool VisitParenExpr(const ParenExpr *E);
   bool VisitBinaryOperator(const BinaryOperator *E);
+  bool VisitLogicalBinOp(const BinaryOperator *E);
   bool VisitPointerArithBinOp(const BinaryOperator *E);
   bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E);
   bool VisitCallExpr(const CallExpr *E);
   bool VisitBuiltinCallExpr(const CallExpr *E);
   bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *E);
   bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E);
   bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E);
   bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E);

clang/lib/AST/Interp/ByteCodeExprGen.cpp

 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitParenExpr(const ParenExpr *PE) {
   return this->visit(PE->getSubExpr());
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
+  // Need short-circuiting for these.
+  if (BO->isLogicalOp())
+    return this->VisitLogicalBinOp(BO);
+
   const Expr *LHS = BO->getLHS();
   const Expr *RHS = BO->getRHS();
 
   // Deal with operations which have composite or void types.
   switch (BO->getOpcode()) {
   case BO_Comma:
     if (!discard(LHS))
       return false;
bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
   case BO_Or:
     return Discard(this->emitBitOr(*T, BO));
   case BO_Shl:
     return Discard(this->emitShl(*LT, *RT, BO));
   case BO_Shr:
     return Discard(this->emitShr(*LT, *RT, BO));
   case BO_Xor:
     return Discard(this->emitBitXor(*T, BO));
-  case BO_LAnd:
   case BO_LOr:
+  case BO_LAnd:
+    llvm_unreachable("Already handled earlier");
   default:
     return this->bail(BO);
   }
 
   llvm_unreachable("Unhandled binary op");
 }
 
 /// Perform addition/subtraction of a pointer and an integer or
if (Op == BO_Add)
     return this->emitAddOffset(OffsetType, E);
   else if (Op == BO_Sub)
     return this->emitSubOffset(OffsetType, E);
 
   return this->bail(E);
 }
 
 template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitLogicalBinOp(const BinaryOperator *E) {
+  assert(E->isLogicalOp());
+  BinaryOperatorKind Op = E->getOpcode();
+  const Expr *LHS = E->getLHS();
+  const Expr *RHS = E->getRHS();
+
+  if (Op == BO_LOr) {
+    // Logical OR. Visit LHS and only evaluate RHS if LHS was FALSE.
+    LabelTy LabelTrue = this->getLabel();
+    LabelTy LabelEnd = this->getLabel();
+
+    if (!this->visit(LHS))
+      return false;
+    if (!this->jumpTrue(LabelTrue))
+      return false;
+
+    if (!this->visit(RHS))
+      return false;
+    if (!this->jump(LabelEnd))
+      return false;
+
+    this->emitLabel(LabelTrue);
+    this->emitConstBool(true, E);
+    this->fallthrough(LabelEnd);
+    this->emitLabel(LabelEnd);
+
+    if (DiscardResult)
+      return this->emitPopBool(E);
+
+    return true;
+  }
+
+  // Logical AND.
+  // Visit LHS. Only visit RHS if LHS was TRUE.
+  LabelTy LabelFalse = this->getLabel();
+  LabelTy LabelEnd = this->getLabel();
+
+  if (!this->visit(LHS))
+    return false;
+  if (!this->jumpFalse(LabelFalse))
+    return false;
+
+  if (!this->visit(RHS))
+    return false;
+  if (!this->jump(LabelEnd))
+    return false;
+
+  this->emitLabel(LabelFalse);
+  this->emitConstBool(false, E);
+  this->fallthrough(LabelEnd);
+  this->emitLabel(LabelEnd);
+
+  if (DiscardResult)
+    return this->emitPopBool(E);
+
+  return true;
+}
+
+template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E) {
   std::optional<PrimType> T = classify(E);
 
   if (!T)
     return false;
 
   if (E->getType()->isPointerType())
     return this->emitNullPtr(E);

clang/test/AST/Interp/cond.cpp

 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -fexperimental-new-constant-interpreter %s -verify
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only %s -verify
 // expected-no-diagnostics
 
 constexpr int cond_then_else(int a, int b) {
   if (a < b) {
     return b - a;
   } else {
     return a - b;
   }
 }
+
+constexpr int dontCallMe(unsigned m) {
+  if (m == 0) return 0;
+  return dontCallMe(m - 2);
+}
+
+// Can't call this because it will run into infinite recursion.
+constexpr int assertNotReached() {
+  return dontCallMe(3);
+}
+
+static_assert(true || true, "");
+static_assert(true || false, "");
+static_assert(false || true, "");
+static_assert(!(false || false), "");
+
+static_assert(true || assertNotReached(), "");
+static_assert(true || true || true || false, "");
+
+static_assert(true && true, "");
+static_assert(!(true && false), "");
+static_assert(!(false && true), "");
+static_assert(!(false && false), "");
+
+static_assert(!(false && assertNotReached()), "");
+static_assert(!(true && true && true && false), "");