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!: Restrict bit sizes #4235

Merged
merged 8 commits into from
Feb 13, 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
5 changes: 4 additions & 1 deletion aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,10 @@ fn create_context(ty: &str, params: &[Param]) -> Result<Vec<Statement>, AztecMac
add_array_to_hasher(
&id,
&UnresolvedType {
typ: UnresolvedTypeData::Integer(Signedness::Unsigned, 32),
typ: UnresolvedTypeData::Integer(
Signedness::Unsigned,
noirc_frontend::IntegerBitSize::ThirtyTwo,
),
span: None,
},
)
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ impl<'a> FunctionContext<'a> {
let element_types = Self::convert_type(element).flatten();
Type::Array(Rc::new(element_types), *len as usize)
}
ast::Type::Integer(Signedness::Signed, bits) => Type::signed(*bits),
ast::Type::Integer(Signedness::Unsigned, bits) => Type::unsigned(*bits),
ast::Type::Integer(Signedness::Signed, bits) => Type::signed((*bits).into()),
ast::Type::Integer(Signedness::Unsigned, bits) => Type::unsigned((*bits).into()),
ast::Type::Bool => Type::unsigned(1),
ast::Type::String(len) => Type::Array(Rc::new(vec![Type::char()]), *len as usize),
ast::Type::FmtString(_, _) => {
Expand Down
63 changes: 59 additions & 4 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,63 @@ use crate::{
};
use iter_extended::vecmap;

#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Ord, PartialOrd)]
pub enum IntegerBitSize {
One,
Eight,
ThirtyTwo,
SixtyFour,
}

impl IntegerBitSize {
pub fn allowed_sizes() -> Vec<Self> {
vec![Self::One, Self::Eight, Self::ThirtyTwo, Self::SixtyFour]
}
}

impl From<IntegerBitSize> for u32 {
fn from(size: IntegerBitSize) -> u32 {
use IntegerBitSize::*;
match size {
One => 1,
Eight => 8,
ThirtyTwo => 32,
SixtyFour => 64,
}
}
}

pub struct InvalidIntegerBitSizeError(pub u32);

impl TryFrom<u32> for IntegerBitSize {
type Error = InvalidIntegerBitSizeError;

fn try_from(value: u32) -> Result<Self, Self::Error> {
use IntegerBitSize::*;
match value {
1 => Ok(One),
8 => Ok(Eight),
32 => Ok(ThirtyTwo),
64 => Ok(SixtyFour),
_ => Err(InvalidIntegerBitSizeError(value)),
}
}
}

impl core::fmt::Display for IntegerBitSize {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", u32::from(*self))
}
}

/// The parser parses types as 'UnresolvedType's which
/// require name resolution to resolve any type names used
/// for structs within, but are otherwise identical to Types.
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedTypeData {
FieldElement,
Array(Option<UnresolvedTypeExpression>, Box<UnresolvedType>), // [4]Witness = Array(4, Witness)
Integer(Signedness, u32), // u32 = Integer(unsigned, 32)
Integer(Signedness, IntegerBitSize), // u32 = Integer(unsigned, ThirtyTwo)
Bool,
Expression(UnresolvedTypeExpression),
String(Option<UnresolvedTypeExpression>),
Expand Down Expand Up @@ -197,11 +246,17 @@ impl UnresolvedType {
}

impl UnresolvedTypeData {
pub fn from_int_token(token: IntType) -> UnresolvedTypeData {
pub fn from_int_token(
token: IntType,
) -> Result<UnresolvedTypeData, InvalidIntegerBitSizeError> {
use {IntType::*, UnresolvedTypeData::Integer};
match token {
Signed(num_bits) => Integer(Signedness::Signed, num_bits),
Unsigned(num_bits) => Integer(Signedness::Unsigned, num_bits),
Signed(num_bits) => {
Ok(Integer(Signedness::Signed, IntegerBitSize::try_from(num_bits)?))
}
Unsigned(num_bits) => {
Ok(Integer(Signedness::Unsigned, IntegerBitSize::try_from(num_bits)?))
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::hir_def::expr::HirBinaryOp;
use crate::hir_def::types::Type;
use crate::BinaryOpKind;
use crate::FunctionReturnType;
use crate::IntegerBitSize;
use crate::Signedness;

#[derive(Error, Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -67,7 +68,7 @@ pub enum TypeCheckError {
#[error("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?}")]
IntegerSignedness { sign_x: Signedness, sign_y: Signedness, span: Span },
#[error("Integers must have the same bit width LHS is {bit_width_x}, RHS is {bit_width_y}")]
IntegerBitWidth { bit_width_x: u32, bit_width_y: u32, span: Span },
IntegerBitWidth { bit_width_x: IntegerBitSize, bit_width_y: IntegerBitSize, span: Span },
#[error("{kind} cannot be used in an infix operation")]
InvalidInfixOp { kind: &'static str, span: Span },
#[error("{kind} cannot be used in a unary operation")]
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir/type_check/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ impl<'interner> TypeChecker<'interner> {
HirExpression::Literal(HirLiteral::Integer(value, false)) => {
let v = value.to_u128();
if let Type::Integer(_, bit_count) = annotated_type {
let bit_count: u32 = (*bit_count).into();
let max = 1 << bit_count;
if v >= max {
self.errors.push(TypeCheckError::OverflowingAssignment {
Expand Down
13 changes: 8 additions & 5 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::{
use crate::{
hir::type_check::TypeCheckError,
node_interner::{ExprId, NodeInterner, TraitId, TypeAliasId},
IntegerBitSize,
};
use iter_extended::vecmap;
use noirc_errors::{Location, Span};
Expand All @@ -27,8 +28,8 @@ pub enum Type {
Array(Box<Type>, Box<Type>),

/// A primitive integer type with the given sign and bit count.
/// E.g. `u32` would be `Integer(Unsigned, 32)`
Integer(Signedness, u32),
/// E.g. `u32` would be `Integer(Unsigned, ThirtyTwo)`
Integer(Signedness, IntegerBitSize),

/// The primitive `bool` type.
Bool,
Expand Down Expand Up @@ -538,7 +539,7 @@ impl Type {
}

pub fn default_range_loop_type() -> Type {
Type::Integer(Signedness::Unsigned, 64)
Type::Integer(Signedness::Unsigned, IntegerBitSize::SixtyFour)
}

pub fn type_variable(id: TypeVariableId) -> Type {
Expand Down Expand Up @@ -1621,8 +1622,10 @@ impl From<&Type> for PrintableType {
PrintableType::Array { length, typ: Box::new(typ.into()) }
}
Type::Integer(sign, bit_width) => match sign {
Signedness::Unsigned => PrintableType::UnsignedInteger { width: *bit_width },
Signedness::Signed => PrintableType::SignedInteger { width: *bit_width },
Signedness::Unsigned => {
PrintableType::UnsignedInteger { width: (*bit_width).into() }
}
Signedness::Signed => PrintableType::SignedInteger { width: (*bit_width).into() },
},
Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => {
match &*binding.borrow() {
Expand Down
10 changes: 0 additions & 10 deletions compiler/noirc_frontend/src/lexer/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ pub enum LexerErrorKind {
InvalidIntegerLiteral { span: Span, found: String },
#[error("{:?} is not a valid attribute", found)]
MalformedFuncAttribute { span: Span, found: String },
#[error("Integer type is larger than the maximum supported size of u127")]
TooManyBits { span: Span, max: u32, got: u32 },
#[error("Logical and used instead of bitwise and")]
LogicalAnd { span: Span },
#[error("Unterminated block comment")]
Expand All @@ -45,7 +43,6 @@ impl LexerErrorKind {
LexerErrorKind::NotADoubleChar { span, .. } => *span,
LexerErrorKind::InvalidIntegerLiteral { span, .. } => *span,
LexerErrorKind::MalformedFuncAttribute { span, .. } => *span,
LexerErrorKind::TooManyBits { span, .. } => *span,
LexerErrorKind::LogicalAnd { span } => *span,
LexerErrorKind::UnterminatedBlockComment { span } => *span,
LexerErrorKind::UnterminatedStringLiteral { span } => *span,
Expand Down Expand Up @@ -85,13 +82,6 @@ impl LexerErrorKind {
format!(" {found} is not a valid attribute"),
*span,
),
LexerErrorKind::TooManyBits { span, max, got } => (
"Integer literal too large".to_string(),
format!(
"The maximum number of bits needed to represent a field is {max}, This integer type needs {got} bits"
),
*span,
),
LexerErrorKind::LogicalAnd { span } => (
"Noir has no logical-and (&&) operator since short-circuiting is much less efficient when compiling to circuits".to_string(),
"Try `&` instead, or use `if` only if you require short-circuiting".to_string(),
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/lexer/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ impl<'a> Lexer<'a> {

// Check if word an int type
// if no error occurred, then it is either a valid integer type or it is not an int type
let parsed_token = IntType::lookup_int_type(&word, Span::inclusive(start, end))?;
let parsed_token = IntType::lookup_int_type(&word)?;

// Check if it is an int type
if let Some(int_type_token) = parsed_token {
Expand Down
8 changes: 1 addition & 7 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ impl IntType {
// XXX: Result<Option<Token, LexerErrorKind>
// Is not the best API. We could split this into two functions. One that checks if the the
// word is a integer, which only returns an Option
pub(crate) fn lookup_int_type(word: &str, span: Span) -> Result<Option<Token>, LexerErrorKind> {
pub(crate) fn lookup_int_type(word: &str) -> Result<Option<Token>, LexerErrorKind> {
// Check if the first string is a 'u' or 'i'

let is_signed = if word.starts_with('i') {
Expand All @@ -324,12 +324,6 @@ impl IntType {
Err(_) => return Ok(None),
};

let max_bits = FieldElement::max_num_bits() / 2;

if str_as_u32 > max_bits {
return Err(LexerErrorKind::TooManyBits { span, max: max_bits, got: str_as_u32 });
}

if is_signed {
Ok(Some(Token::IntType(IntType::Signed(str_as_u32))))
} else {
Expand Down
7 changes: 4 additions & 3 deletions compiler/noirc_frontend/src/monomorphization/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use noirc_errors::{
};

use crate::{
hir_def::function::FunctionSignature, BinaryOpKind, Distinctness, Signedness, Visibility,
hir_def::function::FunctionSignature, BinaryOpKind, Distinctness, IntegerBitSize, Signedness,
Visibility,
};

/// The monomorphized AST is expression-based, all statements are also
Expand Down Expand Up @@ -217,8 +218,8 @@ pub struct Function {
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum Type {
Field,
Array(/*len:*/ u64, Box<Type>), // Array(4, Field) = [Field; 4]
Integer(Signedness, /*bits:*/ u32), // u32 = Integer(unsigned, 32)
Array(/*len:*/ u64, Box<Type>), // Array(4, Field) = [Field; 4]
Integer(Signedness, /*bits:*/ IntegerBitSize), // u32 = Integer(unsigned, ThirtyTwo)
Bool,
String(/*len:*/ u64), // String(4) = str[4]
FmtString(/*len:*/ u64, Box<Type>),
Expand Down
15 changes: 8 additions & 7 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use crate::{
},
node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitImplKind, TraitMethodId},
token::FunctionAttribute,
ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, TypeVariable,
TypeVariableKind, UnaryOp, Visibility,
ContractFunctionType, FunctionKind, IntegerBitSize, Type, TypeBinding, TypeBindings,
TypeVariable, TypeVariableKind, UnaryOp, Visibility,
};

use self::ast::{Definition, FuncId, Function, LocalId, Program};
Expand Down Expand Up @@ -354,6 +354,7 @@ impl<'interner> Monomorphizer<'interner> {
match typ {
ast::Type::Field => Literal(Integer(-value, typ, location)),
ast::Type::Integer(_, bit_size) => {
let bit_size: u32 = bit_size.into();
let base = 1_u128 << bit_size;
Literal(Integer(FieldElement::from(base) - value, typ, location))
}
Expand Down Expand Up @@ -1109,19 +1110,19 @@ impl<'interner> Monomorphizer<'interner> {
}
"modulus_le_bits" => {
let bits = FieldElement::modulus().to_radix_le(2);
Some(self.modulus_array_literal(bits, 1, location))
Some(self.modulus_array_literal(bits, IntegerBitSize::One, location))
}
"modulus_be_bits" => {
let bits = FieldElement::modulus().to_radix_be(2);
Some(self.modulus_array_literal(bits, 1, location))
Some(self.modulus_array_literal(bits, IntegerBitSize::One, location))
}
"modulus_be_bytes" => {
let bytes = FieldElement::modulus().to_bytes_be();
Some(self.modulus_array_literal(bytes, 8, location))
Some(self.modulus_array_literal(bytes, IntegerBitSize::Eight, location))
}
"modulus_le_bytes" => {
let bytes = FieldElement::modulus().to_bytes_le();
Some(self.modulus_array_literal(bytes, 8, location))
Some(self.modulus_array_literal(bytes, IntegerBitSize::Eight, location))
}
_ => None,
};
Expand All @@ -1133,7 +1134,7 @@ impl<'interner> Monomorphizer<'interner> {
fn modulus_array_literal(
&self,
bytes: Vec<u8>,
arr_elem_bits: u32,
arr_elem_bits: IntegerBitSize,
location: Location,
) -> ast::Expression {
use ast::*;
Expand Down
13 changes: 6 additions & 7 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::lexer::errors::LexerErrorKind;
use crate::lexer::token::Token;
use crate::Expression;
use crate::IntegerBitSize;
use small_ord_set::SmallOrdSet;
use thiserror::Error;

Expand Down Expand Up @@ -40,8 +41,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("Integer bit size {0} isn't supported")]
InvalidBitSize(u32),
#[error("{0}")]
Lexer(LexerErrorKind),
}
Expand Down Expand Up @@ -132,8 +133,6 @@ 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 @@ -149,9 +148,9 @@ 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(", ")),
ParserErrorReason::InvalidBitSize(bit_size) => Diagnostic::simple_error(
format!("Use of invalid bit size {}", bit_size),
format!("Allowed bit sizes for integers are {}", IntegerBitSize::allowed_sizes().iter().map(|n| n.to_string()).collect::<Vec<_>>().join(", ")),
error.span,
),
ParserErrorReason::ExperimentalFeature(_) => Diagnostic::simple_warning(
Expand Down
22 changes: 8 additions & 14 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
//! 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 @@ -36,7 +35,7 @@ use crate::ast::{
};
use crate::lexer::Lexer;
use crate::parser::{force, ignore_then_commit, statement_recovery};
use crate::token::{Attribute, Attributes, IntType, Keyword, SecondaryAttribute, Token, TokenKind};
use crate::token::{Attribute, Attributes, Keyword, SecondaryAttribute, Token, TokenKind};
use crate::{
BinaryOp, BinaryOpKind, BlockExpression, ConstrainKind, ConstrainStatement, Distinctness,
ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident,
Expand Down Expand Up @@ -1093,19 +1092,14 @@ 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
.validate(|(_, token), span, emit| {
UnresolvedTypeData::from_int_token(token)
.map(|data| data.with_span(span))
.unwrap_or_else(|err| {
emit(ParserError::with_reason(ParserErrorReason::InvalidBitSize(err.0), span));
UnresolvedType::error(span)
})
})
.map_with_span(|(_, token), span| UnresolvedTypeData::from_int_token(token).with_span(span))
}

fn named_type(type_parser: impl NoirParser<UnresolvedType>) -> impl NoirParser<UnresolvedType> {
Expand Down
Loading
Loading