Skip to content

Commit

Permalink
feat: Add warnings for usage of restricted bit sizes (#4234)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Breaking change warning for
[#4162](#4162)
## Summary\*



## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [x] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
sirasistant authored Feb 2, 2024
1 parent d646243 commit 0ffc38b
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 96 deletions.
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ pub enum ParserErrorReason {
NoFunctionAttributesAllowedOnStruct,
#[error("Assert statements can only accept string literals")]
AssertMessageNotString,
#[error("Integer bit size {0} won't be supported")]
DeprecatedBitSize(u32),
#[error("{0}")]
Lexer(LexerErrorKind),
}
Expand Down Expand Up @@ -130,6 +132,8 @@ impl std::fmt::Display for ParserError {
}
}

pub(crate) static ALLOWED_INTEGER_BIT_SIZES: &[u32] = &[1, 8, 32, 64];

impl From<ParserError> for Diagnostic {
fn from(error: ParserError) -> Diagnostic {
match error.reason {
Expand All @@ -145,6 +149,11 @@ impl From<ParserError> for Diagnostic {
"The 'comptime' keyword has been deprecated. It can be removed without affecting your program".into(),
error.span,
),
ParserErrorReason::DeprecatedBitSize(bit_size) => Diagnostic::simple_warning(
format!("Use of deprecated bit size {}", bit_size),
format!("Bit sizes for integers will be restricted to {}", ALLOWED_INTEGER_BIT_SIZES.iter().map(|n| n.to_string()).collect::<Vec<_>>().join(", ")),
error.span,
),
ParserErrorReason::ExperimentalFeature(_) => Diagnostic::simple_warning(
reason.to_string(),
"".into(),
Expand Down
15 changes: 14 additions & 1 deletion compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
//! prevent other parsers from being tried afterward since there is no longer an error. Thus, they should
//! be limited to cases like the above `fn` example where it is clear we shouldn't back out of the
//! current parser to try alternative parsers in a `choice` expression.
use super::errors::ALLOWED_INTEGER_BIT_SIZES;
use super::{
foldl_with_span, labels::ParsingRuleLabel, parameter_name_recovery, parameter_recovery,
parenthesized, then_commit, then_commit_ignore, top_level_statement_recovery, ExprParser,
Expand All @@ -35,7 +36,7 @@ use crate::ast::{
};
use crate::lexer::Lexer;
use crate::parser::{force, ignore_then_commit, statement_recovery};
use crate::token::{Attribute, Attributes, Keyword, SecondaryAttribute, Token, TokenKind};
use crate::token::{Attribute, Attributes, IntType, Keyword, SecondaryAttribute, Token, TokenKind};
use crate::{
BinaryOp, BinaryOpKind, BlockExpression, ConstrainKind, ConstrainStatement, Distinctness,
ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident,
Expand Down Expand Up @@ -1092,6 +1093,18 @@ fn int_type() -> impl NoirParser<UnresolvedType> {
Err(ParserError::expected_label(ParsingRuleLabel::IntegerType, unexpected, span))
}
}))
.validate(|int_type, span, emit| {
let bit_size = match int_type.1 {
IntType::Signed(bit_size) | IntType::Unsigned(bit_size) => bit_size,
};
if !ALLOWED_INTEGER_BIT_SIZES.contains(&bit_size) {
emit(ParserError::with_reason(
ParserErrorReason::DeprecatedBitSize(bit_size),
span,
));
}
int_type
})
.map_with_span(|(_, token), span| UnresolvedTypeData::from_int_token(token).with_span(span))
}

Expand Down
26 changes: 0 additions & 26 deletions noir_stdlib/src/cmp.nr
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ impl Eq for Field { fn eq(self, other: Field) -> bool { self == other } }

impl Eq for u1 { fn eq(self, other: u1) -> bool { self == other } }
impl Eq for u8 { fn eq(self, other: u8) -> bool { self == other } }
impl Eq for u16 { fn eq(self, other: u16) -> bool { self == other } }
impl Eq for u32 { fn eq(self, other: u32) -> bool { self == other } }
impl Eq for u64 { fn eq(self, other: u64) -> bool { self == other } }

impl Eq for i8 { fn eq(self, other: i8) -> bool { self == other } }
impl Eq for i16 { fn eq(self, other: i16) -> bool { self == other } }
impl Eq for i32 { fn eq(self, other: i32) -> bool { self == other } }
impl Eq for i64 { fn eq(self, other: i64) -> bool { self == other } }

Expand Down Expand Up @@ -111,18 +109,6 @@ impl Ord for u8 {
}
}

impl Ord for u16 {
fn cmp(self, other: u16) -> Ordering {
if self < other {
Ordering::less()
} else if self > other {
Ordering::greater()
} else {
Ordering::equal()
}
}
}

impl Ord for u32 {
fn cmp(self, other: u32) -> Ordering {
if self < other {
Expand Down Expand Up @@ -159,18 +145,6 @@ impl Ord for i8 {
}
}

impl Ord for i16 {
fn cmp(self, other: i16) -> Ordering {
if self < other {
Ordering::less()
} else if self > other {
Ordering::greater()
} else {
Ordering::equal()
}
}
}

impl Ord for i32 {
fn cmp(self, other: i32) -> Ordering {
if self < other {
Expand Down
9 changes: 0 additions & 9 deletions noir_stdlib/src/convert.nr
Original file line number Diff line number Diff line change
Expand Up @@ -24,37 +24,28 @@ impl<T, U> Into<T> for U where T: From<U> {

// docs:start:from-impls
// Unsigned integers
impl From<u8> for u16 { fn from(value: u8) -> u16 { value as u16 } }

impl From<u8> for u32 { fn from(value: u8) -> u32 { value as u32 } }
impl From<u16> for u32 { fn from(value: u16) -> u32 { value as u32 } }

impl From<u8> for u64 { fn from(value: u8) -> u64 { value as u64 } }
impl From<u16> for u64 { fn from(value: u16) -> u64 { value as u64 } }
impl From<u32> for u64 { fn from(value: u32) -> u64 { value as u64 } }

impl From<u8> for Field { fn from(value: u8) -> Field { value as Field } }
impl From<u16> for Field { fn from(value: u16) -> Field { value as Field } }
impl From<u32> for Field { fn from(value: u32) -> Field { value as Field } }
impl From<u64> for Field { fn from(value: u64) -> Field { value as Field } }

// Signed integers
impl From<i8> for i16 { fn from(value: i8) -> i16 { value as i16 } }

impl From<i8> for i32 { fn from(value: i8) -> i32 { value as i32 } }
impl From<i16> for i32 { fn from(value: i16) -> i32 { value as i32 } }

impl From<i8> for i64 { fn from(value: i8) -> i64 { value as i64 } }
impl From<i16> for i64 { fn from(value: i16) -> i64 { value as i64 } }
impl From<i32> for i64 { fn from(value: i32) -> i64 { value as i64 } }

// Booleans
impl From<bool> for u8 { fn from(value: bool) -> u8 { value as u8 } }
impl From<bool> for u16 { fn from(value: bool) -> u16 { value as u16 } }
impl From<bool> for u32 { fn from(value: bool) -> u32 { value as u32 } }
impl From<bool> for u64 { fn from(value: bool) -> u64 { value as u64 } }
impl From<bool> for i8 { fn from(value: bool) -> i8 { value as i8 } }
impl From<bool> for i16 { fn from(value: bool) -> i16 { value as i16 } }
impl From<bool> for i32 { fn from(value: bool) -> i32 { value as i32 } }
impl From<bool> for i64 { fn from(value: bool) -> i64 { value as i64 } }
impl From<bool> for Field { fn from(value: bool) -> Field { value as Field } }
Expand Down
2 changes: 0 additions & 2 deletions noir_stdlib/src/default.nr
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ trait Default {
impl Default for Field { fn default() -> Field { 0 } }

impl Default for u8 { fn default() -> u8 { 0 } }
impl Default for u16 { fn default() -> u16 { 0 } }
impl Default for u32 { fn default() -> u32 { 0 } }
impl Default for u64 { fn default() -> u64 { 0 } }

impl Default for i8 { fn default() -> i8 { 0 } }
impl Default for i16 { fn default() -> i16 { 0 } }
impl Default for i32 { fn default() -> i32 { 0 } }
impl Default for i64 { fn default() -> i64 { 0 } }

Expand Down
20 changes: 0 additions & 20 deletions noir_stdlib/src/ops.nr
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ trait Add {
impl Add for Field { fn add(self, other: Field) -> Field { self + other } }

impl Add for u8 { fn add(self, other: u8) -> u8 { self + other } }
impl Add for u16 { fn add(self, other: u16) -> u16 { self + other } }
impl Add for u32 { fn add(self, other: u32) -> u32 { self + other } }
impl Add for u64 { fn add(self, other: u64) -> u64 { self + other } }

impl Add for i8 { fn add(self, other: i8) -> i8 { self + other } }
impl Add for i16 { fn add(self, other: i16) -> i16 { self + other } }
impl Add for i32 { fn add(self, other: i32) -> i32 { self + other } }
impl Add for i64 { fn add(self, other: i64) -> i64 { self + other } }

Expand All @@ -25,12 +23,10 @@ trait Sub {
impl Sub for Field { fn sub(self, other: Field) -> Field { self - other } }

impl Sub for u8 { fn sub(self, other: u8) -> u8 { self - other } }
impl Sub for u16 { fn sub(self, other: u16) -> u16 { self - other } }
impl Sub for u32 { fn sub(self, other: u32) -> u32 { self - other } }
impl Sub for u64 { fn sub(self, other: u64) -> u64 { self - other } }

impl Sub for i8 { fn sub(self, other: i8) -> i8 { self - other } }
impl Sub for i16 { fn sub(self, other: i16) -> i16 { self - other } }
impl Sub for i32 { fn sub(self, other: i32) -> i32 { self - other } }
impl Sub for i64 { fn sub(self, other: i64) -> i64 { self - other } }

Expand All @@ -43,12 +39,10 @@ trait Mul {
impl Mul for Field { fn mul(self, other: Field) -> Field { self * other } }

impl Mul for u8 { fn mul(self, other: u8) -> u8 { self * other } }
impl Mul for u16 { fn mul(self, other: u16) -> u16 { self * other } }
impl Mul for u32 { fn mul(self, other: u32) -> u32 { self * other } }
impl Mul for u64 { fn mul(self, other: u64) -> u64 { self * other } }

impl Mul for i8 { fn mul(self, other: i8) -> i8 { self * other } }
impl Mul for i16 { fn mul(self, other: i16) -> i16 { self * other } }
impl Mul for i32 { fn mul(self, other: i32) -> i32 { self * other } }
impl Mul for i64 { fn mul(self, other: i64) -> i64 { self * other } }

Expand All @@ -61,12 +55,10 @@ trait Div {
impl Div for Field { fn div(self, other: Field) -> Field { self / other } }

impl Div for u8 { fn div(self, other: u8) -> u8 { self / other } }
impl Div for u16 { fn div(self, other: u16) -> u16 { self / other } }
impl Div for u32 { fn div(self, other: u32) -> u32 { self / other } }
impl Div for u64 { fn div(self, other: u64) -> u64 { self / other } }

impl Div for i8 { fn div(self, other: i8) -> i8 { self / other } }
impl Div for i16 { fn div(self, other: i16) -> i16 { self / other } }
impl Div for i32 { fn div(self, other: i32) -> i32 { self / other } }
impl Div for i64 { fn div(self, other: i64) -> i64 { self / other } }

Expand All @@ -77,12 +69,10 @@ trait Rem{
// docs:end:rem-trait

impl Rem for u8 { fn rem(self, other: u8) -> u8 { self % other } }
impl Rem for u16 { fn rem(self, other: u16) -> u16 { self % other } }
impl Rem for u32 { fn rem(self, other: u32) -> u32 { self % other } }
impl Rem for u64 { fn rem(self, other: u64) -> u64 { self % other } }

impl Rem for i8 { fn rem(self, other: i8) -> i8 { self % other } }
impl Rem for i16 { fn rem(self, other: i16) -> i16 { self % other } }
impl Rem for i32 { fn rem(self, other: i32) -> i32 { self % other } }
impl Rem for i64 { fn rem(self, other: i64) -> i64 { self % other } }

Expand All @@ -95,12 +85,10 @@ trait BitOr {
impl BitOr for bool { fn bitor(self, other: bool) -> bool { self | other } }

impl BitOr for u8 { fn bitor(self, other: u8) -> u8 { self | other } }
impl BitOr for u16 { fn bitor(self, other: u16) -> u16 { self | other } }
impl BitOr for u32 { fn bitor(self, other: u32) -> u32 { self | other } }
impl BitOr for u64 { fn bitor(self, other: u64) -> u64 { self | other } }

impl BitOr for i8 { fn bitor(self, other: i8) -> i8 { self | other } }
impl BitOr for i16 { fn bitor(self, other: i16) -> i16 { self | other } }
impl BitOr for i32 { fn bitor(self, other: i32) -> i32 { self | other } }
impl BitOr for i64 { fn bitor(self, other: i64) -> i64 { self | other } }

Expand All @@ -113,12 +101,10 @@ trait BitAnd {
impl BitAnd for bool { fn bitand(self, other: bool) -> bool { self & other } }

impl BitAnd for u8 { fn bitand(self, other: u8) -> u8 { self & other } }
impl BitAnd for u16 { fn bitand(self, other: u16) -> u16 { self & other } }
impl BitAnd for u32 { fn bitand(self, other: u32) -> u32 { self & other } }
impl BitAnd for u64 { fn bitand(self, other: u64) -> u64 { self & other } }

impl BitAnd for i8 { fn bitand(self, other: i8) -> i8 { self & other } }
impl BitAnd for i16 { fn bitand(self, other: i16) -> i16 { self & other } }
impl BitAnd for i32 { fn bitand(self, other: i32) -> i32 { self & other } }
impl BitAnd for i64 { fn bitand(self, other: i64) -> i64 { self & other } }

Expand All @@ -131,12 +117,10 @@ trait BitXor {
impl BitXor for bool { fn bitxor(self, other: bool) -> bool { self ^ other } }

impl BitXor for u8 { fn bitxor(self, other: u8) -> u8 { self ^ other } }
impl BitXor for u16 { fn bitxor(self, other: u16) -> u16 { self ^ other } }
impl BitXor for u32 { fn bitxor(self, other: u32) -> u32 { self ^ other } }
impl BitXor for u64 { fn bitxor(self, other: u64) -> u64 { self ^ other } }

impl BitXor for i8 { fn bitxor(self, other: i8) -> i8 { self ^ other } }
impl BitXor for i16 { fn bitxor(self, other: i16) -> i16 { self ^ other } }
impl BitXor for i32 { fn bitxor(self, other: i32) -> i32 { self ^ other } }
impl BitXor for i64 { fn bitxor(self, other: i64) -> i64 { self ^ other } }

Expand All @@ -147,13 +131,11 @@ trait Shl {
// docs:end:shl-trait

impl Shl for u8 { fn shl(self, other: u8) -> u8 { self << other } }
impl Shl for u16 { fn shl(self, other: u16) -> u16 { self << other } }
impl Shl for u32 { fn shl(self, other: u32) -> u32 { self << other } }
impl Shl for u64 { fn shl(self, other: u64) -> u64 { self << other } }

// Bit shifting is not currently supported for signed integer types
// impl Shl for i8 { fn shl(self, other: i8) -> i8 { self << other } }
// impl Shl for i16 { fn shl(self, other: i16) -> i16 { self << other } }
// impl Shl for i32 { fn shl(self, other: i32) -> i32 { self << other } }
// impl Shl for i64 { fn shl(self, other: i64) -> i64 { self << other } }

Expand All @@ -164,12 +146,10 @@ trait Shr {
// docs:end:shr-trait

impl Shr for u8 { fn shr(self, other: u8) -> u8 { self >> other } }
impl Shr for u16 { fn shr(self, other: u16) -> u16 { self >> other } }
impl Shr for u32 { fn shr(self, other: u32) -> u32 { self >> other } }
impl Shr for u64 { fn shr(self, other: u64) -> u64 { self >> other } }

// Bit shifting is not currently supported for signed integer types
// impl Shr for i8 { fn shr(self, other: i8) -> i8 { self >> other } }
// impl Shr for i16 { fn shr(self, other: i16) -> i16 { self >> other } }
// impl Shr for i32 { fn shr(self, other: i32) -> i32 { self >> other } }
// impl Shr for i64 { fn shr(self, other: i64) -> i64 { self >> other } }
26 changes: 9 additions & 17 deletions test_programs/compile_success_empty/brillig_cast/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,25 @@ unconstrained fn bool_casts() {

unconstrained fn field_casts() {
assert(5 as u8 as Field == 5);
assert(16 as u4 as Field == 0);
assert(256 as u8 as Field == 0);
}

unconstrained fn uint_casts() {
let x: u32 = 100;
assert(x as u2 == 0);
assert(x as u4 == 4);
assert(x as u6 == 36);
assert(x as u8 == 100);
assert(x as u64 == 100);
assert(x as u126 == 100);
let x: u32 = 300;
assert(x as u8 == 44);
assert(x as u32 == 300);
assert(x as u64 == 300);
}

unconstrained fn int_casts() {
let x: i32 = 100;
assert(x as i2 == 0);
assert(x as i4 == 4);
assert(x as i6 == -28 as i6);
assert(x as i8 == 100);
assert(x as i8 == 100);
assert(x as i8 == 100);
let x: i32 = 456;
assert(x as i8 == -56 as i8);
assert(x as i64 == 456);
}

unconstrained fn mixed_casts() {
assert(100 as u32 as i32 as u32 == 100);
assert(13 as u4 as i2 as u32 == 1);
assert(15 as u4 as i2 as u32 == 3);
assert(257 as u8 as u32 == 1);
assert(1 as u8 as bool == true);
assert(true as i8 == 1);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ fn main() {
assert(signed_modulo(5, 3) == 2);
assert(signed_modulo(2, 3) == 2);

let minus_two: i4 = -2; // 14
let minus_three: i4 = -3; // 13
let minus_five: i4 = -5; // 11
let minus_two: i8 = -2; // 254
let minus_three: i8 = -3; // 253
let minus_five: i8 = -5; // 251
// (5 / -3) * -3 + 2 = -1 * -3 + 2 = 3 + 2 = 5
assert(signed_modulo(5, minus_three) == 2);
// (-5 / 3) * 3 - 2 = -1 * 3 - 2 = -3 - 2 = -5
Expand All @@ -22,6 +22,6 @@ unconstrained fn modulo(x: u32, y: u32) -> u32 {
x % y
}

unconstrained fn signed_modulo(x: i4, y: i4) -> i4 {
unconstrained fn signed_modulo(x: i8, y: i8) -> i8 {
x % y
}
4 changes: 2 additions & 2 deletions test_programs/execution_success/5_over/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ fn main(mut x: u32, y: u32) {
x = std::wrapping_mul(x,x);
assert(y == x);

let c: u3 = 2;
assert(c > x as u3);
let c: u1 = 0;
assert(x as u1 > c);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main(x: u64) {
//regression for 3481
assert(x << 63 == 0);

assert_eq((1 as u56) << (32 as u56), 0x0100000000);
assert_eq((1 as u64) << (32 as u64), 0x0100000000);
}

fn regression_2250() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// Regression test for https://github.com/noir-lang/noir/issues/3493
fn main(x: u4) {
fn main(x: u8) {
if x == 10 {
x + 15;
x + 255;
}
if x == 9 {
x << 3;
x << 7;
}
if x == 8 {
if x == 128 {
x * 3;
}
if x == 7 {
Expand Down
Loading

0 comments on commit 0ffc38b

Please sign in to comment.