transpile: Add CBinOp methods for querying operator kinds (#1790)

ahomescu · web-flow · commit 37d453438dd2 · 2026-05-12T14:42:04.000-07:00
Some less repetition, cleaner code.
diff --git a/c2rust-transpile/src/c_ast/mod.rs b/c2rust-transpile/src/c_ast/mod.rs
@@ -1279,7 +1279,7 @@ impl TypedAstContext {
                             } else {
                                 Some(rhs_type_id)
                             }
-                        } else if op == CBinOp::ShiftLeft || op == CBinOp::ShiftRight {
+                        } else if op.is_bitshift() {
                             Some(lhs_type_id)
                         } else {
                             return;
@@ -2164,28 +2164,81 @@ impl CBinOp {
         }
     }
 
-    /// Does the rust equivalent of this operator have type (T, T) -> U?
-    #[rustfmt::skip]
-    pub fn input_types_same(&self) -> bool {
+    /// Is this a (non-assignment) arithmetic operator?
+    pub fn is_arithmetic(&self) -> bool {
         use CBinOp::*;
-        self.all_types_same() || matches!(self,
-            Less | Greater | LessEqual | GreaterEqual | EqualEqual | NotEqual
-            | And | Or
-            | AssignAdd | AssignSubtract | AssignMultiply | AssignDivide | AssignModulus
-            | AssignBitXor | AssignShiftLeft | AssignShiftRight | AssignBitOr | AssignBitAnd
-            | Assign
-        )
+        matches!(self, Add | Subtract | Multiply | Divide | Modulus)
     }
 
-    /// Does the rust equivalent of this operator have type (T, T) -> T?
-    /// This ignores cases where one argument is a pointer and we translate to `.offset()`.
-    pub fn all_types_same(&self) -> bool {
+    /// Is this a (non-assignment) arithmetic operator that can be used with pointers?
+    pub fn is_pointer_arithmetic(&self) -> bool {
+        use CBinOp::*;
+        matches!(self, Add | Subtract)
+    }
+
+    /// Is this a (non-assignment, non-shift) bitwise operator?
+    pub fn is_bitwise(&self) -> bool {
+        use CBinOp::*;
+        matches!(self, BitAnd | BitOr | BitXor)
+    }
+
+    /// Is this a (non-assignment) bitshift operator?
+    pub fn is_bitshift(&self) -> bool {
+        use CBinOp::*;
+        matches!(self, ShiftLeft | ShiftRight)
+    }
+
+    /// Is this a logical operator?
+    pub fn is_logical(&self) -> bool {
+        use CBinOp::*;
+        matches!(self, And | Or)
+    }
+
+    /// Is this a comparison operator?
+    pub fn is_comparison(&self) -> bool {
         use CBinOp::*;
         matches!(
             self,
-            Multiply | Divide | Modulus | Add | Subtract | BitAnd | BitXor | BitOr
+            EqualEqual | NotEqual | Less | Greater | LessEqual | GreaterEqual
         )
     }
+
+    /// Is this a (simple or compound) assignment operator?
+    pub fn is_assignment(&self) -> bool {
+        matches!(self, Self::Assign) || self.underlying_assignment().is_some()
+    }
+
+    /// Maps compound assignment operators to operator underlying them, and returns `None` for all
+    /// other operators.
+    ///
+    /// For example, `AssignAdd` maps to `Some(Add)` but `Add` maps to `None`.
+    pub fn underlying_assignment(&self) -> Option<CBinOp> {
+        use CBinOp::*;
+        Some(match *self {
+            AssignAdd => Add,
+            AssignSubtract => Subtract,
+            AssignMultiply => Multiply,
+            AssignDivide => Divide,
+            AssignModulus => Modulus,
+            AssignBitXor => BitXor,
+            AssignShiftLeft => ShiftLeft,
+            AssignShiftRight => ShiftRight,
+            AssignBitOr => BitOr,
+            AssignBitAnd => BitAnd,
+            _ => return None,
+        })
+    }
+
+    /// Does the rust equivalent of this operator have type (T, T) -> U?
+    pub fn input_types_same(&self) -> bool {
+        self.all_types_same() || self.is_logical() || self.is_comparison() || self.is_assignment()
+    }
+
+    /// Does the rust equivalent of this operator have type (T, T) -> T?
+    /// This ignores cases where one argument is a pointer and we translate to `.offset()`.
+    pub fn all_types_same(&self) -> bool {
+        self.is_arithmetic() || self.is_bitwise()
+    }
 }
 
 impl From<CBinOp> for BinOp {
@@ -2232,34 +2285,6 @@ impl Display for CBinOp {
     }
 }
 
-impl CBinOp {
-    /// Maps compound assignment operators to operator underlying them, and returns `None` for all
-    /// other operators.
-    ///
-    /// For example, `AssignAdd` maps to `Some(Add)` but `Add` maps to `None`.
-    pub fn underlying_assignment(&self) -> Option<CBinOp> {
-        use CBinOp::*;
-        Some(match *self {
-            AssignAdd => Add,
-            AssignSubtract => Subtract,
-            AssignMultiply => Multiply,
-            AssignDivide => Divide,
-            AssignModulus => Modulus,
-            AssignBitXor => BitXor,
-            AssignShiftLeft => ShiftLeft,
-            AssignShiftRight => ShiftRight,
-            AssignBitOr => BitOr,
-            AssignBitAnd => BitAnd,
-            _ => return None,
-        })
-    }
-
-    /// Determines whether or not this is an assignment op
-    pub fn is_assignment(&self) -> bool {
-        matches!(self, Self::Assign) || self.underlying_assignment().is_some()
-    }
-}
-
 #[derive(Eq, PartialEq, Debug, Copy, Clone)]
 pub enum IntBase {
     Dec,
diff --git a/c2rust-transpile/src/translator/mod.rs b/c2rust-transpile/src/translator/mod.rs
@@ -1638,7 +1638,6 @@ impl<'c> Translation<'c> {
         expr_id: Option<CExprId>,
         qtype: CQualTypeId,
     ) -> bool {
-        use crate::c_ast::CBinOp::{Add, Divide, Modulus, Multiply, Subtract};
         use crate::c_ast::CUnOp::{AddressOf, Negate};
         use crate::c_ast::CastKind::{IntegralToPointer, PointerToIntegral};
 
@@ -1687,9 +1686,7 @@ impl<'c> Translation<'c> {
                 | ExplicitCast(_, _, PointerToIntegral, _, _) => return true,
 
                 Binary(typ, op, _, _, _, _) => {
-                    let problematic_op = matches!(op, Add | Subtract | Multiply | Divide | Modulus);
-
-                    if problematic_op {
+                    if op.is_arithmetic() {
                         let k = &self.ast_context.resolve_type(typ.ctype).kind;
                         if k.is_unsigned_integral_type() || k.is_pointer() {
                             return true;
diff --git a/c2rust-transpile/src/translator/operators.rs b/c2rust-transpile/src/translator/operators.rs
@@ -29,7 +29,7 @@ impl<'c> Translation<'c> {
                     .and_then_try(|_| self.convert_expr(ctx, rhs, Some(expr_type_id)))
             }
 
-            And | Or => {
+            op if op.is_logical() => {
                 let lhs = self.convert_condition(ctx, true, lhs)?;
                 let rhs = self.convert_condition(ctx, true, rhs)?;
                 Ok(lhs
@@ -47,9 +47,7 @@ impl<'c> Translation<'c> {
             }
 
             // No sequence-point cases
-            AssignAdd | AssignSubtract | AssignMultiply | AssignDivide | AssignModulus
-            | AssignBitXor | AssignShiftLeft | AssignShiftRight | AssignBitOr | AssignBitAnd
-            | Assign => self.convert_assignment_operator(
+            op if op.is_assignment() => self.convert_assignment_operator(
                 ctx,
                 op,
                 expr_type_id,
@@ -133,7 +131,7 @@ impl<'c> Translation<'c> {
                     // When we use methods on pointers (ie wrapping_offset_from or offset)
                     // we must ensure we have an explicit raw ptr for the self param, as
                     // self references do not decay
-                    if op == CBinOp::Subtract || op == CBinOp::Add {
+                    if op.is_pointer_arithmetic() {
                         let ty_kind = &self.ast_context.resolve_type(lhs_type_id.ctype).kind;
 
                         if let CTypeKind::Pointer(_) = ty_kind {
@@ -287,21 +285,13 @@ impl<'c> Translation<'c> {
             let neither_ptr =
                 !lhs_resolved_ty.kind.is_pointer() && !rhs_resolved_ty.kind.is_pointer();
 
-            use CBinOp::*;
-            match op.underlying_assignment() {
-                Some(Add) => neither_ptr,
-                Some(Subtract) => neither_ptr,
-                Some(Multiply) => true,
-                Some(Divide) => true,
-                Some(Modulus) => true,
-                Some(BitXor) => true,
-                Some(ShiftLeft) => false,
-                Some(ShiftRight) => false,
-                Some(BitOr) => true,
-                Some(BitAnd) => true,
-                None => true,
-                _ => unreachable!(),
-            }
+            op.underlying_assignment().map_or(true, |op| {
+                if op.is_pointer_arithmetic() {
+                    neither_ptr
+                } else {
+                    op.is_arithmetic() || op.is_bitwise()
+                }
+            })
         };
         if lhs_rhs_types_must_match {
             // For compound assignment, use the compute type; for regular assignment, use lhs type
@@ -388,14 +378,10 @@ impl<'c> Translation<'c> {
             _ => None,
         };
 
-        let is_unsigned_arith = match op {
-            CBinOp::AssignAdd
-            | CBinOp::AssignSubtract
-            | CBinOp::AssignMultiply
-            | CBinOp::AssignDivide
-            | CBinOp::AssignModulus => compute_resolved_ty.kind.is_unsigned_integral_type(),
-            _ => false,
-        };
+        let is_unsigned_arith = op
+            .underlying_assignment()
+            .map_or(false, |op| op.is_arithmetic())
+            && compute_resolved_ty.kind.is_unsigned_integral_type();
 
         let lhs_translation = if initial_lhs_type_id.ctype != expr_or_comp_type_id.ctype
             || ctx.is_used()
@@ -545,25 +531,15 @@ impl<'c> Translation<'c> {
             CBinOp::Add => return self.convert_addition(lhs_type, rhs_type, lhs, rhs),
             CBinOp::Subtract => return self.convert_subtraction(ty, lhs_type, rhs_type, lhs, rhs),
 
-            CBinOp::Multiply | CBinOp::Divide | CBinOp::Modulus if is_unsigned_integral_type => {
+            op if op.is_arithmetic() && is_unsigned_integral_type => {
                 mk().method_call_expr(lhs, op.wrapping_method(), vec![rhs])
             }
 
-            CBinOp::Multiply
-            | CBinOp::Divide
-            | CBinOp::Modulus
-            | CBinOp::BitAnd
-            | CBinOp::BitOr
-            | CBinOp::BitXor
-            | CBinOp::ShiftRight
-            | CBinOp::ShiftLeft => mk().binary_expr(BinOp::from(op), lhs, rhs),
-
-            CBinOp::EqualEqual
-            | CBinOp::NotEqual
-            | CBinOp::Less
-            | CBinOp::Greater
-            | CBinOp::GreaterEqual
-            | CBinOp::LessEqual => bool_to_int(mk().binary_expr(BinOp::from(op), lhs, rhs)),
+            op if op.is_arithmetic() || op.is_bitwise() || op.is_bitshift() => {
+                mk().binary_expr(BinOp::from(op), lhs, rhs)
+            }
+
+            op if op.is_comparison() => bool_to_int(mk().binary_expr(BinOp::from(op), lhs, rhs)),
 
             op => unimplemented!("Translation of binary operator {:?}", op),
         }))
