Skip to content

Commit

Permalink
fix: used signed division for signed modulo (#6635)
Browse files Browse the repository at this point in the history
  • Loading branch information
guipublic authored Nov 28, 2024
1 parent b024581 commit dace078
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 4 deletions.
17 changes: 14 additions & 3 deletions compiler/noirc_evaluator/src/acir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
let numeric_type = match typ {
AcirType::NumericType(numeric_type) => numeric_type,
AcirType::Array(_, _) => {
todo!("cannot divide arrays. This should have been caught by the frontend")
unreachable!("cannot divide arrays. This should have been caught by the frontend")
}
};
match numeric_type {
Expand Down Expand Up @@ -1084,11 +1084,22 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
&mut self,
lhs: AcirVar,
rhs: AcirVar,
typ: AcirType,
bit_size: u32,
predicate: AcirVar,
) -> Result<AcirVar, RuntimeError> {
let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?;
Ok(remainder)
let numeric_type = match typ {
AcirType::NumericType(numeric_type) => numeric_type,
AcirType::Array(_, _) => {
unreachable!("cannot modulo arrays. This should have been caught by the frontend")
}
};

let (_, remainder_var) = match numeric_type {
NumericType::Signed { bit_size } => self.signed_division_var(lhs, rhs, bit_size)?,
_ => self.euclidean_division_var(lhs, rhs, bit_size, predicate)?,
};
Ok(remainder_var)
}

/// Constrains the `AcirVar` variable to be of type `NumericType`.
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1968,6 +1968,7 @@ impl<'a> Context<'a> {
BinaryOp::Mod => self.acir_context.modulo_var(
lhs,
rhs,
binary_type.clone(),
bit_count,
self.current_side_effects_enabled_var,
),
Expand Down
11 changes: 11 additions & 0 deletions docs/docs/noir/concepts/data_types/integers.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ fn main() {

The bit size determines the maximum and minimum range of value the integer type can store. For example, an `i8` variable can store a value in the range of -128 to 127 (i.e. $\\-2^{7}\\$ to $\\2^{7}-1\\$).


```rust
fn main(x: i16, y: i16) {
// modulo
let c = x % y;
let c = x % -13;
}
```

Modulo operation is defined for negative integers thanks to integer division, so that the equality `x = (x/y)*y + (x%y)` holds.

## 128 bits Unsigned Integers

The built-in structure `U128` allows you to use 128-bit unsigned integers almost like a native integer type. However, there are some differences to keep in mind:
Expand Down
2 changes: 2 additions & 0 deletions test_programs/execution_success/regression/Prover.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
x = [0x3f, 0x1c, 0xb8, 0x99, 0xab]
z = 3
u = "169"
v = "-13"
10 changes: 9 additions & 1 deletion test_programs/execution_success/regression/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn bitshift_variable(idx: u8) -> u64 {
bits
}

fn main(x: [u8; 5], z: Field) {
fn main(x: [u8; 5], z: Field, u: i16, v: i16) {
//Issue 1144
let (nib, len) = compact_decode(x, z);
assert(len == 5);
Expand Down Expand Up @@ -130,4 +130,12 @@ fn main(x: [u8; 5], z: Field) {
assert(result_0 == 1);
let result_4 = bitshift_variable(4);
assert(result_4 == 16);

// Issue 6609
assert(u % -13 == 0);
assert(u % v == 0);
assert(u % -11 == 4);
assert(-u % -11 == -4);
assert(u % -11 == u % (v + 2));
assert(-u % -11 == -u % (v + 2));
}

0 comments on commit dace078

Please sign in to comment.