[clang][Interp] Convert logical binop operands to bool

[tbaeder]

Summary

Move the logic for this into visitBool, where it belongs. Then convert the logical binary operator operands to bool using visitBool() and the result back to int, if needed (which it is in C).

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

531–534

This is casting to boolean type, not integer type -- shouldn't that be emitting an int?

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

531–534

That line is casting from PT_Bool to T, because we visited the operands as bool and now we need to convert back to whatever type we should have.

[aaron.ballman]

LGTM

clang/lib/AST/Interp/ByteCodeExprGen.cpp

531–534

Ah!! That makes more sense, thank you.

clang/lib/AST/Interp/ByteCodeExprGen.cpp

 }
 
 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();
+  std::optional<PrimType> T = classify(E->getType());
 
   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))
+    if (!this->visitBool(LHS))
       return false;
     if (!this->jumpTrue(LabelTrue))
       return false;
 
-    if (!this->visit(RHS))
+    if (!this->visitBool(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;
-  }
-
+  } else {
+    assert(Op == BO_LAnd);
     // Logical AND.
     // Visit LHS. Only visit RHS if LHS was TRUE.
     LabelTy LabelFalse = this->getLabel();
     LabelTy LabelEnd = this->getLabel();
 
-  if (!this->visit(LHS))
+    if (!this->visitBool(LHS))
       return false;
     if (!this->jumpFalse(LabelFalse))
       return false;
 
-  if (!this->visit(RHS))
+    if (!this->visitBool(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);
 
+  // For C, cast back to integer type.
+  assert(T);
+  if (T != PT_Bool)
+    return this->emitCast(PT_Bool, *T, E);
   return true;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E) {
   QualType QT = E->getType();
 
   if (classify(QT))
bool ByteCodeExprGen<Emitter>::VisitAbstractConditionalOperator(
     const AbstractConditionalOperator *E) {
   const Expr *Condition = E->getCond();
   const Expr *TrueExpr = E->getTrueExpr();
   const Expr *FalseExpr = E->getFalseExpr();
 
   LabelTy LabelEnd = this->getLabel();   // Label after the operator.
   LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
 
-  if (!this->visit(Condition))
+  if (!this->visitBool(Condition))
     return false;
 
-  // C special case: Convert to bool because our jump ops need that.
-  // TODO: We probably want this to be done in visitBool().
-  if (std::optional<PrimType> CondT = classify(Condition->getType());
-      CondT && CondT != PT_Bool) {
-    if (!this->emitCast(*CondT, PT_Bool, E))
-      return false;
-  }
-
   if (!this->jumpFalse(LabelFalse))
     return false;
 
   if (!this->delegate(TrueExpr))
     return false;
   if (!this->jump(LabelEnd))
     return false;
 
bool ByteCodeExprGen<Emitter>::visitInitializer(const Expr *E) {
 
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/false,
                              /*NewInitializing=*/true);
   return this->Visit(E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitBool(const Expr *E) {
-  if (std::optional<PrimType> T = classify(E->getType())) {
-    return visit(E);
-  } else {
-    return this->bail(E);
+  std::optional<PrimType> T = classify(E->getType());
+  if (!T)
+    return false;
+
+  if (!this->visit(E))
+    return false;
+
+  if (T == PT_Bool)
+    return true;
+
+  // Convert pointers to bool.
+  if (T == PT_Ptr || T == PT_FnPtr) {
+    if (!this->emitNull(*T, E))
+      return false;
+    return this->emitNE(*T, E);
   }
+
+  // Or Floats.
+  if (T == PT_Float)
+    return this->emitCastFloatingIntegralBool(E);
+
+  // Or anything else we can.
+  return this->emitCast(*T, PT_Bool, E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitZeroInitializer(QualType QT,
                                                     const Expr *E) {
   // FIXME: We need the QualType to get the float semantics, but that means we
   //   classify it over and over again in array situations.
   PrimType T = classifyPrim(QT);