Skip to content

Commit

Permalink
fix: Display every bit in integer tokens (#6360)
Browse files Browse the repository at this point in the history
Co-authored-by: Tom French <[email protected]>
  • Loading branch information
jfecher and TomAFrench authored Oct 29, 2024
1 parent e92b519 commit b985fdf
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 76 deletions.
71 changes: 1 addition & 70 deletions acvm-repo/acir_field/src/field_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::ops::{Add, AddAssign, Div, Mul, Neg, Sub, SubAssign};
use crate::AcirField;

// XXX: Switch out for a trait and proper implementations
// This implementation is in-efficient, can definitely remove hex usage and Iterator instances for trivial functionality
// This implementation is inefficient, can definitely remove hex usage and Iterator instances for trivial functionality
#[derive(Default, Clone, Copy, Eq, PartialOrd, Ord)]
pub struct FieldElement<F: PrimeField>(F);

Expand All @@ -33,46 +33,6 @@ impl<F: PrimeField> std::fmt::Display for FieldElement<F> {
write!(f, "-")?;
}

// Number of bits needed to represent the smaller representation
let num_bits = smaller_repr.bits();

// Check if the number represents a power of 2
if smaller_repr.count_ones() == 1 {
let mut bit_index = 0;
for i in 0..num_bits {
if smaller_repr.bit(i) {
bit_index = i;
break;
}
}
return match bit_index {
0 => write!(f, "1"),
1 => write!(f, "2"),
2 => write!(f, "4"),
3 => write!(f, "8"),
_ => write!(f, "2{}", superscript(bit_index)),
};
}

// Check if number is a multiple of a power of 2.
// This is used because when computing the quotient
// we usually have numbers in the form 2^t * q + r
// We focus on 2^64, 2^32, 2^16, 2^8, 2^4 because
// they are common. We could extend this to a more
// general factorization strategy, but we pay in terms of CPU time
let mul_sign = "×";
for power in [64, 32, 16, 8, 4] {
let power_of_two = BigUint::from(2_u128).pow(power);
if &smaller_repr % &power_of_two == BigUint::zero() {
return write!(
f,
"2{}{}{}",
superscript(power as u64),
mul_sign,
smaller_repr / &power_of_two,
);
}
}
write!(f, "{smaller_repr}")
}
}
Expand Down Expand Up @@ -409,35 +369,6 @@ impl<F: PrimeField> SubAssign for FieldElement<F> {
}
}

// For pretty printing powers
fn superscript(n: u64) -> String {
if n == 0 {
"⁰".to_owned()
} else if n == 1 {
"¹".to_owned()
} else if n == 2 {
"²".to_owned()
} else if n == 3 {
"³".to_owned()
} else if n == 4 {
"⁴".to_owned()
} else if n == 5 {
"⁵".to_owned()
} else if n == 6 {
"⁶".to_owned()
} else if n == 7 {
"⁷".to_owned()
} else if n == 8 {
"⁸".to_owned()
} else if n == 9 {
"⁹".to_owned()
} else if n >= 10 {
superscript(n / 10) + &superscript(n % 10)
} else {
panic!("{}", n.to_string() + " can't be converted to superscript.");
}
}

#[cfg(test)]
mod tests {
use super::{AcirField, FieldElement};
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use acvm::{acir::AcirField, FieldElement};
use acvm::FieldElement;
use noirc_errors::{Position, Span, Spanned};
use std::fmt;

Expand Down Expand Up @@ -367,7 +367,7 @@ impl fmt::Display for Token {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Token::Ident(ref s) => write!(f, "{s}"),
Token::Int(n) => write!(f, "{}", n.to_u128()),
Token::Int(n) => write!(f, "{}", n),
Token::Bool(b) => write!(f, "{b}"),
Token::Str(ref b) => write!(f, "{b:?}"),
Token::FmtStr(ref b) => write!(f, "f{b:?}"),
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/tests/bound_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn overflowing_u8() {
if let CompilationError::TypeError(error) = &errors[0].0 {
assert_eq!(
error.to_string(),
"The value `2⁸` cannot fit into `u8` which has range `0..=255`"
"The value `256` cannot fit into `u8` which has range `0..=255`"
);
} else {
panic!("Expected OverflowingAssignment error, got {:?}", errors[0].0);
Expand Down Expand Up @@ -52,7 +52,7 @@ fn overflowing_i8() {
if let CompilationError::TypeError(error) = &errors[0].0 {
assert_eq!(
error.to_string(),
"The value `2⁷` cannot fit into `i8` which has range `-128..=127`"
"The value `128` cannot fit into `i8` which has range `-128..=127`"
);
} else {
panic!("Expected OverflowingAssignment error, got {:?}", errors[0].0);
Expand Down
2 changes: 1 addition & 1 deletion tooling/noirc_abi_wasm/test/browser/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ it('errors when an integer input overflows', async () => {
const { abi, inputs } = await import('../shared/uint_overflow');

expect(() => abiEncode(abi, inputs)).to.throw(
'The value passed for parameter `foo` does not match the specified type:\nValue Field(2³⁸) does not fall within range of allowable values for a Integer { sign: Unsigned, width: 32 }',
'The value passed for parameter `foo` does not match the specified type:\nValue Field(274877906944) does not fall within range of allowable values for a Integer { sign: Unsigned, width: 32 }',
);
});

Expand Down
2 changes: 1 addition & 1 deletion tooling/noirc_abi_wasm/test/node/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ it('errors when an integer input overflows', async () => {
const { abi, inputs } = await import('../shared/uint_overflow');

expect(() => abiEncode(abi, inputs)).to.throw(
'The value passed for parameter `foo` does not match the specified type:\nValue Field(2³⁸) does not fall within range of allowable values for a Integer { sign: Unsigned, width: 32 }',
'The value passed for parameter `foo` does not match the specified type:\nValue Field(274877906944) does not fall within range of allowable values for a Integer { sign: Unsigned, width: 32 }',
);
});

Expand Down

0 comments on commit b985fdf

Please sign in to comment.