fix!: Integer division is not the inverse of integer multiplication

compiler/noirc_frontend/src/hir_def/types.rs

@@ -1643,7 +1643,7 @@ impl Type {
                         Err(UnificationError)
                     }
                 } else if let InfixExpr(lhs, op, rhs) = other {
-                    if let Some(inverse) = op.inverse() {
+                    if let Some(inverse) = op.approx_inverse() {
                         // Handle cases like `4 = a + b` by trying to solve to `a = 4 - b`
                         let new_type = InfixExpr(
                             Box::new(Constant(*value, kind.clone())),
@@ -2368,6 +2368,17 @@ impl BinaryTypeOperator {
 
     /// Return the operator that will "undo" this operation if applied to the rhs
     fn inverse(self) -> Option<BinaryTypeOperator> {
+        match self {
+            BinaryTypeOperator::Addition => Some(BinaryTypeOperator::Subtraction),
+            BinaryTypeOperator::Subtraction => Some(BinaryTypeOperator::Addition),
+            BinaryTypeOperator::Multiplication => None,
+            BinaryTypeOperator::Division => None,
+            BinaryTypeOperator::Modulo => None,
+        }
+    }
+
+    /// Return the operator that will "undo" this operation if applied to the rhs
+    fn approx_inverse(self) -> Option<BinaryTypeOperator> {
         match self {
             BinaryTypeOperator::Addition => Some(BinaryTypeOperator::Subtraction),
             BinaryTypeOperator::Subtraction => Some(BinaryTypeOperator::Addition),

compiler/noirc_frontend/src/hir_def/types/arithmetic.rs

@@ -113,7 +113,7 @@ impl Type {
     /// Precondition: `lhs & rhs are in canonical form`
     ///
     /// - Simplifies `(N +/- M) -/+ M` to `N`
-    /// - Simplifies `(N */÷ M) ÷/* M` to `N`
+    /// - Simplifies `(N * M) ÷ M` to `N`
     fn try_simplify_non_constants_in_lhs(
         lhs: &Type,
         op: BinaryTypeOperator,
@@ -125,7 +125,10 @@ impl Type {
 
         // Note that this is exact, syntactic equality, not unification.
         // `rhs` is expected to already be in canonical form.
-        if l_op.inverse() != Some(op) || l_rhs.canonicalize() != *rhs {
+        if l_op.approx_inverse() != Some(op)
+            || l_op == BinaryTypeOperator::Division
+            || l_rhs.canonicalize() != *rhs
+        {
             return None;
         }
 
@@ -191,7 +194,8 @@ impl Type {
     /// Precondition: `lhs & rhs are in canonical form`
     ///
     /// - Simplifies `(N +/- C1) +/- C2` to `N +/- (C1 +/- C2)` if C1 and C2 are constants.
-    /// - Simplifies `(N */÷ C1) */÷ C2` to `N */÷ (C1 */÷ C2)` if C1 and C2 are constants.
+    /// - Simplifies `(N * C1) ÷ C2` to `N * (C1 ÷ C2)` if C1 and C2 are constants which divide
+    ///   without a remainder.
     fn try_simplify_partial_constants(
         lhs: &Type,
         mut op: BinaryTypeOperator,
@@ -210,13 +214,9 @@ impl Type {
                 let constant = Type::Constant(result, lhs.infix_kind(rhs));
                 Some(Type::InfixExpr(l_type, l_op, Box::new(constant)))
             }
-            (Multiplication | Division, Multiplication | Division) => {
-                // If l_op is a division we want to inverse the rhs operator.
-                if l_op == Division {
-                    op = op.inverse()?;
-                }
-                // If op is a division we need to ensure it divides evenly
-                if op == Division && (r_const == 0 || l_const % r_const != 0) {
+            (Multiplication, Division) => {
+                // We need to ensure the result divides evenly to preserve integer division semantics
+                if r_const == 0 || l_const % r_const != 0 {
                     None
                 } else {
                     let result = op.function(l_const, r_const, &lhs.infix_kind(rhs))?;
@@ -236,7 +236,7 @@ impl Type {
         bindings: &mut TypeBindings,
     ) -> Result<(), UnificationError> {
         if let Type::InfixExpr(lhs_a, op_a, rhs_a) = self {
-            if let Some(inverse) = op_a.inverse() {
+            if let Some(inverse) = op_a.approx_inverse() {
                 if let Some(rhs_a_u32) = rhs_a.evaluate_to_u32() {
                     let rhs_a = Box::new(Type::Constant(rhs_a_u32, lhs_a.infix_kind(rhs_a)));
                     let new_other = Type::InfixExpr(Box::new(other.clone()), inverse, rhs_a);
@@ -251,7 +251,7 @@ impl Type {
         }
 
         if let Type::InfixExpr(lhs_b, op_b, rhs_b) = other {
-            if let Some(inverse) = op_b.inverse() {
+            if let Some(inverse) = op_b.approx_inverse() {
                 if let Some(rhs_b_u32) = rhs_b.evaluate_to_u32() {
                     let rhs_b = Box::new(Type::Constant(rhs_b_u32, lhs_b.infix_kind(rhs_b)));
                     let new_self = Type::InfixExpr(Box::new(self.clone()), inverse, rhs_b);

compiler/noirc_frontend/src/tests.rs

@@ -3219,3 +3219,34 @@ fn error_if_attribute_not_in_scope() {
         CompilationError::ResolverError(ResolverError::AttributeFunctionNotInScope { .. })
     ));
 }
+
+#[test]
+fn arithmetic_generics_rounding_pass() {
+    let src = r#"
+        fn main() {
+            // 3/2*2 = 2
+            round::<3, 2>([1, 2]);
+        }
+
+        fn round<let N: u32, let M: u32>(_x: [Field; N / M * M]) {}
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 0);
+}
+
+#[test]
+fn arithmetic_generics_rounding_fail() {
+    let src = r#"
+        fn main() {
+            // Do not simplify N/M*M to just N
+            // This should be 3/2*2 = 2, not 3
+            round::<3, 2>([1, 2, 3]);
+        }
+
+        fn round<let N: u32, let M: u32>(_x: [Field; N / M * M]) {}
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+}