Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add warnings for usage of restricted bit sizes #4234

Merged
merged 1 commit into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 } }
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
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
Loading