Wrap BinaryOperator in integer cast in C mode (#58)

In C, comparison and logical binary operations always produce an int
where a bool is expected.

For this reason, wrap binary operators in `(... as i32)` if the type of
BinaryOperator is int. ConvertIntegralToBooleanCast will later take care
to convert the integer back to boolean.

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -1742,24 +1742,12 @@ void Converter::ConvertIntegralToBooleanCast(clang::ImplicitCastExpr *expr) {
   auto *stripped = sub_expr->IgnoreParenImpCasts();
 
   if (auto binop = clang::dyn_cast<clang::BinaryOperator>(stripped)) {
-    // Comparison already produces bool, no wrap needed.
-    if (binop->isComparisonOp()) {
+    // Comparisons and logical ops already produces bool, no wrap needed.
+    if ((binop->isComparisonOp() || binop->isLogicalOp()) &&
+        binop->getType()->isBooleanType()) {
       Convert(sub_expr);
       return;
     }
-    // Distribute bool conversion to each argument of the logical op.
-    if (binop->isLogicalOp()) {
-      {
-        PushParen paren(*this);
-        ConvertCondition(binop->getLHS());
-      }
-      StrCat(binop->getOpcodeStr());
-      {
-        PushParen paren(*this);
-        ConvertCondition(binop->getRHS());
-      }
-      return;
-    }
   }
 
   PushParen paren(*this);
@@ -1945,6 +1933,21 @@ bool Converter::VisitExplicitCastExpr(clang::ExplicitCastExpr *expr) {
 }
 
 bool Converter::VisitBinaryOperator(clang::BinaryOperator *expr) {
+  bool needs_cast = (expr->isComparisonOp() || expr->isLogicalOp()) &&
+                    expr->getType()->isIntegerType() &&
+                    !expr->getType()->isBooleanType();
+  PushParen outer(*this, needs_cast);
+  {
+    PushParen inner(*this, needs_cast);
+    ConvertBinaryOperator(expr);
+  }
+  if (needs_cast) {
+    ConvertCast(expr->getType());
+  }
+  return false;
+}
+
+void Converter::ConvertBinaryOperator(clang::BinaryOperator *expr) {
   auto type = expr->getType();
   auto *lhs = expr->getLHS();
   auto *rhs = expr->getRHS();
@@ -2048,10 +2051,19 @@ bool Converter::VisitBinaryOperator(clang::BinaryOperator *expr) {
     }
     ConvertCast(expr->getType());
     computed_expr_type_ = ComputedExprType::FreshValue;
+  } else if (expr->isLogicalOp()) {
+    {
+      PushParen paren(*this);
+      ConvertCondition(expr->getLHS());
+    }
+    StrCat(expr->getOpcodeStr());
+    {
+      PushParen paren(*this);
+      ConvertCondition(expr->getRHS());
+    }
   } else {
     ConvertGenericBinaryOperator(expr);
   }
-  return false;
 }
 
 void Converter::ConvertGenericBinaryOperator(clang::BinaryOperator *expr) {
@@ -2063,8 +2075,10 @@ void Converter::ConvertGenericBinaryOperator(clang::BinaryOperator *expr) {
 
   StrCat(expr->getOpcodeStr());
 
-  PushParen rhs_paren(*this);
-  Convert(expr->getRHS());
+  {
+    PushParen rhs_paren(*this);
+    Convert(expr->getRHS());
+  }
 }
 
 bool Converter::IsReferenceType(const clang::Expr *expr) const {

cpp2rust/converter/converter.h

@@ -252,6 +252,8 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
 
   virtual bool VisitBinaryOperator(clang::BinaryOperator *expr);
 
+  virtual void ConvertBinaryOperator(clang::BinaryOperator *expr);
+
   virtual bool ConvertIncAndDec(clang::UnaryOperator *expr);
 
   virtual bool VisitUnaryOperator(clang::UnaryOperator *expr);

cpp2rust/converter/models/converter_refcount.cpp

@@ -1191,7 +1191,7 @@ void ConverterRefCount::EmitStmtExprTail(clang::Expr *tail) {
   StrCat("__result");
 }
 
-bool ConverterRefCount::VisitBinaryOperator(clang::BinaryOperator *expr) {
+void ConverterRefCount::ConvertBinaryOperator(clang::BinaryOperator *expr) {
   auto *lhs = expr->getLHS();
   auto *rhs = expr->getRHS();
   auto lhs_type = lhs->getType();
@@ -1229,7 +1229,7 @@ bool ConverterRefCount::VisitBinaryOperator(clang::BinaryOperator *expr) {
     }
     StrCat(token::kSemiColon);
     EmitSetOrAssign(lhs, "rhs_0");
-    return false;
+    return;
   }
 
   if (IsUnsignedArithOp(expr)) {
@@ -1249,7 +1249,7 @@ bool ConverterRefCount::VisitBinaryOperator(clang::BinaryOperator *expr) {
       StrCat(token::kSemiColon);
       EmitSetOrAssign(lhs, "rhs_0");
     }
-    return false;
+    return;
   }
 
   // pointer subtraction. The Sub trait gets elements by Value, so we need
@@ -1263,15 +1263,15 @@ bool ConverterRefCount::VisitBinaryOperator(clang::BinaryOperator *expr) {
     }
     ConvertCast(expr->getType());
     computed_expr_type_ = ComputedExprType::FreshValue;
-    return false;
+    return;
   }
 
   if (expr->isAssignmentOp()) {
     ConvertAssignment(lhs, rhs, opcode_as_string);
-    return false;
+    return;
   }
 
-  return Converter::VisitBinaryOperator(expr);
+  Converter::ConvertBinaryOperator(expr);
 }
 
 bool ConverterRefCount::VisitInitListExpr(clang::InitListExpr *expr) {
@@ -1840,7 +1840,7 @@ void ConverterRefCount::ConvertGenericBinaryOperator(
     return;
   }
 
-  PushParen paren(*this);
+  PushParen outer(*this);
   Convert(lhs);
   StrCat(opcode);
   Convert(rhs);

cpp2rust/converter/models/converter_refcount.h

@@ -82,7 +82,7 @@ class ConverterRefCount final : public Converter {
 
   bool VisitExplicitCastExpr(clang::ExplicitCastExpr *expr) override;
 
-  bool VisitBinaryOperator(clang::BinaryOperator *expr) override;
+  void ConvertBinaryOperator(clang::BinaryOperator *expr) override;
 
   bool VisitStmtExpr(clang::StmtExpr *expr) override;
 

tests/benchmarks/out/refcount/bst.rs

@@ -26,25 +26,21 @@ impl ByteRepr for node_t {}
 pub fn find_0(node: Ptr<node_t>, value: i32) -> Ptr<node_t> {
     let node: Value<Ptr<node_t>> = Rc::new(RefCell::new(node));
     let value: Value<i32> = Rc::new(RefCell::new(value));
-    if {
-        let _lhs = {
-            let _lhs = (*value.borrow());
-            _lhs < (*(*(*node.borrow()).upgrade().deref()).value.borrow())
-        };
-        _lhs && (!((*(*(*node.borrow()).upgrade().deref()).left.borrow()).is_null())).clone()
-    } {
+    if ({
+        let _lhs = (*value.borrow());
+        _lhs < (*(*(*node.borrow()).upgrade().deref()).value.borrow())
+    }) && (!((*(*(*node.borrow()).upgrade().deref()).left.borrow()).is_null()))
+    {
         return ({
             let _node: Ptr<node_t> = (*(*(*node.borrow()).upgrade().deref()).left.borrow()).clone();
             let _value: i32 = (*value.borrow());
             find_0(_node, _value)
         });
-    } else if {
-        let _lhs = {
-            let _lhs = (*value.borrow());
-            _lhs > (*(*(*node.borrow()).upgrade().deref()).value.borrow())
-        };
-        _lhs && (!((*(*(*node.borrow()).upgrade().deref()).right.borrow()).is_null())).clone()
-    } {
+    } else if ({
+        let _lhs = (*value.borrow());
+        _lhs > (*(*(*node.borrow()).upgrade().deref()).value.borrow())
+    }) && (!((*(*(*node.borrow()).upgrade().deref()).right.borrow()).is_null()))
+    {
         return ({
             let _node: Ptr<node_t> =
                 (*(*(*node.borrow()).upgrade().deref()).right.borrow()).clone();

tests/benchmarks/out/refcount/fibonacci.rs

@@ -8,7 +8,7 @@ use std::os::fd::AsFd;
 use std::rc::{Rc, Weak};
 pub fn fib_0(n: u64) -> u64 {
     let n: Value<u64> = Rc::new(RefCell::new(n));
-    return if (((*n.borrow()) == 0_u64) || ((*n.borrow()) == 1_u64)) {
+    return if ((*n.borrow()) == 0_u64) || ((*n.borrow()) == 1_u64) {
         (*n.borrow())
     } else {
         ({

tests/benchmarks/out/refcount/ptr_fibonacci.rs

@@ -8,7 +8,7 @@ use std::os::fd::AsFd;
 use std::rc::{Rc, Weak};
 pub fn fib_0(n: Ptr<u64>) {
     let n: Value<Ptr<u64>> = Rc::new(RefCell::new(n));
-    if ((((*n.borrow()).read()) == 0_u64) || (((*n.borrow()).read()) == 1_u64)) {
+    if (((*n.borrow()).read()) == 0_u64) || (((*n.borrow()).read()) == 1_u64) {
         return;
     }
     let n_1: Value<u64> = Rc::new(RefCell::new(((*n.borrow()).read()).wrapping_sub(1_u64)));

tests/benchmarks/out/refcount/ref_fibonacci.rs

@@ -7,7 +7,7 @@ use std::io::{Read, Seek, Write};
 use std::os::fd::AsFd;
 use std::rc::{Rc, Weak};
 pub fn fib_0(n: Ptr<u64>) {
-    if (((n.read()) == 0_u64) || ((n.read()) == 1_u64)) {
+    if ((n.read()) == 0_u64) || ((n.read()) == 1_u64) {
         return;
     }
     let n_1: Value<u64> = Rc::new(RefCell::new((n.read()).wrapping_sub(1_u64)));

tests/benchmarks/out/unsafe/bst.rs

@@ -14,13 +14,13 @@ pub struct node_t {
     pub value: i32,
 }
 pub unsafe fn find_0(mut node: *mut node_t, mut value: i32) -> *mut node_t {
-    if (((value) < ((*node).value)) && (!(((*node).left).is_null()))) {
+    if ((value) < ((*node).value)) && (!(((*node).left).is_null())) {
         return (unsafe {
             let _node: *mut node_t = (*node).left;
             let _value: i32 = value;
             find_0(_node, _value)
         });
-    } else if (((value) > ((*node).value)) && (!(((*node).right).is_null()))) {
+    } else if ((value) > ((*node).value)) && (!(((*node).right).is_null())) {
         return (unsafe {
             let _node: *mut node_t = (*node).right;
             let _value: i32 = value;

tests/benchmarks/out/unsafe/fibonacci.rs

@@ -7,7 +7,7 @@ use std::io::{Read, Seek, Write};
 use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
 use std::rc::Rc;
 pub unsafe fn fib_0(mut n: u64) -> u64 {
-    return if (((n) == (0_u64)) || ((n) == (1_u64))) {
+    return if ((n) == (0_u64)) || ((n) == (1_u64)) {
         n
     } else {
         (unsafe {