Wrap BinaryOperator in integer cast in C mode

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 {

tests/benchmarks/out/unsafe/ptr_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: *mut u64) {
-    if (((*n) == (0_u64)) || ((*n) == (1_u64))) {
+    if ((*n) == (0_u64)) || ((*n) == (1_u64)) {
         return;
     }
     let mut n_1: u64 = (*n).wrapping_sub(1_u64);

tests/benchmarks/out/unsafe/ref_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(n: *mut u64) {
-    if (((*n) == (0_u64)) || ((*n) == (1_u64))) {
+    if ((*n) == (0_u64)) || ((*n) == (1_u64)) {
         return;
     }
     let mut n_1: u64 = (*n).wrapping_sub(1_u64);

tests/ub/out/refcount/ub6.rs

@@ -48,13 +48,11 @@ pub fn any_2(arr: Ptr<Option<Value<Box<[Ptr<i32>]>>>>, n1: Ptr<i32>) -> bool {
         let _lhs = (*i.borrow());
         _lhs < (n1.read())
     } {
-        let __rhs = {
-            let _lhs = (*out.borrow());
-            _lhs || (((*arr.upgrade().deref()).as_ref().unwrap().borrow()
+        let __rhs = (*out.borrow())
+            || (((*arr.upgrade().deref()).as_ref().unwrap().borrow()
                 [((*i.borrow()) as u64) as usize]
                 .read())
-                == 0)
-        };
+                == 0);
         (*out.borrow_mut()) = __rhs;
         (*i.borrow_mut()).prefix_inc();
     }

tests/ub/out/unsafe/ub6.rs

@@ -29,7 +29,7 @@ pub unsafe fn any_2(arr: *mut Option<Box<[*mut i32]>>, n1: *mut i32) -> bool {
     let mut out: bool = false;
     let mut i: i32 = 0;
     'loop_: while ((i) < (*n1)) {
-        out = ((out) || ((*(*arr).as_mut().unwrap()[(i as u64) as usize]) == (0)));
+        out = (out) || ((*(*arr).as_mut().unwrap()[(i as u64) as usize]) == (0));
         i.prefix_inc();
     }
     return out;

tests/unit/bool_condition_logical.cpp

@@ -9,6 +9,9 @@ int observe(int v) {
   return v;
 }
 
+int returns_one() { return 1; }
+int returns_zero() { return 0; }
+
 int main() {
   int n = 3;
   int zero = 0;
@@ -77,5 +80,50 @@ int main() {
     assert(true);
   }
 
+  unsigned long long ull = 7ull;
+  if ((p != nullptr) && ull) {
+    assert(true);
+  }
+  if ((x > y) && ull) {
+    assert(true);
+  }
+
+  long long mask = (1ll << 4) | (1ll << 5);
+  long long bits = 1ll << 4;
+  if ((n != 0) && (bits & mask)) {
+    assert(true);
+  }
+  if ((n != 0) || (bits & 0x100ll)) {
+    assert(true);
+  }
+
+  const char *cp = "hi";
+  const char *cnp = nullptr;
+  if ((x > y) && cp) {
+    assert(true);
+  }
+  if ((x < y) || cnp) {
+    assert(false);
+  }
+  if ((x > y) && (n && cp)) {
+    assert(true);
+  }
+
+  if ((x > y) && returns_one()) {
+    assert(true);
+  }
+  if ((x > y) && !returns_zero()) {
+    assert(true);
+  }
+  if ((x < y) || returns_one()) {
+    assert(true);
+  }
+  if ((x < y) || !returns_one()) {
+    assert(false);
+  }
+  if ((p != nullptr) && returns_one() && (n != 0)) {
+    assert(true);
+  }
+
   return 0;
 }