From 2413f4ec301d1ebcc7cf0e7abb49a0f5afe12ba1 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 24 Feb 2024 21:43:38 +0000 Subject: [PATCH 01/67] feat: add mvp of unsafe blocks --- aztec_macros/src/lib.rs | 36 +++++--- compiler/noirc_frontend/src/ast/expression.rs | 37 +++++--- compiler/noirc_frontend/src/ast/function.rs | 2 +- compiler/noirc_frontend/src/ast/statement.rs | 16 +++- compiler/noirc_frontend/src/debug/mod.rs | 59 +++++++----- .../src/hir/resolution/resolver.rs | 8 +- .../src/hir/type_check/errors.rs | 4 +- .../noirc_frontend/src/hir/type_check/expr.rs | 91 ++++++++++++------- .../noirc_frontend/src/hir/type_check/mod.rs | 9 +- .../noirc_frontend/src/hir/type_check/stmt.rs | 75 +++++++++------ compiler/noirc_frontend/src/hir_def/expr.rs | 9 +- compiler/noirc_frontend/src/lexer/token.rs | 4 + .../src/monomorphization/mod.rs | 2 +- compiler/noirc_frontend/src/parser/parser.rs | 54 ++++++----- noir_stdlib/src/array.nr | 21 +++-- noir_stdlib/src/collections/map.nr | 2 +- noir_stdlib/src/field/bn254.nr | 37 +++++--- noir_stdlib/src/lib.nr | 18 +++- noir_stdlib/src/option.nr | 2 +- noir_stdlib/src/uint128.nr | 25 +++-- noir_stdlib/src/{unsafe.nr => unsafe_func.nr} | 0 tooling/nargo_fmt/src/visitor/expr.rs | 4 +- 22 files changed, 329 insertions(+), 186 deletions(-) rename noir_stdlib/src/{unsafe.nr => unsafe_func.nr} (100%) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 09deb2c9712..ba577d293ff 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -600,7 +600,7 @@ fn generate_storage_implementation(module: &mut SortedModule) -> Result<(), Azte true, )), )], - &BlockExpression(vec![storage_constructor_statement]), + &BlockExpression { is_unsafe: false, statements: vec![storage_constructor_statement] }, &[], &return_type(chained_path!("Self")), )); @@ -635,12 +635,12 @@ fn transform_function( // Add access to the storage struct if storage_defined { let storage_def = abstract_storage(&ty.to_lowercase(), false); - func.def.body.0.insert(0, storage_def); + func.def.body.statements.insert(0, storage_def); } // Insert the context creation as the first action let create_context = create_context(&context_name, &func.def.parameters)?; - func.def.body.0.splice(0..0, (create_context).iter().cloned()); + func.def.body.statements.splice(0..0, (create_context).iter().cloned()); // Add the inputs to the params let input = create_inputs(&inputs_name); @@ -650,14 +650,14 @@ fn transform_function( if let Some(return_values) = abstract_return_values(func) { // In case we are pushing return values to the context, we remove the statement that originated it // This avoids running duplicate code, since blocks like if/else can be value returning statements - func.def.body.0.pop(); + func.def.body.statements.pop(); // Add the new return statement - func.def.body.0.push(return_values); + func.def.body.statements.push(return_values); } // Push the finish method call to the end of the function let finish_def = create_context_finish(); - func.def.body.0.push(finish_def); + func.def.body.statements.push(finish_def); let return_type = create_return_type(&return_type_name); func.def.return_type = return_type; @@ -681,7 +681,7 @@ fn transform_vm_function( ) -> Result<(), AztecMacroError> { // Push Avm context creation to the beginning of the function let create_context = create_avm_context()?; - func.def.body.0.insert(0, create_context); + func.def.body.statements.insert(0, create_context); // We want the function to be seen as a public function func.def.is_open = true; @@ -701,7 +701,7 @@ fn transform_vm_function( /// /// This will allow developers to access their contract' storage struct in unconstrained functions fn transform_unconstrained(func: &mut NoirFunction) { - func.def.body.0.insert(0, abstract_storage("Unconstrained", true)); + func.def.body.statements.insert(0, abstract_storage("Unconstrained", true)); } fn collect_crate_structs(crate_id: &CrateId, context: &HirContext) -> Vec { @@ -1032,10 +1032,15 @@ fn generate_selector_impl(structure: &NoirStruct) -> TypeImpl { let mut from_signature_path = selector_path.clone(); from_signature_path.segments.push(ident("from_signature")); - let selector_fun_body = BlockExpression(vec![make_statement(StatementKind::Expression(call( - variable_path(from_signature_path), - vec![expression(ExpressionKind::Literal(Literal::Str(SIGNATURE_PLACEHOLDER.to_string())))], - )))]); + let selector_fun_body = BlockExpression { + is_unsafe: false, + statements: vec![make_statement(StatementKind::Expression(call( + variable_path(from_signature_path), + vec![expression(ExpressionKind::Literal(Literal::Str( + SIGNATURE_PLACEHOLDER.to_string(), + )))], + )))], + }; // Define `FunctionSelector` return type let return_type = @@ -1259,7 +1264,7 @@ fn create_avm_context() -> Result { /// Any primitive type that can be cast will be casted to a field and pushed to the context. fn abstract_return_values(func: &NoirFunction) -> Option { let current_return_type = func.return_type().typ; - let last_statement = func.def.body.0.last()?; + let last_statement = func.def.body.statements.last()?; // TODO: (length, type) => We can limit the size of the array returned to be limited by kernel size // Doesn't need done until we have settled on a kernel size @@ -1515,7 +1520,10 @@ fn create_loop_over(var: Expression, loop_body: Vec) -> Statement { // What will be looped over // - `hasher.add({ident}[i] as Field)` - let for_loop_block = expression(ExpressionKind::Block(BlockExpression(loop_body))); + let for_loop_block = expression(ExpressionKind::Block(BlockExpression { + is_unsafe: false, + statements: loop_body, + })); // `for i in 0..{ident}.len()` make_statement(StatementKind::For(ForLoopStatement { diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 2a252633a29..dd4eca24784 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -180,16 +180,19 @@ impl Expression { // with tuples without calling them. E.g. `if c { t } else { e }(a, b)` is interpreted // as a sequence of { if, tuple } rather than a function call. This behavior matches rust. let kind = if matches!(&lhs.kind, ExpressionKind::If(..)) { - ExpressionKind::Block(BlockExpression(vec![ - Statement { kind: StatementKind::Expression(lhs), span }, - Statement { - kind: StatementKind::Expression(Expression::new( - ExpressionKind::Tuple(arguments), + ExpressionKind::Block(BlockExpression { + is_unsafe: false, + statements: vec![ + Statement { kind: StatementKind::Expression(lhs), span }, + Statement { + kind: StatementKind::Expression(Expression::new( + ExpressionKind::Tuple(arguments), + span, + )), span, - )), - span, - }, - ])) + }, + ], + }) } else { ExpressionKind::Call(Box::new(CallExpression { func: Box::new(lhs), arguments })) }; @@ -456,19 +459,22 @@ pub struct IndexExpression { } #[derive(Debug, PartialEq, Eq, Clone)] -pub struct BlockExpression(pub Vec); +pub struct BlockExpression { + pub is_unsafe: bool, + pub statements: Vec, +} impl BlockExpression { pub fn pop(&mut self) -> Option { - self.0.pop().map(|stmt| stmt.kind) + self.statements.pop().map(|stmt| stmt.kind) } pub fn len(&self) -> usize { - self.0.len() + self.statements.len() } pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.statements.is_empty() } } @@ -537,8 +543,9 @@ impl Display for Literal { impl Display for BlockExpression { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{{")?; - for statement in &self.0 { + let safety = if self.is_unsafe { "unsafe " } else { "" }; + writeln!(f, "{safety}{{")?; + for statement in &self.statements { let statement = statement.kind.to_string(); for line in statement.lines() { writeln!(f, " {line}")?; diff --git a/compiler/noirc_frontend/src/ast/function.rs b/compiler/noirc_frontend/src/ast/function.rs index 46f0ac0fa0f..3e8b78c1312 100644 --- a/compiler/noirc_frontend/src/ast/function.rs +++ b/compiler/noirc_frontend/src/ast/function.rs @@ -83,7 +83,7 @@ impl NoirFunction { &mut self.def } pub fn number_of_statements(&self) -> usize { - self.def.body.0.len() + self.def.body.statements.len() } pub fn span(&self) -> Span { self.def.span diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index f39b71405d3..654554fe0e8 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -588,10 +588,13 @@ impl ForRange { }; let block_span = block.span; - let new_block = BlockExpression(vec![ - let_elem, - Statement { kind: StatementKind::Expression(block), span: block_span }, - ]); + let new_block = BlockExpression { + is_unsafe: false, + statements: vec![ + let_elem, + Statement { kind: StatementKind::Expression(block), span: block_span }, + ], + }; let new_block = Expression::new(ExpressionKind::Block(new_block), block_span); let for_loop = Statement { kind: StatementKind::For(ForLoopStatement { @@ -603,7 +606,10 @@ impl ForRange { span: for_loop_span, }; - let block = ExpressionKind::Block(BlockExpression(vec![let_array, for_loop])); + let block = ExpressionKind::Block(BlockExpression { + is_unsafe: false, + statements: vec![let_array, for_loop], + }); StatementKind::Expression(Expression::new(block, for_loop_span)) } } diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index a88567fcaf9..983ec1ccc81 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -93,10 +93,10 @@ impl DebugInstrumenter { }) .collect(); - self.walk_scope(&mut func.body.0, func.span); + self.walk_scope(&mut func.body.statements, func.span); // prepend fn params: - func.body.0 = [set_fn_params, func.body.0.clone()].concat(); + func.body.statements = [set_fn_params, func.body.statements.clone()].concat(); } // Modify a vector of statements in-place, adding instrumentation for sets and drops. @@ -212,7 +212,10 @@ impl DebugInstrumenter { pattern: ast::Pattern::Tuple(vars_pattern, let_stmt.pattern.span()), r#type: ast::UnresolvedType::unspecified(), expression: ast::Expression { - kind: ast::ExpressionKind::Block(ast::BlockExpression(block_stmts)), + kind: ast::ExpressionKind::Block(ast::BlockExpression { + is_unsafe: true, + statements: block_stmts, + }), span: let_stmt.expression.span, }, }), @@ -299,11 +302,14 @@ impl DebugInstrumenter { kind: ast::StatementKind::Assign(ast::AssignStatement { lvalue: assign_stmt.lvalue.clone(), expression: ast::Expression { - kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ - ast::Statement { kind: let_kind, span: expression_span }, - new_assign_stmt, - ast::Statement { kind: ret_kind, span: expression_span }, - ])), + kind: ast::ExpressionKind::Block(ast::BlockExpression { + is_unsafe: false, + statements: vec![ + ast::Statement { kind: let_kind, span: expression_span }, + new_assign_stmt, + ast::Statement { kind: ret_kind, span: expression_span }, + ], + }), span: expression_span, }, }), @@ -313,7 +319,7 @@ impl DebugInstrumenter { fn walk_expr(&mut self, expr: &mut ast::Expression) { match &mut expr.kind { - ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { + ast::ExpressionKind::Block(ast::BlockExpression { ref mut statements, .. }) => { self.scope.push(HashMap::default()); self.walk_scope(statements, expr.span); } @@ -384,14 +390,17 @@ impl DebugInstrumenter { self.walk_expr(&mut for_stmt.block); for_stmt.block = ast::Expression { - kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ - set_stmt, - ast::Statement { - kind: ast::StatementKind::Semi(for_stmt.block.clone()), - span: for_stmt.block.span, - }, - drop_stmt, - ])), + kind: ast::ExpressionKind::Block(ast::BlockExpression { + is_unsafe: false, + statements: vec![ + set_stmt, + ast::Statement { + kind: ast::StatementKind::Semi(for_stmt.block.clone()), + span: for_stmt.block.span, + }, + drop_stmt, + ], + }), span: for_stmt.span, }; } @@ -447,7 +456,9 @@ pub fn build_debug_crate_file() -> String { __debug_var_assign_oracle(var_id, value); } pub fn __debug_var_assign(var_id: u32, value: T) { - __debug_var_assign_inner(var_id, value); + unsafe {{ + __debug_var_assign_inner(var_id, value); + }} } #[oracle(__debug_var_drop)] @@ -456,7 +467,9 @@ pub fn build_debug_crate_file() -> String { __debug_var_drop_oracle(var_id); } pub fn __debug_var_drop(var_id: u32) { - __debug_var_drop_inner(var_id); + unsafe {{ + __debug_var_drop_inner(var_id); + }} } #[oracle(__debug_dereference_assign)] @@ -465,7 +478,9 @@ pub fn build_debug_crate_file() -> String { __debug_dereference_assign_oracle(var_id, value); } pub fn __debug_dereference_assign(var_id: u32, value: T) { - __debug_dereference_assign_inner(var_id, value); + unsafe {{ + __debug_dereference_assign_inner(var_id, value); + }} } "# .to_string(), @@ -489,7 +504,9 @@ pub fn build_debug_crate_file() -> String { __debug_oracle_member_assign_{n}(var_id, value, {vars}); }} pub fn __debug_member_assign_{n}(var_id: u32, value: T, {var_sig}) {{ - __debug_inner_member_assign_{n}(var_id, value, {vars}); + unsafe {{ + __debug_inner_member_assign_{n}(var_id, value, {vars}); + }} }} "# diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 7f9e48353a7..ad469f8146d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -244,7 +244,7 @@ impl<'a> Resolver<'a> { typ: typ.clone(), span: name.span(), }), - body: BlockExpression(Vec::new()), + body: BlockExpression { is_unsafe: false, statements: Vec::new() }, span: name.span(), where_clause: where_clause.to_vec(), return_type: return_type.clone(), @@ -1926,9 +1926,9 @@ impl<'a> Resolver<'a> { } fn resolve_block(&mut self, block_expr: BlockExpression) -> HirExpression { - let statements = - self.in_new_scope(|this| vecmap(block_expr.0, |stmt| this.intern_stmt(stmt.kind))); - HirExpression::Block(HirBlockExpression(statements)) + let statements = self + .in_new_scope(|this| vecmap(block_expr.statements, |stmt| this.intern_stmt(stmt.kind))); + HirExpression::Block(HirBlockExpression { is_unsafe: block_expr.is_unsafe, statements }) } pub fn intern_block(&mut self, block: BlockExpression) -> ExprId { diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 96d30100d8b..ff2a4977e72 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -122,6 +122,8 @@ pub enum TypeCheckError { ConstrainedReferenceToUnconstrained { span: Span }, #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] UnconstrainedSliceReturnToConstrained { span: Span }, + #[error("unsafe")] + Unsafe { span: Span }, } impl TypeCheckError { @@ -211,7 +213,7 @@ impl From for Diagnostic { | TypeCheckError::OverflowingAssignment { span, .. } | TypeCheckError::FieldModulo { span } | TypeCheckError::ConstrainedReferenceToUnconstrained { span } - | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } => { + | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } | TypeCheckError::Unsafe { span } => { Diagnostic::simple_error(error.to_string(), String::new(), span) } TypeCheckError::PublicReturnType { typ, span } => Diagnostic::simple_error( diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index b78f07c88f2..ce0246f1e94 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -56,13 +56,14 @@ impl<'interner> TypeChecker<'interner> { /// an equivalent HirExpression::Call in the form `foo(a, b, c)`. This cannot /// be done earlier since we need to know the type of the object `a` to resolve which /// function `foo` to refer to. - pub(crate) fn check_expression(&mut self, expr_id: &ExprId) -> Type { + pub(crate) fn check_expression(&mut self, expr_id: &ExprId, allow_unsafe_call: bool) -> Type { let typ = match self.interner.expression(expr_id) { HirExpression::Ident(ident) => self.check_ident(ident, expr_id), HirExpression::Literal(literal) => { match literal { HirLiteral::Array(HirArrayLiteral::Standard(arr)) => { - let elem_types = vecmap(&arr, |arg| self.check_expression(arg)); + let elem_types = + vecmap(&arr, |arg| self.check_expression(arg, allow_unsafe_call)); let first_elem_type = elem_types .first() @@ -94,7 +95,7 @@ impl<'interner> TypeChecker<'interner> { arr_type } HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) => { - let elem_type = self.check_expression(&repeated_element); + let elem_type = self.check_expression(&repeated_element, allow_unsafe_call); let length = match length { Type::Constant(length) => { Type::constant_variable(length, self.interner) @@ -111,7 +112,8 @@ impl<'interner> TypeChecker<'interner> { } HirLiteral::FmtStr(string, idents) => { let len = Type::Constant(string.len() as u64); - let types = vecmap(&idents, |elem| self.check_expression(elem)); + let types = + vecmap(&idents, |elem| self.check_expression(elem, allow_unsafe_call)); Type::FmtString(Box::new(len), Box::new(Type::Tuple(types))) } HirLiteral::Unit => Type::Unit, @@ -119,8 +121,8 @@ impl<'interner> TypeChecker<'interner> { } HirExpression::Infix(infix_expr) => { // The type of the infix expression must be looked up from a type table - let lhs_type = self.check_expression(&infix_expr.lhs); - let rhs_type = self.check_expression(&infix_expr.rhs); + let lhs_type = self.check_expression(&infix_expr.lhs, allow_unsafe_call); + let rhs_type = self.check_expression(&infix_expr.rhs, allow_unsafe_call); let lhs_span = self.interner.expr_span(&infix_expr.lhs); let rhs_span = self.interner.expr_span(&infix_expr.rhs); @@ -149,7 +151,9 @@ impl<'interner> TypeChecker<'interner> { } } } - HirExpression::Index(index_expr) => self.check_index_expression(expr_id, index_expr), + HirExpression::Index(index_expr) => { + self.check_index_expression(expr_id, index_expr, allow_unsafe_call) + } HirExpression::Call(call_expr) => { // Need to setup these flags here as `self` is borrowed mutably to type check the rest of the call expression // These flags are later used to type check calls to unconstrained functions from constrained functions @@ -161,15 +165,22 @@ impl<'interner> TypeChecker<'interner> { self.check_if_deprecated(&call_expr.func); - let function = self.check_expression(&call_expr.func); + let function = self.check_expression(&call_expr.func, allow_unsafe_call); let args = vecmap(&call_expr.arguments, |arg| { - let typ = self.check_expression(arg); + let typ = self.check_expression(arg, allow_unsafe_call); (typ, *arg, self.interner.expr_span(arg)) }); - // Check that we are not passing a mutable reference from a constrained runtime to an unconstrained runtime if is_current_func_constrained && is_unconstrained_call { + if !allow_unsafe_call { + self.errors.push(TypeCheckError::Unsafe { + span: self.interner.expr_span(expr_id), + }); + return Type::Error; + } + + // Check that we are not passing a mutable reference from a constrained runtime to an unconstrained runtime for (typ, _, _) in args.iter() { if matches!(&typ.follow_bindings(), Type::MutableReference(_)) { self.errors.push(TypeCheckError::ConstrainedReferenceToUnconstrained { @@ -197,7 +208,8 @@ impl<'interner> TypeChecker<'interner> { return_type } HirExpression::MethodCall(mut method_call) => { - let mut object_type = self.check_expression(&method_call.object).follow_bindings(); + let mut object_type = + self.check_expression(&method_call.object, allow_unsafe_call).follow_bindings(); let method_name = method_call.method.0.contents.as_str(); match self.lookup_method(&object_type, method_name, expr_id) { Some(method_ref) => { @@ -232,23 +244,24 @@ impl<'interner> TypeChecker<'interner> { // Type check the new call now that it has been changed from a method call // to a function call. This way we avoid duplicating code. - self.check_expression(expr_id) + self.check_expression(expr_id, allow_unsafe_call) } None => Type::Error, } } HirExpression::Cast(cast_expr) => { // Evaluate the LHS - let lhs_type = self.check_expression(&cast_expr.lhs); + let lhs_type = self.check_expression(&cast_expr.lhs, allow_unsafe_call); let span = self.interner.expr_span(expr_id); self.check_cast(lhs_type, cast_expr.r#type, span) } HirExpression::Block(block_expr) => { let mut block_type = Type::Unit; + let allow_unsafe = allow_unsafe_call || block_expr.is_unsafe; let statements = block_expr.statements(); for (i, stmt) in statements.iter().enumerate() { - let expr_type = self.check_statement(stmt); + let expr_type = self.check_statement(stmt, allow_unsafe); if let crate::hir_def::stmt::HirStatement::Semi(expr) = self.interner.statement(stmt) @@ -272,17 +285,21 @@ impl<'interner> TypeChecker<'interner> { block_type } HirExpression::Prefix(prefix_expr) => { - let rhs_type = self.check_expression(&prefix_expr.rhs); + let rhs_type = self.check_expression(&prefix_expr.rhs, allow_unsafe_call); let span = self.interner.expr_span(&prefix_expr.rhs); self.type_check_prefix_operand(&prefix_expr.operator, &rhs_type, span) } - HirExpression::If(if_expr) => self.check_if_expr(&if_expr, expr_id), - HirExpression::Constructor(constructor) => self.check_constructor(constructor, expr_id), - HirExpression::MemberAccess(access) => self.check_member_access(access, *expr_id), - HirExpression::Error => Type::Error, - HirExpression::Tuple(elements) => { - Type::Tuple(vecmap(&elements, |elem| self.check_expression(elem))) + HirExpression::If(if_expr) => self.check_if_expr(&if_expr, expr_id, allow_unsafe_call), + HirExpression::Constructor(constructor) => { + self.check_constructor(constructor, expr_id, allow_unsafe_call) + } + HirExpression::MemberAccess(access) => { + self.check_member_access(access, *expr_id, allow_unsafe_call) } + HirExpression::Error => Type::Error, + HirExpression::Tuple(elements) => Type::Tuple(vecmap(&elements, |elem| { + self.check_expression(elem, allow_unsafe_call) + })), HirExpression::Lambda(lambda) => { let captured_vars = vecmap(lambda.captures, |capture| { self.interner.definition_type(capture.ident.id) @@ -296,7 +313,7 @@ impl<'interner> TypeChecker<'interner> { typ }); - let actual_return = self.check_expression(&lambda.body); + let actual_return = self.check_expression(&lambda.body, allow_unsafe_call); let span = self.interner.expr_span(&lambda.body); self.unify(&actual_return, &lambda.return_type, || TypeCheckError::TypeMismatch { @@ -525,8 +542,9 @@ impl<'interner> TypeChecker<'interner> { &mut self, id: &ExprId, mut index_expr: expr::HirIndexExpression, + allow_unsafe_call: bool, ) -> Type { - let index_type = self.check_expression(&index_expr.index); + let index_type = self.check_expression(&index_expr.index, allow_unsafe_call); let span = self.interner.expr_span(&index_expr.index); index_type.unify( @@ -541,7 +559,7 @@ impl<'interner> TypeChecker<'interner> { // When writing `a[i]`, if `a : &mut ...` then automatically dereference `a` as many // times as needed to get the underlying array. - let lhs_type = self.check_expression(&index_expr.collection); + let lhs_type = self.check_expression(&index_expr.collection, allow_unsafe_call); let (new_lhs, lhs_type) = self.insert_auto_dereferences(index_expr.collection, lhs_type); index_expr.collection = new_lhs; self.interner.replace_expr(id, HirExpression::Index(index_expr)); @@ -594,9 +612,14 @@ impl<'interner> TypeChecker<'interner> { } } - fn check_if_expr(&mut self, if_expr: &expr::HirIfExpression, expr_id: &ExprId) -> Type { - let cond_type = self.check_expression(&if_expr.condition); - let then_type = self.check_expression(&if_expr.consequence); + fn check_if_expr( + &mut self, + if_expr: &expr::HirIfExpression, + expr_id: &ExprId, + allow_unsafe_call: bool, + ) -> Type { + let cond_type = self.check_expression(&if_expr.condition, allow_unsafe_call); + let then_type = self.check_expression(&if_expr.consequence, allow_unsafe_call); let expr_span = self.interner.expr_span(&if_expr.condition); @@ -609,7 +632,7 @@ impl<'interner> TypeChecker<'interner> { match if_expr.alternative { None => Type::Unit, Some(alternative) => { - let else_type = self.check_expression(&alternative); + let else_type = self.check_expression(&alternative, allow_unsafe_call); let expr_span = self.interner.expr_span(expr_id); self.unify(&then_type, &else_type, || { @@ -639,6 +662,7 @@ impl<'interner> TypeChecker<'interner> { &mut self, constructor: expr::HirConstructorExpression, expr_id: &ExprId, + allow_unsafe_call: bool, ) -> Type { let typ = constructor.r#type; let generics = constructor.struct_generics; @@ -658,7 +682,7 @@ impl<'interner> TypeChecker<'interner> { // mismatch here as long as we continue typechecking the rest of the program to the best // of our ability. if param_name == arg_ident.0.contents { - let arg_type = self.check_expression(&arg); + let arg_type = self.check_expression(&arg, allow_unsafe_call); let span = self.interner.expr_span(expr_id); self.unify_with_coercions(&arg_type, ¶m_type, arg, || { @@ -674,8 +698,13 @@ impl<'interner> TypeChecker<'interner> { Type::Struct(typ, generics) } - fn check_member_access(&mut self, mut access: expr::HirMemberAccess, expr_id: ExprId) -> Type { - let lhs_type = self.check_expression(&access.lhs).follow_bindings(); + fn check_member_access( + &mut self, + mut access: expr::HirMemberAccess, + expr_id: ExprId, + allow_unsafe_call: bool, + ) -> Type { + let lhs_type = self.check_expression(&access.lhs, allow_unsafe_call).follow_bindings(); let span = self.interner.expr_span(&expr_id); let access_lhs = &mut access.lhs; diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 21d1c75a0f2..57e2e47ed8c 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -168,7 +168,7 @@ impl<'interner> TypeChecker<'interner> { } fn check_function_body(&mut self, body: &ExprId) -> Type { - self.check_expression(body) + self.check_expression(body, false) } pub fn check_global( @@ -182,7 +182,7 @@ impl<'interner> TypeChecker<'interner> { current_function: None, }; let statement = this.interner.get_global(id).let_statement; - this.check_statement(&statement); + this.check_statement(&statement, false); this.errors } @@ -297,7 +297,10 @@ mod test { expression: expr_id, }; let stmt_id = interner.push_stmt(HirStatement::Let(let_stmt)); - let expr_id = interner.push_expr(HirExpression::Block(HirBlockExpression(vec![stmt_id]))); + let expr_id = interner.push_expr(HirExpression::Block(HirBlockExpression { + is_unsafe: false, + statements: vec![stmt_id], + })); interner.push_expr_location(expr_id, Span::single_char(0), file); // Create function to enclose the let statement diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 358bea86922..13595bc5bc5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -19,7 +19,7 @@ impl<'interner> TypeChecker<'interner> { /// All statements have a unit type `()` as their type so the type of the statement /// is not interesting. Type checking must still be done on statements to ensure any /// expressions used within them are typed correctly. - pub(crate) fn check_statement(&mut self, stmt_id: &StmtId) -> Type { + pub(crate) fn check_statement(&mut self, stmt_id: &StmtId, allow_unsafe_call: bool) -> Type { match self.interner.statement(stmt_id) { // Lets lay out a convincing argument that the handling of // SemiExpressions and Expressions below is correct. @@ -42,23 +42,27 @@ impl<'interner> TypeChecker<'interner> { // // The reason why we still modify the database, is to make sure it is future-proof HirStatement::Expression(expr_id) => { - return self.check_expression(&expr_id); + return self.check_expression(&expr_id, allow_unsafe_call); } HirStatement::Semi(expr_id) => { - self.check_expression(&expr_id); + self.check_expression(&expr_id, allow_unsafe_call); } - HirStatement::Let(let_stmt) => self.check_let_stmt(let_stmt), - HirStatement::Constrain(constrain_stmt) => self.check_constrain_stmt(constrain_stmt), - HirStatement::Assign(assign_stmt) => self.check_assign_stmt(assign_stmt, stmt_id), - HirStatement::For(for_loop) => self.check_for_loop(for_loop), + HirStatement::Let(let_stmt) => self.check_let_stmt(let_stmt, allow_unsafe_call), + HirStatement::Constrain(constrain_stmt) => { + self.check_constrain_stmt(constrain_stmt, allow_unsafe_call); + } + HirStatement::Assign(assign_stmt) => { + self.check_assign_stmt(assign_stmt, stmt_id, allow_unsafe_call); + } + HirStatement::For(for_loop) => self.check_for_loop(for_loop, allow_unsafe_call), HirStatement::Error => (), } Type::Unit } - fn check_for_loop(&mut self, for_loop: HirForStatement) { - let start_range_type = self.check_expression(&for_loop.start_range); - let end_range_type = self.check_expression(&for_loop.end_range); + fn check_for_loop(&mut self, for_loop: HirForStatement, allow_unsafe_call: bool) { + let start_range_type = self.check_expression(&for_loop.start_range, allow_unsafe_call); + let end_range_type = self.check_expression(&for_loop.end_range, allow_unsafe_call); let start_span = self.interner.expr_span(&for_loop.start_range); let end_span = self.interner.expr_span(&for_loop.end_range); @@ -81,7 +85,7 @@ impl<'interner> TypeChecker<'interner> { self.interner.push_definition_type(for_loop.identifier.id, start_range_type); - self.check_expression(&for_loop.block); + self.check_expression(&for_loop.block, allow_unsafe_call); } /// Associate a given HirPattern with the given Type, and remember @@ -132,10 +136,16 @@ impl<'interner> TypeChecker<'interner> { } } - fn check_assign_stmt(&mut self, assign_stmt: HirAssignStatement, stmt_id: &StmtId) { - let expr_type = self.check_expression(&assign_stmt.expression); + fn check_assign_stmt( + &mut self, + assign_stmt: HirAssignStatement, + stmt_id: &StmtId, + allow_unsafe_call: bool, + ) { + let expr_type = self.check_expression(&assign_stmt.expression, allow_unsafe_call); let span = self.interner.expr_span(&assign_stmt.expression); - let (lvalue_type, new_lvalue, mutable) = self.check_lvalue(&assign_stmt.lvalue, span); + let (lvalue_type, new_lvalue, mutable) = + self.check_lvalue(&assign_stmt.lvalue, span, allow_unsafe_call); if !mutable { let (name, span) = self.get_lvalue_name_and_span(&assign_stmt.lvalue); @@ -177,7 +187,12 @@ impl<'interner> TypeChecker<'interner> { } /// Type check an lvalue - the left hand side of an assignment statement. - fn check_lvalue(&mut self, lvalue: &HirLValue, assign_span: Span) -> (Type, HirLValue, bool) { + fn check_lvalue( + &mut self, + lvalue: &HirLValue, + assign_span: Span, + allow_unsafe_call: bool, + ) -> (Type, HirLValue, bool) { match lvalue { HirLValue::Ident(ident, _) => { let mut mutable = true; @@ -196,7 +211,8 @@ impl<'interner> TypeChecker<'interner> { (typ.clone(), HirLValue::Ident(ident.clone(), typ), mutable) } HirLValue::MemberAccess { object, field_name, .. } => { - let (lhs_type, object, mut mutable) = self.check_lvalue(object, assign_span); + let (lhs_type, object, mut mutable) = + self.check_lvalue(object, assign_span, allow_unsafe_call); let mut object = Box::new(object); let span = field_name.span(); let field_name = field_name.clone(); @@ -228,7 +244,7 @@ impl<'interner> TypeChecker<'interner> { (object_type, lvalue, mutable) } HirLValue::Index { array, index, .. } => { - let index_type = self.check_expression(index); + let index_type = self.check_expression(index, allow_unsafe_call); let expr_span = self.interner.expr_span(index); index_type.unify( @@ -242,7 +258,7 @@ impl<'interner> TypeChecker<'interner> { ); let (mut lvalue_type, mut lvalue, mut mutable) = - self.check_lvalue(array, assign_span); + self.check_lvalue(array, assign_span, allow_unsafe_call); // Before we check that the lvalue is an array, try to dereference it as many times // as needed to unwrap any &mut wrappers. @@ -272,7 +288,8 @@ impl<'interner> TypeChecker<'interner> { (typ.clone(), HirLValue::Index { array, index: *index, typ }, mutable) } HirLValue::Dereference { lvalue, element_type: _ } => { - let (reference_type, lvalue, _) = self.check_lvalue(lvalue, assign_span); + let (reference_type, lvalue, _) = + self.check_lvalue(lvalue, assign_span, allow_unsafe_call); let lvalue = Box::new(lvalue); let element_type = Type::type_variable(self.interner.next_type_variable_id()); @@ -290,19 +307,20 @@ impl<'interner> TypeChecker<'interner> { } } - fn check_let_stmt(&mut self, let_stmt: HirLetStatement) { - let resolved_type = self.check_declaration(let_stmt.expression, let_stmt.r#type); + fn check_let_stmt(&mut self, let_stmt: HirLetStatement, allow_unsafe_call: bool) { + let resolved_type = + self.check_declaration(let_stmt.expression, let_stmt.r#type, allow_unsafe_call); // Set the type of the pattern to be equal to the annotated type self.bind_pattern(&let_stmt.pattern, resolved_type); } - fn check_constrain_stmt(&mut self, stmt: HirConstrainStatement) { - let expr_type = self.check_expression(&stmt.0); + fn check_constrain_stmt(&mut self, stmt: HirConstrainStatement, allow_unsafe_call: bool) { + let expr_type = self.check_expression(&stmt.0, allow_unsafe_call); let expr_span = self.interner.expr_span(&stmt.0); // Must type check the assertion message expression so that we instantiate bindings - stmt.2.map(|assert_msg_expr| self.check_expression(&assert_msg_expr)); + stmt.2.map(|assert_msg_expr| self.check_expression(&assert_msg_expr, true)); self.unify(&expr_type, &Type::Bool, || TypeCheckError::TypeMismatch { expr_typ: expr_type.to_string(), @@ -314,9 +332,14 @@ impl<'interner> TypeChecker<'interner> { /// All declaration statements check that the user specified type(UST) is equal to the /// expression on the RHS, unless the UST is unspecified in which case /// the type of the declaration is inferred to match the RHS. - fn check_declaration(&mut self, rhs_expr: ExprId, annotated_type: Type) -> Type { + fn check_declaration( + &mut self, + rhs_expr: ExprId, + annotated_type: Type, + allow_unsafe_call: bool, + ) -> Type { // Type check the expression on the RHS - let expr_type = self.check_expression(&rhs_expr); + let expr_type = self.check_expression(&rhs_expr, allow_unsafe_call); // First check if the LHS is unspecified // If so, then we give it the same type as the expression diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index b4c590de491..f094e1058dd 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -36,7 +36,7 @@ pub enum HirExpression { impl HirExpression { /// Returns an empty block expression pub const fn empty_block() -> HirExpression { - HirExpression::Block(HirBlockExpression(vec![])) + HirExpression::Block(HirBlockExpression { is_unsafe: false, statements: vec![] }) } } @@ -247,11 +247,14 @@ pub struct HirIndexExpression { } #[derive(Debug, Clone)] -pub struct HirBlockExpression(pub Vec); +pub struct HirBlockExpression { + pub is_unsafe: bool, + pub statements: Vec, +} impl HirBlockExpression { pub fn statements(&self) -> &[StmtId] { - &self.0 + &self.statements } } diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 5674ae5a39a..1594b521e49 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -675,6 +675,7 @@ pub enum Keyword { Type, Unchecked, Unconstrained, + Unsafe, Use, Where, While, @@ -718,6 +719,7 @@ impl fmt::Display for Keyword { Keyword::Type => write!(f, "type"), Keyword::Unchecked => write!(f, "unchecked"), Keyword::Unconstrained => write!(f, "unconstrained"), + Keyword::Unsafe => write!(f, "unsafe"), Keyword::Use => write!(f, "use"), Keyword::Where => write!(f, "where"), Keyword::While => write!(f, "while"), @@ -764,6 +766,8 @@ impl Keyword { "type" => Keyword::Type, "unchecked" => Keyword::Unchecked, "unconstrained" => Keyword::Unconstrained, + "unsafe" => Keyword::Unsafe, + "use" => Keyword::Use, "where" => Keyword::Where, "while" => Keyword::While, diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ce880401d77..adba8187aa0 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -423,7 +423,7 @@ impl<'interner> Monomorphizer<'interner> { } }, HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]), - HirExpression::Block(block) => self.block(block.0)?, + HirExpression::Block(block) => self.block(block.statements)?, HirExpression::Prefix(prefix) => { let location = self.interner.expr_location(&expr); diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 75f4a6359bf..38146a67e7c 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -333,21 +333,28 @@ fn block_expr<'a>( fn block<'a>( statement: impl NoirParser + 'a, ) -> impl NoirParser + 'a { + use crate::token::Keyword; use Token::*; - statement - .recover_via(statement_recovery()) - .then(just(Semicolon).or_not().map_with_span(|s, span| (s, span))) - .map_with_span(|(kind, rest), span| (Statement { kind, span }, rest)) - .repeated() - .validate(check_statements_require_semicolon) - .delimited_by(just(LeftBrace), just(RightBrace)) - .recover_with(nested_delimiters( - LeftBrace, - RightBrace, - [(LeftParen, RightParen), (LeftBracket, RightBracket)], - |span| vec![Statement { kind: StatementKind::Error, span }], - )) - .map(BlockExpression) + + keyword(Keyword::Unsafe) + .or_not() + .map(|unsafe_token| unsafe_token.is_some()) + .then( + statement + .recover_via(statement_recovery()) + .then(just(Semicolon).or_not().map_with_span(|s, span| (s, span))) + .map_with_span(|(kind, rest), span| (Statement { kind, span }, rest)) + .repeated() + .validate(check_statements_require_semicolon) + .delimited_by(just(LeftBrace), just(RightBrace)) + .recover_with(nested_delimiters( + LeftBrace, + RightBrace, + [(LeftParen, RightParen), (LeftBracket, RightBracket)], + |span| vec![Statement { kind: StatementKind::Error, span }], + )), + ) + .map(|(is_unsafe, statements)| BlockExpression { is_unsafe, statements }) } fn check_statements_require_semicolon( @@ -993,10 +1000,13 @@ where // Wrap the inner `if` expression in a block expression. // i.e. rewrite the sugared form `if cond1 {} else if cond2 {}` as `if cond1 {} else { if cond2 {} }`. let if_expression = Expression::new(kind, span); - let desugared_else = BlockExpression(vec![Statement { - kind: StatementKind::Expression(if_expression), - span, - }]); + let desugared_else = BlockExpression { + is_unsafe: false, + statements: vec![Statement { + kind: StatementKind::Expression(if_expression), + span, + }], + }; Expression::new(ExpressionKind::Block(desugared_else), span) })); @@ -1288,13 +1298,13 @@ mod test { // Regression for #1310: this should be parsed as a block and not a function call let res = parse_with(block(fresh_statement()), "{ if true { 1 } else { 2 } (3, 4) }").unwrap(); - match unwrap_expr(&res.0.last().unwrap().kind) { + match unwrap_expr(&res.statements.last().unwrap().kind) { // The `if` followed by a tuple is currently creates a block around both in case // there was none to start with, so there is an extra block here. ExpressionKind::Block(block) => { - assert_eq!(block.0.len(), 2); - assert!(matches!(unwrap_expr(&block.0[0].kind), ExpressionKind::If(_))); - assert!(matches!(unwrap_expr(&block.0[1].kind), ExpressionKind::Tuple(_))); + assert_eq!(block.statements.len(), 2); + assert!(matches!(unwrap_expr(&block.statements[0].kind), ExpressionKind::If(_))); + assert!(matches!(unwrap_expr(&block.statements[1].kind), ExpressionKind::Tuple(_))); } _ => unreachable!(), } diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index 3da4b649174..8ddc43bbda6 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -11,14 +11,21 @@ impl [T; N] { } pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { - let sorted_index = self.get_sorting_index(ordering); - let mut result = self; - // Ensure the indexes are correct - for i in 0..N { - let pos = find_index(sorted_index, i); - assert(sorted_index[pos] == i); - } + let sorted_index = unsafe { + // Safety: This is safe we we enforce proper ordering using these indices + let sorted_index: [u64; N] = self.get_sorting_index(ordering); + + // Ensure the indexes are correct + for i in 0..N { + let pos = find_index(sorted_index, i); + assert(sorted_index[pos] == i); + } + + sorted_index + }; + // Sort the array using the indexes + let mut result = self; for i in 0..N { result[i] = self[sorted_index[i]]; } diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index 056299b4238..5f78b3047eb 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -171,7 +171,7 @@ impl HashMap { for slot in self._table { if slot.is_valid() { - let (_, value) = slot.key_value_unchecked(); + let (_, value) = unsafe {slot.key_value_unchecked()}; values[valid_amount] = Option::some(value); valid_amount += 1; } diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index 9e1445fd3ba..0931ef3ca25 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -24,15 +24,21 @@ unconstrained fn decompose_unsafe(x: Field) -> (Field, Field) { /// Decompose a single field into two 16 byte fields. pub fn decompose(x: Field) -> (Field, Field) { // Take hints of the decomposition - let (xlo, xhi) = decompose_unsafe(x); - let borrow = lt_unsafe(PLO, xlo, 16); + let (xlo, xhi) = unsafe { + let (xlo, xhi) = decompose_unsafe(x); + + // Range check the limbs + xlo.assert_max_bit_size(128); + xhi.assert_max_bit_size(128); - // Range check the limbs - xlo.assert_max_bit_size(128); - xhi.assert_max_bit_size(128); + // Check that the decomposition is correct + assert_eq(x, xlo + TWO_POW_128 * xhi); - // Check that the decomposition is correct - assert_eq(x, xlo + TWO_POW_128 * xhi); + (xlo, xhi) + }; + + + let borrow = unsafe {lt_unsafe(PLO, xlo, 16)}; // Check that (xlo < plo && xhi <= phi) || (xlo >= plo && xhi < phi) let rlo = PLO - xlo + (borrow as Field) * TWO_POW_128; @@ -72,7 +78,7 @@ pub fn assert_gt(a: Field, b: Field) { let (alo, ahi) = decompose(a); let (blo, bhi) = decompose(b); - let borrow = lte_unsafe(alo, blo, 16); + let borrow = unsafe { lte_unsafe(alo, blo, 16) }; // Assert that (alo > blo && ahi >= bhi) || (alo <= blo && ahi > bhi) let rlo = alo - blo - 1 + (borrow as Field) * TWO_POW_128; @@ -89,13 +95,18 @@ pub fn assert_lt(a: Field, b: Field) { pub fn gt(a: Field, b: Field) -> bool { if a == b { false - } else if lt_unsafe(a, b, 32) { - assert_gt(b, a); - false } else { - assert_gt(a, b); - true + let a_lt_b = unsafe {lt_unsafe(a, b, 32) }; + if a_lt_b { + assert_gt(b, a); + false + } else { + assert_gt(a, b); + true + } } + + } pub fn lt(a: Field, b: Field) -> bool { diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index ebde4b88858..6586d10ca66 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -13,7 +13,7 @@ mod sha256; mod sha512; mod field; mod ec; -mod unsafe; +mod unsafe_func; mod collections; mod compat; mod convert; @@ -32,12 +32,20 @@ mod bigint; #[oracle(print)] unconstrained fn print_oracle(with_newline: bool, input: T) {} -unconstrained pub fn print(input: T) { - print_oracle(false, input); +unconstrained fn print_unconstrained(with_newline: bool, input: T) { + print_oracle(with_newline, input); } -unconstrained pub fn println(input: T) { - print_oracle(true, input); +pub fn println(input: T) { + unsafe { + print_unconstrained(true, input); + } +} + +pub fn print(input: T) { + unsafe { + print_unconstrained(false, input); + } } #[foreign(recursive_aggregation)] diff --git a/noir_stdlib/src/option.nr b/noir_stdlib/src/option.nr index 1c32f758af7..8029e4f68d8 100644 --- a/noir_stdlib/src/option.nr +++ b/noir_stdlib/src/option.nr @@ -6,7 +6,7 @@ struct Option { impl Option { /// Constructs a None value pub fn none() -> Self { - Self { _is_some: false, _value: crate::unsafe::zeroed() } + Self { _is_some: false, _value: crate::unsafe_func::zeroed() } } /// Constructs a Some wrapper around the given value diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index b91ed5c4cb2..c65f83dcb44 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -191,21 +191,26 @@ impl Mul for U128 { impl Div for U128 { fn div(self: Self, b: U128) -> U128 { - let (q,r) = self.unconstrained_div(b); - let a = b * q + r; - assert_eq(self, a); - assert(r < b); - q + unsafe { + let (q,r) = self.unconstrained_div(b); + let a = b * q + r; + assert_eq(self, a); + assert(r < b); + q + } } } impl Rem for U128 { fn rem(self: Self, b: U128) -> U128 { - let (q,r) = self.unconstrained_div(b); - let a = b * q + r; - assert_eq(self, a); - assert(r < b); - r + unsafe { + let (q,r) = self.unconstrained_div(b); + let a = b * q + r; + assert_eq(self, a); + assert(r < b); + + r + } } } diff --git a/noir_stdlib/src/unsafe.nr b/noir_stdlib/src/unsafe_func.nr similarity index 100% rename from noir_stdlib/src/unsafe.nr rename to noir_stdlib/src/unsafe_func.nr diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 2cd0e881e84..f9836adda18 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -119,11 +119,11 @@ impl FmtVisitor<'_> { self.last_position = block_span.start() + 1; // `{` self.push_str("{"); - self.trim_spaces_after_opening_brace(&block.0); + self.trim_spaces_after_opening_brace(&block.statements); self.indent.block_indent(self.config); - self.visit_stmts(block.0); + self.visit_stmts(block.statements); let span = (self.last_position..block_span.end() - 1).into(); self.close_block(span); From 21b04cf7a44eb2ecea963cd93b4e317165184772 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 26 Feb 2024 20:03:15 +0000 Subject: [PATCH 02/67] chore: fix tests --- .../brillig_cast/src/main.nr | 12 ++++---- .../src/main.nr | 8 +++--- .../src/main.nr | 28 +++++++++---------- .../brillig_modulo/src/main.nr | 28 ++++++++++--------- 4 files changed, 40 insertions(+), 36 deletions(-) diff --git a/test_programs/compile_success_empty/brillig_cast/src/main.nr b/test_programs/compile_success_empty/brillig_cast/src/main.nr index ecb832468ba..f2bede99d4c 100644 --- a/test_programs/compile_success_empty/brillig_cast/src/main.nr +++ b/test_programs/compile_success_empty/brillig_cast/src/main.nr @@ -2,11 +2,13 @@ // // The features being tested are cast operations on brillig fn main() { - bool_casts(); - field_casts(); - uint_casts(); - int_casts(); - mixed_casts(); + unsafe { + bool_casts(); + field_casts(); + uint_casts(); + int_casts(); + mixed_casts(); + } } unconstrained fn bool_casts() { diff --git a/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr b/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr index 54f06858846..0c22297b8b4 100644 --- a/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr +++ b/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr @@ -2,10 +2,10 @@ fn main() { let x = 4; let y = 2; - assert((x + y) == add(x, y)); - assert((x - y) == sub(x, y)); - assert((x * y) == mul(x, y)); - assert((x / y) == div(x, y)); + assert((x + y) == unsafe { add(x, y) }); + assert((x - y) == unsafe { sub(x, y) }); + assert((x * y) == unsafe { mul(x, y) }); + assert((x / y) == unsafe { div(x, y) }); } unconstrained fn add(x: Field, y: Field) -> Field { diff --git a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr index a873bcd5dbd..b1837a91c12 100644 --- a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr +++ b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr @@ -3,28 +3,28 @@ fn main() { let x: u32 = 6; let y: u32 = 2; - assert((x + y) == add(x, y)); + assert((x + y) == unsafe { add(x, y) }); - assert((x - y) == sub(x, y)); + assert((x - y) == unsafe { sub(x, y) }); - assert((x * y) == mul(x, y)); + assert((x * y) == unsafe { mul(x, y) }); - assert((x / y) == div(x, y)); + assert((x / y) == unsafe { div(x, y) }); // TODO SSA => ACIR has some issues with i32 ops - assert(check_signed_div(6, 2, 3)); + assert(unsafe { check_signed_div(6, 2, 3)}); - assert(eq(1, 2) == false); - assert(eq(1, 1)); + assert(unsafe { eq(1, 2) } == false); + assert(unsafe { eq(1, 1) }); - assert(lt(x, y) == false); - assert(lt(y, x)); + assert(unsafe { lt(x, y) } == false); + assert(unsafe { lt(y, x) }); - assert((x & y) == and(x, y)); - assert((x | y) == or(x, y)); + assert((x & y) == unsafe { and(x, y) }); + assert((x | y) == unsafe { or(x, y) }); // TODO SSA => ACIR has some issues with xor ops - assert(check_xor(x, y, 4)); - assert((x >> y) == shr(x, y)); - assert((x << y) == shl(x, y)); + assert(unsafe { check_xor(x, y, 4) }); + assert((x >> y) == unsafe { shr(x, y) }); + assert((x << y) == unsafe { shl(x, y) }); } unconstrained fn add(x: u32, y: u32) -> u32 { diff --git a/test_programs/compile_success_empty/brillig_modulo/src/main.nr b/test_programs/compile_success_empty/brillig_modulo/src/main.nr index 195ed31fb08..dff5eadcb55 100644 --- a/test_programs/compile_success_empty/brillig_modulo/src/main.nr +++ b/test_programs/compile_success_empty/brillig_modulo/src/main.nr @@ -2,20 +2,22 @@ // // The features being tested is modulo operations on brillig fn main() { - assert(modulo(47, 3) == 2); - assert(modulo(2, 3) == 2); - assert(signed_modulo(5, 3) == 2); - assert(signed_modulo(2, 3) == 2); + unsafe { + assert(modulo(47, 3) == 2); + assert(modulo(2, 3) == 2); + assert(signed_modulo(5, 3) == 2); + assert(signed_modulo(2, 3) == 2); - 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 - assert(signed_modulo(minus_five, 3) == minus_two); - // (-5 / -3) * -3 - 2 = 1 * -3 - 2 = -3 - 2 = -5 - assert(signed_modulo(minus_five, minus_three) == minus_two); + 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 + assert(signed_modulo(minus_five, 3) == minus_two); + // (-5 / -3) * -3 - 2 = 1 * -3 - 2 = -3 - 2 = -5 + assert(signed_modulo(minus_five, minus_three) == minus_two); + } } unconstrained fn modulo(x: u32, y: u32) -> u32 { From 953b1e81d3a4b498227a372b6414895b2af81bd4 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 26 Feb 2024 20:18:06 +0000 Subject: [PATCH 03/67] chore: fix some tests --- .../src/main.nr | 14 ++++---- .../src/main.nr | 34 ++++++++++--------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr b/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr index 0c22297b8b4..fedfe48cb0d 100644 --- a/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr +++ b/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr @@ -1,11 +1,13 @@ // Tests arithmetic operations on fields fn main() { - let x = 4; - let y = 2; - assert((x + y) == unsafe { add(x, y) }); - assert((x - y) == unsafe { sub(x, y) }); - assert((x * y) == unsafe { mul(x, y) }); - assert((x / y) == unsafe { div(x, y) }); + unsafe { + let x = 4; + let y = 2; + assert((x + y) == add(x, y)); + assert((x - y) == sub(x, y)); + assert((x * y) == mul(x, y)); + assert((x / y) == div(x, y)); + } } unconstrained fn add(x: Field, y: Field) -> Field { diff --git a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr index b1837a91c12..ee8eb9b3518 100644 --- a/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr +++ b/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr @@ -3,28 +3,30 @@ fn main() { let x: u32 = 6; let y: u32 = 2; - assert((x + y) == unsafe { add(x, y) }); + unsafe { + assert((x + y) == add(x, y)); - assert((x - y) == unsafe { sub(x, y) }); + assert((x - y) == sub(x, y)); - assert((x * y) == unsafe { mul(x, y) }); + assert((x * y) == mul(x, y)); - assert((x / y) == unsafe { div(x, y) }); - // TODO SSA => ACIR has some issues with i32 ops - assert(unsafe { check_signed_div(6, 2, 3)}); + assert((x / y) == div(x, y)); + // TODO SSA => ACIR has some issues with i32 ops + assert(check_signed_div(6, 2, 3)); - assert(unsafe { eq(1, 2) } == false); - assert(unsafe { eq(1, 1) }); + assert(eq(1, 2) == false); + assert(eq(1, 1)); - assert(unsafe { lt(x, y) } == false); - assert(unsafe { lt(y, x) }); + assert(lt(x, y) == false); + assert(lt(y, x)); - assert((x & y) == unsafe { and(x, y) }); - assert((x | y) == unsafe { or(x, y) }); - // TODO SSA => ACIR has some issues with xor ops - assert(unsafe { check_xor(x, y, 4) }); - assert((x >> y) == unsafe { shr(x, y) }); - assert((x << y) == unsafe { shl(x, y) }); + assert((x & y) == and(x, y)); + assert((x | y) == or(x, y)); + // TODO SSA => ACIR has some issues with xor ops + assert(check_xor(x, y, 4)); + assert((x >> y) == shr(x, y)); + assert((x << y) == shl(x, y)); + } } unconstrained fn add(x: u32, y: u32) -> u32 { From ed0b681779baf76790615545941b92f2c8531d1e Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 27 Feb 2024 22:46:03 +0000 Subject: [PATCH 04/67] chore: add unsafe blocks to tests --- .../field_comparisons/src/main.nr | 6 +++-- .../brillig_acir_as_brillig/src/main.nr | 8 +++--- .../brillig_arrays/src/main.nr | 6 +++-- .../brillig_blake2s/src/main.nr | 4 ++- .../brillig_calls_array/src/main.nr | 6 +++-- .../brillig_calls_conditionals/src/main.nr | 10 ++++--- .../brillig_conditional/src/main.nr | 4 ++- .../brillig_ecdsa_secp256k1/src/main.nr | 4 ++- .../brillig_ecdsa_secp256r1/src/main.nr | 4 ++- .../brillig_identity_function/src/main.nr | 22 +++++++++------- .../brillig_keccak/src/main.nr | 26 ++++++++++--------- 11 files changed, 61 insertions(+), 39 deletions(-) diff --git a/test_programs/compile_success_empty/field_comparisons/src/main.nr b/test_programs/compile_success_empty/field_comparisons/src/main.nr index 48cca6c89fc..f43244189c6 100644 --- a/test_programs/compile_success_empty/field_comparisons/src/main.nr +++ b/test_programs/compile_success_empty/field_comparisons/src/main.nr @@ -81,6 +81,8 @@ unconstrained fn checks_in_brillig() { } fn main() { - checks(); - checks_in_brillig(); + unsafe { + checks(); + checks_in_brillig(); + } } diff --git a/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr b/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr index 5bd6ce0adb2..1a595ecfb38 100644 --- a/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr +++ b/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr @@ -1,7 +1,9 @@ fn main(x: u32) { - assert(entry_point(x) == 2); - swap_entry_point(x, x + 1); - assert(deep_entry_point(x) == 4); + unsafe { + assert(entry_point(x) == 2); + swap_entry_point(x, x + 1); + assert(deep_entry_point(x) == 4); + } } fn inner(x: u32) -> u32 { diff --git a/test_programs/execution_success/brillig_arrays/src/main.nr b/test_programs/execution_success/brillig_arrays/src/main.nr index e535b6001a4..c7f0757f31e 100644 --- a/test_programs/execution_success/brillig_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_arrays/src/main.nr @@ -2,8 +2,10 @@ // // The features being tested are array reads and writes fn main(x: [Field; 3]) { - read_array(x); - read_write_array(x); + unsafe { + read_array(x); + read_write_array(x); + } } unconstrained fn read_array(x: [Field; 3]) { diff --git a/test_programs/execution_success/brillig_blake2s/src/main.nr b/test_programs/execution_success/brillig_blake2s/src/main.nr index 5bd52666ae9..8b70fbfe0c3 100644 --- a/test_programs/execution_success/brillig_blake2s/src/main.nr +++ b/test_programs/execution_success/brillig_blake2s/src/main.nr @@ -3,7 +3,9 @@ use dep::std; // // The features being tested is blake2s in brillig fn main(x: [u8; 5], result: [u8; 32]) { - assert(blake2s(x) == result); + unsafe { + assert(blake2s(x) == result); + } } unconstrained fn blake2s(x: [u8; 5]) -> [u8; 32] { diff --git a/test_programs/execution_success/brillig_calls_array/src/main.nr b/test_programs/execution_success/brillig_calls_array/src/main.nr index 1b1d89f6366..8b27a9bb202 100644 --- a/test_programs/execution_success/brillig_calls_array/src/main.nr +++ b/test_programs/execution_success/brillig_calls_array/src/main.nr @@ -2,8 +2,10 @@ // // The features being tested is brillig calls passing arrays around fn main(x: [u32; 3]) { - assert(entry_point(x) == 9); - another_entry_point(x); + unsafe { + assert(entry_point(x) == 9); + another_entry_point(x); + } } unconstrained fn inner(x: [u32; 3]) -> [u32; 3] { diff --git a/test_programs/execution_success/brillig_calls_conditionals/src/main.nr b/test_programs/execution_success/brillig_calls_conditionals/src/main.nr index 0a1718d0171..318da4caf72 100644 --- a/test_programs/execution_success/brillig_calls_conditionals/src/main.nr +++ b/test_programs/execution_success/brillig_calls_conditionals/src/main.nr @@ -2,10 +2,12 @@ // // The features being tested is brillig calls with conditionals fn main(x: [u32; 3]) { - assert(entry_point(x[0]) == 7); - assert(entry_point(x[1]) == 8); - assert(entry_point(x[2]) == 9); - assert(entry_point(42) == 0); + unsafe { + assert(entry_point(x[0]) == 7); + assert(entry_point(x[1]) == 8); + assert(entry_point(x[2]) == 9); + assert(entry_point(42) == 0); + } } unconstrained fn inner_1() -> u32 { diff --git a/test_programs/execution_success/brillig_conditional/src/main.nr b/test_programs/execution_success/brillig_conditional/src/main.nr index a59336a877b..8ababf82319 100644 --- a/test_programs/execution_success/brillig_conditional/src/main.nr +++ b/test_programs/execution_success/brillig_conditional/src/main.nr @@ -2,7 +2,9 @@ // // The features being tested is basic conditonal on brillig fn main(x: Field) { - assert(4 == conditional(x == 1)); + unsafe { + assert(4 == conditional(x == 1)); + } } unconstrained fn conditional(x: bool) -> Field { diff --git a/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr b/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr index 5d84d885567..4d94bffffd0 100644 --- a/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr +++ b/test_programs/execution_success/brillig_ecdsa_secp256k1/src/main.nr @@ -3,7 +3,9 @@ use dep::std; // // The features being tested is ecdsa in brillig fn main(hashed_message: [u8; 32], pub_key_x: [u8; 32], pub_key_y: [u8; 32], signature: [u8; 64]) { - assert(ecdsa(hashed_message, pub_key_x, pub_key_y, signature)); + unsafe { + assert(ecdsa(hashed_message, pub_key_x, pub_key_y, signature)); + } } unconstrained fn ecdsa( diff --git a/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr b/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr index 9da07f531aa..68b1ea41421 100644 --- a/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr +++ b/test_programs/execution_success/brillig_ecdsa_secp256r1/src/main.nr @@ -3,7 +3,9 @@ use dep::std; // // The features being tested is ecdsa in brillig fn main(hashed_message: [u8; 32], pub_key_x: [u8; 32], pub_key_y: [u8; 32], signature: [u8; 64]) { - assert(ecdsa(hashed_message, pub_key_x, pub_key_y, signature)); + unsafe { + assert(ecdsa(hashed_message, pub_key_x, pub_key_y, signature)); + } } unconstrained fn ecdsa( diff --git a/test_programs/execution_success/brillig_identity_function/src/main.nr b/test_programs/execution_success/brillig_identity_function/src/main.nr index f41188b1f0d..c2759fe054f 100644 --- a/test_programs/execution_success/brillig_identity_function/src/main.nr +++ b/test_programs/execution_success/brillig_identity_function/src/main.nr @@ -6,17 +6,19 @@ struct myStruct { // // The features being tested is the identity function in Brillig fn main(x: Field) { - assert(x == identity(x)); - // TODO: add support for array comparison - let arr = identity_array([x, x]); - assert(x == arr[0]); - assert(x == arr[1]); + unsafe { + assert(x == identity(x)); + // TODO: add support for array comparison + let arr = identity_array([x, x]); + assert(x == arr[0]); + assert(x == arr[1]); - let s = myStruct { foo: x, foo_arr: [x, x] }; - let identity_struct = identity_struct(s); - assert(x == identity_struct.foo); - assert(x == identity_struct.foo_arr[0]); - assert(x == identity_struct.foo_arr[1]); + let s = myStruct { foo: x, foo_arr: [x, x] }; + let identity_struct = identity_struct(s); + assert(x == identity_struct.foo); + assert(x == identity_struct.foo_arr[0]); + assert(x == identity_struct.foo_arr[1]); + } } unconstrained fn identity(x: Field) -> Field { diff --git a/test_programs/execution_success/brillig_keccak/src/main.nr b/test_programs/execution_success/brillig_keccak/src/main.nr index 1e9b65a6eb4..26072e2e432 100644 --- a/test_programs/execution_success/brillig_keccak/src/main.nr +++ b/test_programs/execution_success/brillig_keccak/src/main.nr @@ -3,21 +3,23 @@ use dep::std; // // The features being tested is keccak256 in brillig fn main(x: Field, result: [u8; 32]) { - // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field - // The padding is taken care of by the program - let digest = keccak256([x as u8], 1); - assert(digest == result); - //#1399: variable meesage size - let message_size = 4; - let hash_a = keccak256([1, 2, 3, 4], message_size); - let hash_b = keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size); + unsafe { + // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field + // The padding is taken care of by the program + let digest = keccak256([x as u8], 1); + assert(digest == result); + //#1399: variable meesage size + let message_size = 4; + let hash_a = keccak256([1, 2, 3, 4], message_size); + let hash_b = keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size); - assert(hash_a == hash_b); + assert(hash_a == hash_b); - let message_size_big = 8; - let hash_c = keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size_big); + let message_size_big = 8; + let hash_c = keccak256([1, 2, 3, 4, 0, 0, 0, 0], message_size_big); - assert(hash_a != hash_c); + assert(hash_a != hash_c); + } } unconstrained fn keccak256(data: [u8; N], msg_len: u32) -> [u8; 32] { From 68364fddc24fdad94ffeaac8e1ed8313209ed29e Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 27 Feb 2024 22:55:19 +0000 Subject: [PATCH 05/67] chore: fix tests --- .../field_comparisons/src/main.nr | 38 +++++++++++-------- .../brillig_assert/src/main.nr | 4 +- .../brillig_calls/src/main.nr | 10 +++-- .../brillig_fns_as_values/src/main.nr | 16 ++++---- .../brillig_hash_to_field/src/main.nr | 4 +- .../brillig_loop/src/main.nr | 6 ++- .../brillig_nested_arrays/src/main.nr | 20 +++++----- .../execution_success/brillig_not/src/main.nr | 6 ++- .../brillig_oracle/src/main.nr | 30 ++++++++------- .../brillig_recursion/src/main.nr | 4 +- .../brillig_sha256/src/main.nr | 4 +- .../brillig_unitialised_arrays/src/main.nr | 6 ++- .../execution_success/databus/src/main.nr | 4 +- .../global_consts/src/main.nr | 2 +- .../execution_success/hashmap/src/utils.nr | 2 +- .../nested_arrays_from_brillig/src/main.nr | 4 +- .../regression_4124/src/main.nr | 2 +- .../out_of_bounds_alignment/src/main.nr | 4 +- 18 files changed, 100 insertions(+), 66 deletions(-) diff --git a/test_programs/compile_success_empty/field_comparisons/src/main.nr b/test_programs/compile_success_empty/field_comparisons/src/main.nr index f43244189c6..9cc01c8ab2f 100644 --- a/test_programs/compile_success_empty/field_comparisons/src/main.nr +++ b/test_programs/compile_success_empty/field_comparisons/src/main.nr @@ -17,9 +17,11 @@ fn check_plo_phi() { } fn check_decompose_unsafe() { - assert_eq(decompose_unsafe(TWO_POW_128), (0, 1)); - assert_eq(decompose_unsafe(TWO_POW_128 + 0x1234567890), (0x1234567890, 1)); - assert_eq(decompose_unsafe(0x1234567890), (0x1234567890, 0)); + unsafe { + assert_eq(decompose_unsafe(TWO_POW_128), (0, 1)); + assert_eq(decompose_unsafe(TWO_POW_128 + 0x1234567890), (0x1234567890, 1)); + assert_eq(decompose_unsafe(0x1234567890), (0x1234567890, 0)); + } } fn check_decompose() { @@ -29,22 +31,26 @@ fn check_decompose() { } fn check_lt_unsafe() { - assert(lt_unsafe(0, 1, 16)); - assert(lt_unsafe(0, 0x100, 16)); - assert(lt_unsafe(0x100, TWO_POW_128 - 1, 16)); - assert(!lt_unsafe(0, TWO_POW_128, 16)); + unsafe { + assert(lt_unsafe(0, 1, 16)); + assert(lt_unsafe(0, 0x100, 16)); + assert(lt_unsafe(0x100, TWO_POW_128 - 1, 16)); + assert(!lt_unsafe(0, TWO_POW_128, 16)); + } } fn check_lte_unsafe() { - assert(lte_unsafe(0, 1, 16)); - assert(lte_unsafe(0, 0x100, 16)); - assert(lte_unsafe(0x100, TWO_POW_128 - 1, 16)); - assert(!lte_unsafe(0, TWO_POW_128, 16)); + unsafe { + assert(lte_unsafe(0, 1, 16)); + assert(lte_unsafe(0, 0x100, 16)); + assert(lte_unsafe(0x100, TWO_POW_128 - 1, 16)); + assert(!lte_unsafe(0, TWO_POW_128, 16)); - assert(lte_unsafe(0, 0, 16)); - assert(lte_unsafe(0x100, 0x100, 16)); - assert(lte_unsafe(TWO_POW_128 - 1, TWO_POW_128 - 1, 16)); - assert(lte_unsafe(TWO_POW_128, TWO_POW_128, 16)); + assert(lte_unsafe(0, 0, 16)); + assert(lte_unsafe(0x100, 0x100, 16)); + assert(lte_unsafe(TWO_POW_128 - 1, TWO_POW_128 - 1, 16)); + assert(lte_unsafe(TWO_POW_128, TWO_POW_128, 16)); + } } fn check_assert_gt() { @@ -81,8 +87,8 @@ unconstrained fn checks_in_brillig() { } fn main() { + checks(); unsafe { - checks(); checks_in_brillig(); } } diff --git a/test_programs/execution_success/brillig_assert/src/main.nr b/test_programs/execution_success/brillig_assert/src/main.nr index 16fe7b29061..c6c39b61bc9 100644 --- a/test_programs/execution_success/brillig_assert/src/main.nr +++ b/test_programs/execution_success/brillig_assert/src/main.nr @@ -2,7 +2,9 @@ // // The features being tested is using assert on brillig fn main(x: Field) { - assert(1 == conditional(x as bool)); + unsafe { + assert(1 == conditional(x as bool)); + } } unconstrained fn conditional(x: bool) -> Field { diff --git a/test_programs/execution_success/brillig_calls/src/main.nr b/test_programs/execution_success/brillig_calls/src/main.nr index 5c39713f5bb..3e23da53b18 100644 --- a/test_programs/execution_success/brillig_calls/src/main.nr +++ b/test_programs/execution_success/brillig_calls/src/main.nr @@ -2,10 +2,12 @@ // // The features being tested is brillig calls fn main(x: u32) { - assert(entry_point(x) == 2); - swap_entry_point(x, x + 1); - assert(deep_entry_point(x) == 4); - multiple_values_entry_point(x); + unsafe { + assert(entry_point(x) == 2); + swap_entry_point(x, x + 1); + assert(deep_entry_point(x) == 4); + multiple_values_entry_point(x); + } } unconstrained fn returns_multiple_values(x: u32) -> (u32, u32, u32, u32) { diff --git a/test_programs/execution_success/brillig_fns_as_values/src/main.nr b/test_programs/execution_success/brillig_fns_as_values/src/main.nr index ea3148915b8..207a88985be 100644 --- a/test_programs/execution_success/brillig_fns_as_values/src/main.nr +++ b/test_programs/execution_success/brillig_fns_as_values/src/main.nr @@ -5,13 +5,15 @@ struct MyStruct { } fn main(x: u32) { - assert(wrapper(increment, x) == x + 1); - assert(wrapper(increment_acir, x) == x + 1); - assert(wrapper(decrement, x) == std::wrapping_sub(x, 1)); - assert(wrapper_with_struct(MyStruct { operation: increment }, x) == x + 1); - assert(wrapper_with_struct(MyStruct { operation: decrement }, x) == std::wrapping_sub(x, 1)); - // https://github.com/noir-lang/noir/issues/1975 - assert(increment(x) == x + 1); + unsafe { + assert(wrapper(increment, x) == x + 1); + assert(wrapper(increment_acir, x) == x + 1); + assert(wrapper(decrement, x) == std::wrapping_sub(x, 1)); + assert(wrapper_with_struct(MyStruct { operation: increment }, x) == x + 1); + assert(wrapper_with_struct(MyStruct { operation: decrement }, x) == std::wrapping_sub(x, 1)); + // https://github.com/noir-lang/noir/issues/1975 + assert(increment(x) == x + 1); + } } unconstrained fn wrapper(func: fn(u32) -> u32, param: u32) -> u32 { diff --git a/test_programs/execution_success/brillig_hash_to_field/src/main.nr b/test_programs/execution_success/brillig_hash_to_field/src/main.nr index 4b4177a521e..dadf8441284 100644 --- a/test_programs/execution_success/brillig_hash_to_field/src/main.nr +++ b/test_programs/execution_success/brillig_hash_to_field/src/main.nr @@ -3,7 +3,9 @@ use dep::std; // // The features being tested is hash_to_field in brillig fn main(input: Field) -> pub Field { - hash_to_field(input) + unsafe { + hash_to_field(input) + } } unconstrained fn hash_to_field(input: Field) -> Field { diff --git a/test_programs/execution_success/brillig_loop/src/main.nr b/test_programs/execution_success/brillig_loop/src/main.nr index 05d35469342..770660bb2a1 100644 --- a/test_programs/execution_success/brillig_loop/src/main.nr +++ b/test_programs/execution_success/brillig_loop/src/main.nr @@ -2,8 +2,10 @@ // // The features being tested is basic looping on brillig fn main(sum: u32) { - assert(loop(4) == sum); - assert(plain_loop() == sum); + unsafe { + assert(loop(4) == sum); + assert(plain_loop() == sum); + } } unconstrained fn loop(x: u32) -> u32 { diff --git a/test_programs/execution_success/brillig_nested_arrays/src/main.nr b/test_programs/execution_success/brillig_nested_arrays/src/main.nr index 5a5657246a8..80171d45a87 100644 --- a/test_programs/execution_success/brillig_nested_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_nested_arrays/src/main.nr @@ -28,14 +28,16 @@ unconstrained fn create_and_assert_inside_brillig(x: Field, y: Field) { } fn main(x: Field, y: Field) { - let header = Header { params: [1, 2, 3] }; - let note0 = MyNote { array: [1, 2], plain: 3, header }; - let note1 = MyNote { array: [4, 5], plain: 6, header }; - - assert(access_nested([note0, note1], x, y) == (2 + 4 + 3 + 1)); - - let notes = create_inside_brillig(); - assert_inside_brillig(notes, x, y); - create_and_assert_inside_brillig(x, y); + unsafe { + let header = Header { params: [1, 2, 3] }; + let note0 = MyNote { array: [1, 2], plain: 3, header }; + let note1 = MyNote { array: [4, 5], plain: 6, header }; + + assert(access_nested([note0, note1], x, y) == (2 + 4 + 3 + 1)); + + let notes = create_inside_brillig(); + assert_inside_brillig(notes, x, y); + create_and_assert_inside_brillig(x, y); + } } diff --git a/test_programs/execution_success/brillig_not/src/main.nr b/test_programs/execution_success/brillig_not/src/main.nr index d34b3edb4b6..557d1e2e31f 100644 --- a/test_programs/execution_success/brillig_not/src/main.nr +++ b/test_programs/execution_success/brillig_not/src/main.nr @@ -2,8 +2,10 @@ // // The features being tested is not instruction on brillig fn main(x: Field, y: Field) { - assert(false == not_operator(x as bool)); - assert(true == not_operator(y as bool)); + unsafe { + assert(false == not_operator(x as bool)); + assert(true == not_operator(y as bool)); + } } unconstrained fn not_operator(x: bool) -> bool { diff --git a/test_programs/execution_success/brillig_oracle/src/main.nr b/test_programs/execution_success/brillig_oracle/src/main.nr index 6a9e5806621..9a31652d6a3 100644 --- a/test_programs/execution_success/brillig_oracle/src/main.nr +++ b/test_programs/execution_success/brillig_oracle/src/main.nr @@ -3,21 +3,23 @@ use dep::std::test::OracleMock; // Tests oracle usage in brillig/unconstrained functions fn main(_x: Field) { - let size = 20; - // TODO: Add a method along the lines of `(0..size).to_array()`. - let mut mock_oracle_response = [0; 20]; - // TODO: Add an `array.reverse()` method. - let mut reversed_mock_oracle_response = [0; 20]; - for i in 0..size { - mock_oracle_response[i] = i; - reversed_mock_oracle_response[19 - i] = i; + unsafe { + let size = 20; + // TODO: Add a method along the lines of `(0..size).to_array()`. + let mut mock_oracle_response = [0; 20]; + // TODO: Add an `array.reverse()` method. + let mut reversed_mock_oracle_response = [0; 20]; + for i in 0..size { + mock_oracle_response[i] = i; + reversed_mock_oracle_response[19 - i] = i; + } + + // TODO: this method of returning a slice feels hacky. + let _ = OracleMock::mock("get_number_sequence").with_params(size).returns((20, mock_oracle_response)); + let _ = OracleMock::mock("get_reverse_number_sequence").with_params(size).returns((20, reversed_mock_oracle_response)); + + get_number_sequence_wrapper(size as Field); } - - // TODO: this method of returning a slice feels hacky. - let _ = OracleMock::mock("get_number_sequence").with_params(size).returns((20, mock_oracle_response)); - let _ = OracleMock::mock("get_reverse_number_sequence").with_params(size).returns((20, reversed_mock_oracle_response)); - - get_number_sequence_wrapper(size as Field); } // Define oracle functions which we have mocked above diff --git a/test_programs/execution_success/brillig_recursion/src/main.nr b/test_programs/execution_success/brillig_recursion/src/main.nr index a87ef28bc56..c69468013b1 100644 --- a/test_programs/execution_success/brillig_recursion/src/main.nr +++ b/test_programs/execution_success/brillig_recursion/src/main.nr @@ -2,7 +2,9 @@ // // The feature being tested is brillig recursion fn main(x: u32) { - assert(fibonacci(x) == 55); + unsafe { + assert(fibonacci(x) == 55); + } } unconstrained fn fibonacci(x: u32) -> u32 { diff --git a/test_programs/execution_success/brillig_sha256/src/main.nr b/test_programs/execution_success/brillig_sha256/src/main.nr index e76109df9c3..bad56a4d639 100644 --- a/test_programs/execution_success/brillig_sha256/src/main.nr +++ b/test_programs/execution_success/brillig_sha256/src/main.nr @@ -3,7 +3,9 @@ use dep::std; // // The features being tested is sha256 in brillig fn main(x: Field, result: [u8; 32]) { - assert(result == sha256(x)); + unsafe { + assert(result == sha256(x)); + } } unconstrained fn sha256(x: Field) -> [u8; 32] { diff --git a/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr b/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr index 5ec657b0d35..d4b74162cfb 100644 --- a/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr @@ -1,6 +1,8 @@ fn main(x: Field, y: Field) -> pub Field { - let notes = create_notes(x, y); - sum_x(notes, x, y) + unsafe { + let notes = create_notes(x, y); + sum_x(notes, x, y) + } } fn sum_x(notes: [Field; 2], x: Field, y: Field) -> Field { diff --git a/test_programs/execution_success/databus/src/main.nr b/test_programs/execution_success/databus/src/main.nr index 1cf95be8a22..0d4fb7d91ea 100644 --- a/test_programs/execution_success/databus/src/main.nr +++ b/test_programs/execution_success/databus/src/main.nr @@ -2,7 +2,9 @@ use dep::std; fn main(mut x: u32, y: call_data u32, z: call_data [u32; 4]) -> return_data u32 { let a = z[x]; - a + foo(y) + unsafe { + a + foo(y) + } } // Use an unconstrained function to force the compiler to avoid inlining diff --git a/test_programs/execution_success/global_consts/src/main.nr b/test_programs/execution_success/global_consts/src/main.nr index 3c8ecc67a0c..bb40a238d90 100644 --- a/test_programs/execution_success/global_consts/src/main.nr +++ b/test_programs/execution_success/global_consts/src/main.nr @@ -25,7 +25,7 @@ unconstrained fn calculate_global_value() -> Field { } // Regression test for https://github.com/noir-lang/noir/issues/4318 -global CALCULATED_GLOBAL: Field = calculate_global_value(); +global CALCULATED_GLOBAL: Field = unsafe { calculate_global_value() }; fn main( a: [Field; M + N - N], diff --git a/test_programs/execution_success/hashmap/src/utils.nr b/test_programs/execution_success/hashmap/src/utils.nr index 45c9ca9bbf7..59c4b356f7c 100644 --- a/test_programs/execution_success/hashmap/src/utils.nr +++ b/test_programs/execution_success/hashmap/src/utils.nr @@ -2,7 +2,7 @@ pub(crate) fn cut(input: [T; N]) -> [T; M] { assert(M as u64 < N as u64, "M should be less than N."); - let mut new = [dep::std::unsafe::zeroed(); M]; + let mut new = [dep::std::unsafe_func::zeroed(); M]; for i in 0..M { new[i] = input[i]; } diff --git a/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr b/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr index 1bcbd7d5421..9664b4d1ce6 100644 --- a/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr +++ b/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr @@ -20,7 +20,9 @@ unconstrained fn create_inside_brillig(values: [Field; 6]) -> [MyNote; 2] { } fn main(values: [Field; 6]) { - let notes = create_inside_brillig(values); + let notes = unsafe { + create_inside_brillig(values) + }; assert(access_nested(notes) == (2 + 4 + 3 + 1)); } diff --git a/test_programs/execution_success/regression_4124/src/main.nr b/test_programs/execution_success/regression_4124/src/main.nr index 49ff68ee6ad..2741ca481bb 100644 --- a/test_programs/execution_success/regression_4124/src/main.nr +++ b/test_programs/execution_success/regression_4124/src/main.nr @@ -11,7 +11,7 @@ impl MyDeserialize<1> for Field { } pub fn storage_read() -> [Field; N] { - dep::std::unsafe::zeroed() + dep::std::unsafe_func::zeroed() } struct PublicMutable { diff --git a/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr b/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr index a47ab37eb31..fb90e3d96c5 100644 --- a/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr +++ b/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr @@ -13,5 +13,7 @@ fn test_acir() { #[test(should_fail)] fn test_brillig() { - assert_eq(out_of_bounds_unconstrained_wrapper([0; 50], [0; 50]), 0); + unsafe { + assert_eq(out_of_bounds_unconstrained_wrapper([0; 50], [0; 50]), 0); + } } From 32d50f8a3fc95848ed9e51f1c0a76347a05f34a3 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 27 Feb 2024 23:26:05 +0000 Subject: [PATCH 06/67] chore: track `unsafe` status on `TypeChecker` --- .../noirc_frontend/src/hir/type_check/expr.rs | 94 ++++++++----------- .../noirc_frontend/src/hir/type_check/mod.rs | 18 +++- .../noirc_frontend/src/hir/type_check/stmt.rs | 75 ++++++--------- 3 files changed, 86 insertions(+), 101 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index ce0246f1e94..c073f90a14f 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -56,14 +56,13 @@ impl<'interner> TypeChecker<'interner> { /// an equivalent HirExpression::Call in the form `foo(a, b, c)`. This cannot /// be done earlier since we need to know the type of the object `a` to resolve which /// function `foo` to refer to. - pub(crate) fn check_expression(&mut self, expr_id: &ExprId, allow_unsafe_call: bool) -> Type { + pub(crate) fn check_expression(&mut self, expr_id: &ExprId) -> Type { let typ = match self.interner.expression(expr_id) { HirExpression::Ident(ident) => self.check_ident(ident, expr_id), HirExpression::Literal(literal) => { match literal { HirLiteral::Array(HirArrayLiteral::Standard(arr)) => { - let elem_types = - vecmap(&arr, |arg| self.check_expression(arg, allow_unsafe_call)); + let elem_types = vecmap(&arr, |arg| self.check_expression(arg)); let first_elem_type = elem_types .first() @@ -95,7 +94,7 @@ impl<'interner> TypeChecker<'interner> { arr_type } HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) => { - let elem_type = self.check_expression(&repeated_element, allow_unsafe_call); + let elem_type = self.check_expression(&repeated_element); let length = match length { Type::Constant(length) => { Type::constant_variable(length, self.interner) @@ -112,8 +111,7 @@ impl<'interner> TypeChecker<'interner> { } HirLiteral::FmtStr(string, idents) => { let len = Type::Constant(string.len() as u64); - let types = - vecmap(&idents, |elem| self.check_expression(elem, allow_unsafe_call)); + let types = vecmap(&idents, |elem| self.check_expression(elem)); Type::FmtString(Box::new(len), Box::new(Type::Tuple(types))) } HirLiteral::Unit => Type::Unit, @@ -121,8 +119,8 @@ impl<'interner> TypeChecker<'interner> { } HirExpression::Infix(infix_expr) => { // The type of the infix expression must be looked up from a type table - let lhs_type = self.check_expression(&infix_expr.lhs, allow_unsafe_call); - let rhs_type = self.check_expression(&infix_expr.rhs, allow_unsafe_call); + let lhs_type = self.check_expression(&infix_expr.lhs); + let rhs_type = self.check_expression(&infix_expr.rhs); let lhs_span = self.interner.expr_span(&infix_expr.lhs); let rhs_span = self.interner.expr_span(&infix_expr.rhs); @@ -151,9 +149,7 @@ impl<'interner> TypeChecker<'interner> { } } } - HirExpression::Index(index_expr) => { - self.check_index_expression(expr_id, index_expr, allow_unsafe_call) - } + HirExpression::Index(index_expr) => self.check_index_expression(expr_id, index_expr), HirExpression::Call(call_expr) => { // Need to setup these flags here as `self` is borrowed mutably to type check the rest of the call expression // These flags are later used to type check calls to unconstrained functions from constrained functions @@ -165,15 +161,15 @@ impl<'interner> TypeChecker<'interner> { self.check_if_deprecated(&call_expr.func); - let function = self.check_expression(&call_expr.func, allow_unsafe_call); + let function = self.check_expression(&call_expr.func); let args = vecmap(&call_expr.arguments, |arg| { - let typ = self.check_expression(arg, allow_unsafe_call); + let typ = self.check_expression(arg); (typ, *arg, self.interner.expr_span(arg)) }); if is_current_func_constrained && is_unconstrained_call { - if !allow_unsafe_call { + if !self.allow_unsafe { self.errors.push(TypeCheckError::Unsafe { span: self.interner.expr_span(expr_id), }); @@ -208,8 +204,7 @@ impl<'interner> TypeChecker<'interner> { return_type } HirExpression::MethodCall(mut method_call) => { - let mut object_type = - self.check_expression(&method_call.object, allow_unsafe_call).follow_bindings(); + let mut object_type = self.check_expression(&method_call.object).follow_bindings(); let method_name = method_call.method.0.contents.as_str(); match self.lookup_method(&object_type, method_name, expr_id) { Some(method_ref) => { @@ -244,24 +239,30 @@ impl<'interner> TypeChecker<'interner> { // Type check the new call now that it has been changed from a method call // to a function call. This way we avoid duplicating code. - self.check_expression(expr_id, allow_unsafe_call) + self.check_expression(expr_id) } None => Type::Error, } } HirExpression::Cast(cast_expr) => { // Evaluate the LHS - let lhs_type = self.check_expression(&cast_expr.lhs, allow_unsafe_call); + let lhs_type = self.check_expression(&cast_expr.lhs); let span = self.interner.expr_span(expr_id); self.check_cast(lhs_type, cast_expr.r#type, span) } HirExpression::Block(block_expr) => { let mut block_type = Type::Unit; - let allow_unsafe = allow_unsafe_call || block_expr.is_unsafe; + // Before entering the block we cache the old value of `allow_unsafe` so it can be restored. + let old_allow_unsafe = self.allow_unsafe; + + // If we're already in an unsafe block then entering a new block should preserve this even if + // the inner block isn't marked as unsafe. + self.allow_unsafe |= block_expr.is_unsafe; + let statements = block_expr.statements(); for (i, stmt) in statements.iter().enumerate() { - let expr_type = self.check_statement(stmt, allow_unsafe); + let expr_type = self.check_statement(stmt); if let crate::hir_def::stmt::HirStatement::Semi(expr) = self.interner.statement(stmt) @@ -282,24 +283,23 @@ impl<'interner> TypeChecker<'interner> { } } + // Finally, we restore the original value of `self.allow_unsafe`. + self.allow_unsafe = old_allow_unsafe; + block_type } HirExpression::Prefix(prefix_expr) => { - let rhs_type = self.check_expression(&prefix_expr.rhs, allow_unsafe_call); + let rhs_type = self.check_expression(&prefix_expr.rhs); let span = self.interner.expr_span(&prefix_expr.rhs); self.type_check_prefix_operand(&prefix_expr.operator, &rhs_type, span) } - HirExpression::If(if_expr) => self.check_if_expr(&if_expr, expr_id, allow_unsafe_call), - HirExpression::Constructor(constructor) => { - self.check_constructor(constructor, expr_id, allow_unsafe_call) - } - HirExpression::MemberAccess(access) => { - self.check_member_access(access, *expr_id, allow_unsafe_call) - } + HirExpression::If(if_expr) => self.check_if_expr(&if_expr, expr_id), + HirExpression::Constructor(constructor) => self.check_constructor(constructor, expr_id), + HirExpression::MemberAccess(access) => self.check_member_access(access, *expr_id), HirExpression::Error => Type::Error, - HirExpression::Tuple(elements) => Type::Tuple(vecmap(&elements, |elem| { - self.check_expression(elem, allow_unsafe_call) - })), + HirExpression::Tuple(elements) => { + Type::Tuple(vecmap(&elements, |elem| self.check_expression(elem))) + } HirExpression::Lambda(lambda) => { let captured_vars = vecmap(lambda.captures, |capture| { self.interner.definition_type(capture.ident.id) @@ -313,7 +313,7 @@ impl<'interner> TypeChecker<'interner> { typ }); - let actual_return = self.check_expression(&lambda.body, allow_unsafe_call); + let actual_return = self.check_expression(&lambda.body); let span = self.interner.expr_span(&lambda.body); self.unify(&actual_return, &lambda.return_type, || TypeCheckError::TypeMismatch { @@ -542,9 +542,8 @@ impl<'interner> TypeChecker<'interner> { &mut self, id: &ExprId, mut index_expr: expr::HirIndexExpression, - allow_unsafe_call: bool, ) -> Type { - let index_type = self.check_expression(&index_expr.index, allow_unsafe_call); + let index_type = self.check_expression(&index_expr.index); let span = self.interner.expr_span(&index_expr.index); index_type.unify( @@ -559,7 +558,7 @@ impl<'interner> TypeChecker<'interner> { // When writing `a[i]`, if `a : &mut ...` then automatically dereference `a` as many // times as needed to get the underlying array. - let lhs_type = self.check_expression(&index_expr.collection, allow_unsafe_call); + let lhs_type = self.check_expression(&index_expr.collection); let (new_lhs, lhs_type) = self.insert_auto_dereferences(index_expr.collection, lhs_type); index_expr.collection = new_lhs; self.interner.replace_expr(id, HirExpression::Index(index_expr)); @@ -612,14 +611,9 @@ impl<'interner> TypeChecker<'interner> { } } - fn check_if_expr( - &mut self, - if_expr: &expr::HirIfExpression, - expr_id: &ExprId, - allow_unsafe_call: bool, - ) -> Type { - let cond_type = self.check_expression(&if_expr.condition, allow_unsafe_call); - let then_type = self.check_expression(&if_expr.consequence, allow_unsafe_call); + fn check_if_expr(&mut self, if_expr: &expr::HirIfExpression, expr_id: &ExprId) -> Type { + let cond_type = self.check_expression(&if_expr.condition); + let then_type = self.check_expression(&if_expr.consequence); let expr_span = self.interner.expr_span(&if_expr.condition); @@ -632,7 +626,7 @@ impl<'interner> TypeChecker<'interner> { match if_expr.alternative { None => Type::Unit, Some(alternative) => { - let else_type = self.check_expression(&alternative, allow_unsafe_call); + let else_type = self.check_expression(&alternative); let expr_span = self.interner.expr_span(expr_id); self.unify(&then_type, &else_type, || { @@ -662,7 +656,6 @@ impl<'interner> TypeChecker<'interner> { &mut self, constructor: expr::HirConstructorExpression, expr_id: &ExprId, - allow_unsafe_call: bool, ) -> Type { let typ = constructor.r#type; let generics = constructor.struct_generics; @@ -682,7 +675,7 @@ impl<'interner> TypeChecker<'interner> { // mismatch here as long as we continue typechecking the rest of the program to the best // of our ability. if param_name == arg_ident.0.contents { - let arg_type = self.check_expression(&arg, allow_unsafe_call); + let arg_type = self.check_expression(&arg); let span = self.interner.expr_span(expr_id); self.unify_with_coercions(&arg_type, ¶m_type, arg, || { @@ -698,13 +691,8 @@ impl<'interner> TypeChecker<'interner> { Type::Struct(typ, generics) } - fn check_member_access( - &mut self, - mut access: expr::HirMemberAccess, - expr_id: ExprId, - allow_unsafe_call: bool, - ) -> Type { - let lhs_type = self.check_expression(&access.lhs, allow_unsafe_call).follow_bindings(); + fn check_member_access(&mut self, mut access: expr::HirMemberAccess, expr_id: ExprId) -> Type { + let lhs_type = self.check_expression(&access.lhs).follow_bindings(); let span = self.interner.expr_span(&expr_id); let access_lhs = &mut access.lhs; diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 57e2e47ed8c..b24f5df69bf 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -26,6 +26,11 @@ pub struct TypeChecker<'interner> { errors: Vec, current_function: Option, + /// Whether the `TypeChecker` should allow unsafe calls. + /// + /// This is generally only set to true within an `unsafe` block. + allow_unsafe: bool, + /// Trait constraints are collected during type checking until they are /// verified at the end of a function. This is because constraints arise /// on each variable, but it is only until function calls when the types @@ -164,11 +169,17 @@ fn function_info(interner: &NodeInterner, function_body_id: &ExprId) -> (noirc_e impl<'interner> TypeChecker<'interner> { fn new(interner: &'interner mut NodeInterner) -> Self { - Self { interner, errors: Vec::new(), trait_constraints: Vec::new(), current_function: None } + Self { + interner, + errors: Vec::new(), + allow_unsafe: false, + trait_constraints: Vec::new(), + current_function: None, + } } fn check_function_body(&mut self, body: &ExprId) -> Type { - self.check_expression(body, false) + self.check_expression(body) } pub fn check_global( @@ -178,11 +189,12 @@ impl<'interner> TypeChecker<'interner> { let mut this = Self { interner, errors: Vec::new(), + allow_unsafe: false, trait_constraints: Vec::new(), current_function: None, }; let statement = this.interner.get_global(id).let_statement; - this.check_statement(&statement, false); + this.check_statement(&statement); this.errors } diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 13595bc5bc5..d644891a5b0 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -19,7 +19,7 @@ impl<'interner> TypeChecker<'interner> { /// All statements have a unit type `()` as their type so the type of the statement /// is not interesting. Type checking must still be done on statements to ensure any /// expressions used within them are typed correctly. - pub(crate) fn check_statement(&mut self, stmt_id: &StmtId, allow_unsafe_call: bool) -> Type { + pub(crate) fn check_statement(&mut self, stmt_id: &StmtId) -> Type { match self.interner.statement(stmt_id) { // Lets lay out a convincing argument that the handling of // SemiExpressions and Expressions below is correct. @@ -42,27 +42,27 @@ impl<'interner> TypeChecker<'interner> { // // The reason why we still modify the database, is to make sure it is future-proof HirStatement::Expression(expr_id) => { - return self.check_expression(&expr_id, allow_unsafe_call); + return self.check_expression(&expr_id); } HirStatement::Semi(expr_id) => { - self.check_expression(&expr_id, allow_unsafe_call); + self.check_expression(&expr_id); } - HirStatement::Let(let_stmt) => self.check_let_stmt(let_stmt, allow_unsafe_call), + HirStatement::Let(let_stmt) => self.check_let_stmt(let_stmt), HirStatement::Constrain(constrain_stmt) => { - self.check_constrain_stmt(constrain_stmt, allow_unsafe_call); + self.check_constrain_stmt(constrain_stmt); } HirStatement::Assign(assign_stmt) => { - self.check_assign_stmt(assign_stmt, stmt_id, allow_unsafe_call); + self.check_assign_stmt(assign_stmt, stmt_id); } - HirStatement::For(for_loop) => self.check_for_loop(for_loop, allow_unsafe_call), + HirStatement::For(for_loop) => self.check_for_loop(for_loop), HirStatement::Error => (), } Type::Unit } - fn check_for_loop(&mut self, for_loop: HirForStatement, allow_unsafe_call: bool) { - let start_range_type = self.check_expression(&for_loop.start_range, allow_unsafe_call); - let end_range_type = self.check_expression(&for_loop.end_range, allow_unsafe_call); + fn check_for_loop(&mut self, for_loop: HirForStatement) { + let start_range_type = self.check_expression(&for_loop.start_range); + let end_range_type = self.check_expression(&for_loop.end_range); let start_span = self.interner.expr_span(&for_loop.start_range); let end_span = self.interner.expr_span(&for_loop.end_range); @@ -85,7 +85,7 @@ impl<'interner> TypeChecker<'interner> { self.interner.push_definition_type(for_loop.identifier.id, start_range_type); - self.check_expression(&for_loop.block, allow_unsafe_call); + self.check_expression(&for_loop.block); } /// Associate a given HirPattern with the given Type, and remember @@ -136,16 +136,10 @@ impl<'interner> TypeChecker<'interner> { } } - fn check_assign_stmt( - &mut self, - assign_stmt: HirAssignStatement, - stmt_id: &StmtId, - allow_unsafe_call: bool, - ) { - let expr_type = self.check_expression(&assign_stmt.expression, allow_unsafe_call); + fn check_assign_stmt(&mut self, assign_stmt: HirAssignStatement, stmt_id: &StmtId) { + let expr_type = self.check_expression(&assign_stmt.expression); let span = self.interner.expr_span(&assign_stmt.expression); - let (lvalue_type, new_lvalue, mutable) = - self.check_lvalue(&assign_stmt.lvalue, span, allow_unsafe_call); + let (lvalue_type, new_lvalue, mutable) = self.check_lvalue(&assign_stmt.lvalue, span); if !mutable { let (name, span) = self.get_lvalue_name_and_span(&assign_stmt.lvalue); @@ -187,12 +181,7 @@ impl<'interner> TypeChecker<'interner> { } /// Type check an lvalue - the left hand side of an assignment statement. - fn check_lvalue( - &mut self, - lvalue: &HirLValue, - assign_span: Span, - allow_unsafe_call: bool, - ) -> (Type, HirLValue, bool) { + fn check_lvalue(&mut self, lvalue: &HirLValue, assign_span: Span) -> (Type, HirLValue, bool) { match lvalue { HirLValue::Ident(ident, _) => { let mut mutable = true; @@ -211,8 +200,7 @@ impl<'interner> TypeChecker<'interner> { (typ.clone(), HirLValue::Ident(ident.clone(), typ), mutable) } HirLValue::MemberAccess { object, field_name, .. } => { - let (lhs_type, object, mut mutable) = - self.check_lvalue(object, assign_span, allow_unsafe_call); + let (lhs_type, object, mut mutable) = self.check_lvalue(object, assign_span); let mut object = Box::new(object); let span = field_name.span(); let field_name = field_name.clone(); @@ -244,7 +232,7 @@ impl<'interner> TypeChecker<'interner> { (object_type, lvalue, mutable) } HirLValue::Index { array, index, .. } => { - let index_type = self.check_expression(index, allow_unsafe_call); + let index_type = self.check_expression(index); let expr_span = self.interner.expr_span(index); index_type.unify( @@ -258,7 +246,7 @@ impl<'interner> TypeChecker<'interner> { ); let (mut lvalue_type, mut lvalue, mut mutable) = - self.check_lvalue(array, assign_span, allow_unsafe_call); + self.check_lvalue(array, assign_span); // Before we check that the lvalue is an array, try to dereference it as many times // as needed to unwrap any &mut wrappers. @@ -288,8 +276,7 @@ impl<'interner> TypeChecker<'interner> { (typ.clone(), HirLValue::Index { array, index: *index, typ }, mutable) } HirLValue::Dereference { lvalue, element_type: _ } => { - let (reference_type, lvalue, _) = - self.check_lvalue(lvalue, assign_span, allow_unsafe_call); + let (reference_type, lvalue, _) = self.check_lvalue(lvalue, assign_span); let lvalue = Box::new(lvalue); let element_type = Type::type_variable(self.interner.next_type_variable_id()); @@ -307,20 +294,23 @@ impl<'interner> TypeChecker<'interner> { } } - fn check_let_stmt(&mut self, let_stmt: HirLetStatement, allow_unsafe_call: bool) { - let resolved_type = - self.check_declaration(let_stmt.expression, let_stmt.r#type, allow_unsafe_call); + fn check_let_stmt(&mut self, let_stmt: HirLetStatement) { + let resolved_type = self.check_declaration(let_stmt.expression, let_stmt.r#type); // Set the type of the pattern to be equal to the annotated type self.bind_pattern(&let_stmt.pattern, resolved_type); } - fn check_constrain_stmt(&mut self, stmt: HirConstrainStatement, allow_unsafe_call: bool) { - let expr_type = self.check_expression(&stmt.0, allow_unsafe_call); + fn check_constrain_stmt(&mut self, stmt: HirConstrainStatement) { + let expr_type = self.check_expression(&stmt.0); let expr_span = self.interner.expr_span(&stmt.0); // Must type check the assertion message expression so that we instantiate bindings - stmt.2.map(|assert_msg_expr| self.check_expression(&assert_msg_expr, true)); + // We always allow unsafe calls for assert messages as these may be a dynamic assert message call. + let old_allow_unsafe = self.allow_unsafe; + self.allow_unsafe = true; + stmt.2.map(|assert_msg_expr| self.check_expression(&assert_msg_expr)); + self.allow_unsafe = old_allow_unsafe; self.unify(&expr_type, &Type::Bool, || TypeCheckError::TypeMismatch { expr_typ: expr_type.to_string(), @@ -332,14 +322,9 @@ impl<'interner> TypeChecker<'interner> { /// All declaration statements check that the user specified type(UST) is equal to the /// expression on the RHS, unless the UST is unspecified in which case /// the type of the declaration is inferred to match the RHS. - fn check_declaration( - &mut self, - rhs_expr: ExprId, - annotated_type: Type, - allow_unsafe_call: bool, - ) -> Type { + fn check_declaration(&mut self, rhs_expr: ExprId, annotated_type: Type) -> Type { // Type check the expression on the RHS - let expr_type = self.check_expression(&rhs_expr, allow_unsafe_call); + let expr_type = self.check_expression(&rhs_expr); // First check if the LHS is unspecified // If so, then we give it the same type as the expression From e7cdfebfd8ab9f31f861aebe7491fc281dce4929 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 27 Feb 2024 23:29:43 +0000 Subject: [PATCH 07/67] chore: small refactor --- compiler/noirc_frontend/src/hir/type_check/stmt.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index d644891a5b0..cf80ce7fcb5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -306,11 +306,14 @@ impl<'interner> TypeChecker<'interner> { let expr_span = self.interner.expr_span(&stmt.0); // Must type check the assertion message expression so that we instantiate bindings - // We always allow unsafe calls for assert messages as these may be a dynamic assert message call. - let old_allow_unsafe = self.allow_unsafe; - self.allow_unsafe = true; - stmt.2.map(|assert_msg_expr| self.check_expression(&assert_msg_expr)); - self.allow_unsafe = old_allow_unsafe; + if let Some(assert_msg_expr) = stmt.2 { + // We always allow unsafe calls for assert messages as these may be a dynamic assert message call. + // We then must restore the original value however. + let old_allow_unsafe = self.allow_unsafe; + self.allow_unsafe = true; + self.check_expression(&assert_msg_expr); + self.allow_unsafe = old_allow_unsafe; + }; self.unify(&expr_type, &Type::Bool, || TypeCheckError::TypeMismatch { expr_typ: expr_type.to_string(), From 6325f92ab9119f663069a575bf6d7b3ffd7ad83c Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 27 Feb 2024 23:50:43 +0000 Subject: [PATCH 08/67] chore: fix tests --- noir_stdlib/src/collections/bounded_vec.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir_stdlib/src/collections/bounded_vec.nr b/noir_stdlib/src/collections/bounded_vec.nr index 752b96d6591..04201b0fa19 100644 --- a/noir_stdlib/src/collections/bounded_vec.nr +++ b/noir_stdlib/src/collections/bounded_vec.nr @@ -5,7 +5,7 @@ struct BoundedVec { impl BoundedVec { pub fn new() -> Self { - let zeroed = crate::unsafe::zeroed(); + let zeroed = crate::unsafe_func::zeroed(); BoundedVec { storage: [zeroed; MaxLen], len: 0 } } @@ -68,7 +68,7 @@ impl BoundedVec { self.len -= 1; let elem = self.storage[self.len]; - self.storage[self.len] = crate::unsafe::zeroed(); + self.storage[self.len] = crate::unsafe_func::zeroed(); elem } From 3c89ab71c339b073531e96dc8615567e59dcbf3a Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 6 Jun 2024 17:42:19 +0000 Subject: [PATCH 09/67] chore: update elaborator and fix tests --- aztec_macros/src/transforms/functions.rs | 6 +- compiler/noirc_frontend/src/ast/statement.rs | 2 +- .../src/elaborator/expressions.rs | 12 +- compiler/noirc_frontend/src/elaborator/mod.rs | 4 +- .../noirc_frontend/src/elaborator/types.rs | 5 + .../src/hir/resolution/resolver.rs | 4 +- .../noirc_frontend/src/hir/type_check/expr.rs | 2 +- .../noirc_frontend/src/hir_def/function.rs | 6 +- compiler/noirc_frontend/src/parser/parser.rs | 2 +- .../array_length_defaulting/src/main.nr | 2 +- .../turbofish_generic_count/src/main.nr | 2 +- .../brillig_slice_input/src/main.nr | 12 +- .../acir_inside_brillig_recursion/src/main.nr | 4 +- .../aes128_encrypt/src/main.nr | 21 ++-- .../src/main.nr | 6 +- .../execution_success/bigint/src/main.nr | 2 +- .../brillig_array_to_slice/src/main.nr | 10 +- .../src/main.nr | 4 +- .../execution_success/generics/src/main.nr | 2 +- .../is_unconstrained/src/main.nr | 4 +- .../execution_success/unit_value/src/main.nr | 2 +- .../noir_test_success/mock_oracle/src/main.nr | 112 ++++++++++-------- 22 files changed, 138 insertions(+), 88 deletions(-) diff --git a/aztec_macros/src/transforms/functions.rs b/aztec_macros/src/transforms/functions.rs index 4ddf8cf580b..d1977b607c0 100644 --- a/aztec_macros/src/transforms/functions.rs +++ b/aztec_macros/src/transforms/functions.rs @@ -735,8 +735,10 @@ fn create_loop_over(var: Expression, loop_body: Vec) -> Statement { // What will be looped over // - `hasher.add({ident}[i] as Field)` - let for_loop_block = - expression(ExpressionKind::Block(BlockExpression { is_unsafe: false,statements: loop_body })); + let for_loop_block = expression(ExpressionKind::Block(BlockExpression { + is_unsafe: false, + statements: loop_body, + })); // `for i in 0..{ident}.len()` make_statement(StatementKind::For(ForLoopStatement { diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 100746e4644..37775d36919 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -665,7 +665,7 @@ impl ForRange { }); Statement { kind: StatementKind::Expression(Expression::new(block, for_loop_span)), - span: for_loop_span + span: for_loop_span, } } } diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index ed0b050b7a5..3e793355d5d 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -81,6 +81,13 @@ impl<'context> Elaborator<'context> { let mut block_type = Type::Unit; let mut statements = Vec::with_capacity(block.statements.len()); + // Before entering the block we cache the old value of `in_unsafe_block` so it can be restored. + let old_in_unsafe_block = self.in_unsafe_block; + + // If we're already in an unsafe block then entering a new block should preserve this even if + // the inner block isn't marked as unsafe. + self.in_unsafe_block |= block.is_unsafe; + for (i, statement) in block.statements.into_iter().enumerate() { let (id, stmt_type) = self.elaborate_statement(statement); statements.push(id); @@ -100,8 +107,11 @@ impl<'context> Elaborator<'context> { } } + // Finally, we restore the original value of `self.in_unsafe_block`. + self.in_unsafe_block = old_in_unsafe_block; + self.pop_scope(); - (HirBlockExpression { is_unsafe: false, statements }, block_type) + (HirBlockExpression { is_unsafe: block.is_unsafe, statements }, block_type) } fn elaborate_literal(&mut self, literal: Literal, span: Span) -> (HirExpression, Type) { diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index f703127231f..09c607a3ac2 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -89,6 +89,7 @@ pub struct Elaborator<'context> { file: FileId, + in_unsafe_block: bool, in_unconstrained_fn: bool, nested_loops: usize, @@ -180,6 +181,7 @@ impl<'context> Elaborator<'context> { interner: &mut context.def_interner, def_maps: &mut context.def_maps, file: FileId::dummy(), + in_unsafe_block: false, in_unconstrained_fn: false, nested_loops: 0, in_contract: false, @@ -639,7 +641,7 @@ impl<'context> Elaborator<'context> { .collect(); let statements = std::mem::take(&mut func.def.body.statements); - let body = BlockExpression { is_unsafe: false, statements }; + let body = BlockExpression { is_unsafe: false, statements }; let meta = FuncMeta { name: name_ident, diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 3baa7054fc5..1db3a958a6a 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1155,6 +1155,11 @@ impl<'context> Elaborator<'context> { let is_unconstrained_call = self.is_unconstrained_call(call.func); let crossing_runtime_boundary = is_current_func_constrained && is_unconstrained_call; if crossing_runtime_boundary { + if !self.in_unsafe_block { + self.push_err(TypeCheckError::Unsafe {span}); + return Type::Error; + } + let called_func_id = self .interner .lookup_function_from_expr(&call.func) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 619250ccf68..0368d6dbd50 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1979,8 +1979,8 @@ impl<'a> Resolver<'a> { } fn resolve_block(&mut self, block_expr: BlockExpression) -> HirBlockExpression { - let statements = self - .in_new_scope(|this| vecmap(block_expr.statements, |stmt| this.intern_stmt(stmt))); + let statements = + self.in_new_scope(|this| vecmap(block_expr.statements, |stmt| this.intern_stmt(stmt))); HirBlockExpression { is_unsafe: block_expr.is_unsafe, statements } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index d6618e73d0d..00c014c9530 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -340,7 +340,7 @@ impl<'interner> TypeChecker<'interner> { // Check that we are not passing a mutable reference from a constrained runtime to an unconstrained runtime if is_current_func_constrained && is_unconstrained_call { if !self.allow_unsafe { - self.errors.push(TypeCheckError::Unsafe {span}); + self.errors.push(TypeCheckError::Unsafe { span }); return Type::Error; } for (typ, _, _) in args.iter() { diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index b338e9f8caa..a92212bb50f 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -193,11 +193,7 @@ impl FuncMeta { let is_unsafe = block.is_unsafe; let (kind, span) = (*kind, *span); self.function_body = FunctionBody::Resolving; - FunctionBody::Unresolved( - kind, - BlockExpression { is_unsafe, statements }, - span, - ) + FunctionBody::Unresolved(kind, BlockExpression { is_unsafe, statements }, span) } FunctionBody::Resolving => FunctionBody::Resolving, FunctionBody::Resolved => FunctionBody::Resolved, diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index ddde3952868..8c6c5341e6b 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -430,7 +430,7 @@ fn block<'a>( |span| vec![Statement { kind: StatementKind::Error, span }], )), ) - .map(|(is_unsafe, statements)|BlockExpression { is_unsafe, statements }) + .map(|(is_unsafe, statements)| BlockExpression { is_unsafe, statements }) } fn check_statements_require_semicolon( diff --git a/test_programs/compile_failure/array_length_defaulting/src/main.nr b/test_programs/compile_failure/array_length_defaulting/src/main.nr index 216a9ae3f0c..f328f90cb07 100644 --- a/test_programs/compile_failure/array_length_defaulting/src/main.nr +++ b/test_programs/compile_failure/array_length_defaulting/src/main.nr @@ -1,5 +1,5 @@ fn main() { - let x = dep::std::unsafe::zeroed(); + let x = dep::std::unsafe_func::zeroed(); foo(x); } diff --git a/test_programs/compile_failure/turbofish_generic_count/src/main.nr b/test_programs/compile_failure/turbofish_generic_count/src/main.nr index a360641fa15..9478366c60c 100644 --- a/test_programs/compile_failure/turbofish_generic_count/src/main.nr +++ b/test_programs/compile_failure/turbofish_generic_count/src/main.nr @@ -7,7 +7,7 @@ struct Bar { impl Bar { fn zeroed(_self: Self) -> A { - dep::std::unsafe::zeroed() + dep::std::unsafe_func::zeroed() } } diff --git a/test_programs/compile_success_empty/brillig_slice_input/src/main.nr b/test_programs/compile_success_empty/brillig_slice_input/src/main.nr index 8403cb7d4a0..596f364b49f 100644 --- a/test_programs/compile_success_empty/brillig_slice_input/src/main.nr +++ b/test_programs/compile_success_empty/brillig_slice_input/src/main.nr @@ -25,8 +25,10 @@ fn main() { y: 8, } ]); - let brillig_sum = sum_slice(slice); - assert_eq(brillig_sum, 55); + unsafe { + let brillig_sum = sum_slice(slice); + assert_eq(brillig_sum, 55); + } slice = slice.push_back([ Point { @@ -38,6 +40,8 @@ fn main() { y: 13, } ]); - let brillig_sum = sum_slice(slice); - assert_eq(brillig_sum, 100); + unsafe { + let brillig_sum = sum_slice(slice); + assert_eq(brillig_sum, 100); + } } diff --git a/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr b/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr index 92f8524a771..49b7c00b6b9 100644 --- a/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr +++ b/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr @@ -1,5 +1,7 @@ fn main() { - assert_eq(fibonacci(3), fibonacci_hint(3)); + unsafe { + assert_eq(fibonacci(3), fibonacci_hint(3)); + } } unconstrained fn fibonacci_hint(x: u32) -> u32 { diff --git a/test_programs/execution_success/aes128_encrypt/src/main.nr b/test_programs/execution_success/aes128_encrypt/src/main.nr index f6ed0f309c3..5abe56187b8 100644 --- a/test_programs/execution_success/aes128_encrypt/src/main.nr +++ b/test_programs/execution_success/aes128_encrypt/src/main.nr @@ -33,12 +33,19 @@ unconstrained fn cipher(plaintext: [u8; 12], iv: [u8; 16], key: [u8; 16]) -> [u8 fn main(inputs: str<12>, iv: str<16>, key: str<16>, output: str<32>) { let result = std::aes128::aes128_encrypt(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()); - let output_bytes: [u8; 16] = decode_hex(output); - for i in 0..16 { - assert(result[i] == output_bytes[i]); - } - let unconstrained_result = cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()); - for i in 0..16 { - assert(unconstrained_result[i] == output_bytes[i]); + + let output_bytes: [u8; 16] = unsafe { + let output_bytes: [u8; 16] = decode_hex(output); + for i in 0..16 { + assert(result[i] == output_bytes[i]); + } + output_bytes + }; + + unsafe { + let unconstrained_result = cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()); + for i in 0..16 { + assert(unconstrained_result[i] == output_bytes[i]); + } } } diff --git a/test_programs/execution_success/array_to_slice_constant_length/src/main.nr b/test_programs/execution_success/array_to_slice_constant_length/src/main.nr index e81dd4a0c5f..5668a9ff388 100644 --- a/test_programs/execution_success/array_to_slice_constant_length/src/main.nr +++ b/test_programs/execution_success/array_to_slice_constant_length/src/main.nr @@ -5,6 +5,8 @@ unconstrained fn return_array(val: Field) -> [Field; 1] { } fn main(val: Field) { - let array = return_array(val); - assert_constant(array.as_slice().len()); + unsafe { + let array = return_array(val); + assert_constant(array.as_slice().len()); + } } diff --git a/test_programs/execution_success/bigint/src/main.nr b/test_programs/execution_success/bigint/src/main.nr index c454c2b66cd..ab109cfe430 100644 --- a/test_programs/execution_success/bigint/src/main.nr +++ b/test_programs/execution_success/bigint/src/main.nr @@ -17,7 +17,7 @@ fn main(mut x: [u8; 5], y: [u8; 5]) { let c = if x[0] != 0 { test_unconstrained1(a, b) } else { - test_unconstrained2(a, b) + unsafe {test_unconstrained2(a, b)} }; assert(c.array[0] == dep::std::wrapping_mul(x[0], y[0])); diff --git a/test_programs/execution_success/brillig_array_to_slice/src/main.nr b/test_programs/execution_success/brillig_array_to_slice/src/main.nr index 262d3b5d86a..f54adb39963 100644 --- a/test_programs/execution_success/brillig_array_to_slice/src/main.nr +++ b/test_programs/execution_success/brillig_array_to_slice/src/main.nr @@ -10,9 +10,11 @@ unconstrained fn brillig_as_slice(x: Field) -> (u32, Field, Field) { } fn main(x: Field) { - let (slice_len, dynamic_0, slice_0) = brillig_as_slice(x); - assert(slice_len == 1); - assert(dynamic_0 == 2); - assert(slice_0 == 2); + unsafe { + let (slice_len, dynamic_0, slice_0) = brillig_as_slice(x); + assert(slice_len == 1); + assert(dynamic_0 == 2); + assert(slice_0 == 2); + } } diff --git a/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr b/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr index 142290ecbe2..41f2688e751 100644 --- a/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr +++ b/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr @@ -38,11 +38,11 @@ struct Outer { // If we don't take into account block parameter liveness, this function will need 5*500=2500 stack items unconstrained fn main(conditions: [bool; 5]) -> pub Outer { let out0 = if conditions[0] { - let mut outer: Outer = dep::std::unsafe::zeroed(); + let mut outer: Outer = dep::std::unsafe_func::zeroed(); outer.middle_a.inner_a.a = 1; outer } else { - let mut outer: Outer= dep::std::unsafe::zeroed(); + let mut outer: Outer= dep::std::unsafe_func::zeroed(); outer.middle_f.inner_c.d = 2; outer }; diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index c8616960559..00435961aa7 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -34,7 +34,7 @@ impl Bar { impl Bar { // This is to test that we can use turbofish on methods as well fn zeroed(_self: Self) -> A { - dep::std::unsafe::zeroed() + dep::std::unsafe_func::zeroed() } } diff --git a/test_programs/execution_success/is_unconstrained/src/main.nr b/test_programs/execution_success/is_unconstrained/src/main.nr index af40af1029f..af3e10de937 100644 --- a/test_programs/execution_success/is_unconstrained/src/main.nr +++ b/test_programs/execution_success/is_unconstrained/src/main.nr @@ -9,6 +9,8 @@ unconstrained fn unconstrained_intermediate() { } fn main() { - unconstrained_intermediate(); + unsafe { + unconstrained_intermediate(); + } check(false); } diff --git a/test_programs/execution_success/unit_value/src/main.nr b/test_programs/execution_success/unit_value/src/main.nr index f3844e03cf2..1c643df140c 100644 --- a/test_programs/execution_success/unit_value/src/main.nr +++ b/test_programs/execution_success/unit_value/src/main.nr @@ -1,5 +1,5 @@ fn get_transaction() { - dep::std::unsafe::zeroed() + dep::std::unsafe_func::zeroed() } fn main() { diff --git a/test_programs/noir_test_success/mock_oracle/src/main.nr b/test_programs/noir_test_success/mock_oracle/src/main.nr index d840ffaef66..5e6f9467ddc 100644 --- a/test_programs/noir_test_success/mock_oracle/src/main.nr +++ b/test_programs/noir_test_success/mock_oracle/src/main.nr @@ -34,68 +34,84 @@ unconstrained fn struct_field(point: Point, array: [Field; 4]) -> Field { #[test(should_fail)] fn test_mock_no_returns() { - OracleMock::mock("void_field"); - void_field(); // Some return value must be set + unsafe { + OracleMock::mock("void_field"); + void_field(); // Some return value must be set + } } #[test] fn test_mock() { - OracleMock::mock("void_field").returns(10); - assert_eq(void_field(), 10); + unsafe { + OracleMock::mock("void_field").returns(10); + assert_eq(void_field(), 10); + } } #[test] fn test_multiple_mock() { - let first_mock = OracleMock::mock("void_field").returns(10); - OracleMock::mock("void_field").returns(42); + unsafe { + let first_mock = OracleMock::mock("void_field").returns(10); + OracleMock::mock("void_field").returns(42); - // The mocks are searched for in creation order, so the first one prevents the second from being called. - assert_eq(void_field(), 10); + // The mocks are searched for in creation order, so the first one prevents the second from being called. + assert_eq(void_field(), 10); - first_mock.clear(); - assert_eq(void_field(), 42); + first_mock.clear(); + assert_eq(void_field(), 42); + } } #[test] fn test_multiple_mock_times() { - OracleMock::mock("void_field").returns(10).times(2); - OracleMock::mock("void_field").returns(42); + unsafe { + OracleMock::mock("void_field").returns(10).times(2); + OracleMock::mock("void_field").returns(42); - assert_eq(void_field(), 10); - assert_eq(void_field(), 10); - assert_eq(void_field(), 42); + assert_eq(void_field(), 10); + assert_eq(void_field(), 10); + assert_eq(void_field(), 42); + } } #[test] fn test_mock_with_params() { - OracleMock::mock("field_field").with_params((5,)).returns(10); - assert_eq(field_field(5), 10); + unsafe { + OracleMock::mock("field_field").with_params((5,)).returns(10); + assert_eq(field_field(5), 10); + } } #[test] fn test_multiple_mock_with_params() { - OracleMock::mock("field_field").with_params((5,)).returns(10); - OracleMock::mock("field_field").with_params((7,)).returns(14); + unsafe { + OracleMock::mock("field_field").with_params((5,)).returns(10); + OracleMock::mock("field_field").with_params((7,)).returns(14); - assert_eq(field_field(5), 10); - assert_eq(field_field(7), 14); + assert_eq(field_field(5), 10); + assert_eq(field_field(7), 14); + } } #[test] fn test_mock_last_params() { - let mock = OracleMock::mock("field_field").returns(10); - assert_eq(field_field(5), 10); + unsafe { + let mock = OracleMock::mock("field_field").returns(10); + assert_eq(field_field(5), 10); - assert_eq(mock.get_last_params(), 5); + assert_eq(mock.get_last_params(), 5); + } } #[test] fn test_mock_last_params_many_calls() { - let mock = OracleMock::mock("field_field").returns(10); - assert_eq(field_field(5), 10); - assert_eq(field_field(7), 10); + unsafe { + let mock = OracleMock::mock("field_field").returns(10); + assert_eq(field_field(5), 10); + assert_eq(field_field(7), 10); - assert_eq(mock.get_last_params(), 7); + assert_eq(mock.get_last_params(), 7); + } } #[test] @@ -106,25 +122,25 @@ fn test_mock_struct_field() { let another_array = [4, 3, 2, 1]; let point = Point { x: 14, y: 27 }; - OracleMock::mock("struct_field").returns(42).times(2); - let timeless_mock = OracleMock::mock("struct_field").returns(0); - - assert_eq(42, struct_field(point, array)); - assert_eq(42, struct_field(point, array)); - // The times(2) mock is now cleared - - assert_eq(0, struct_field(point, array)); - - let last_params: (Point, [Field; 4]) = timeless_mock.get_last_params(); - assert_eq(last_params.0, point); - assert_eq(last_params.1, array); - - // We clear the mock with no times() to allow other mocks to be callable - timeless_mock.clear(); - - OracleMock::mock("struct_field").with_params((point, array)).returns(10); - OracleMock::mock("struct_field").with_params((point, another_array)).returns(20); - assert_eq(10, struct_field(point, array)); - assert_eq(20, struct_field(point, another_array)); + unsafe { + OracleMock::mock("struct_field").returns(42).times(2); + let timeless_mock = OracleMock::mock("struct_field").returns(0); + assert_eq(42, struct_field(point, array)); + assert_eq(42, struct_field(point, array)); + // The times(2) mock is now cleared + + assert_eq(0, struct_field(point, array)); + let last_params: (Point, [Field; 4]) = timeless_mock.get_last_params(); + assert_eq(last_params.0, point); + assert_eq(last_params.1, array); + + // We clear the mock with no times() to allow other mocks to be callable + timeless_mock.clear(); + + OracleMock::mock("struct_field").with_params((point, array)).returns(10); + OracleMock::mock("struct_field").with_params((point, another_array)).returns(20); + assert_eq(10, struct_field(point, array)); + assert_eq(20, struct_field(point, another_array)); + } } From ff0d1b3c8fe73ad85bb0c08ab1c249fc110169f5 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 6 Jun 2024 17:46:52 +0000 Subject: [PATCH 10/67] chore: fmt --- compiler/noirc_frontend/src/elaborator/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 1db3a958a6a..7f76f3f6342 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1156,7 +1156,7 @@ impl<'context> Elaborator<'context> { let crossing_runtime_boundary = is_current_func_constrained && is_unconstrained_call; if crossing_runtime_boundary { if !self.in_unsafe_block { - self.push_err(TypeCheckError::Unsafe {span}); + self.push_err(TypeCheckError::Unsafe { span }); return Type::Error; } From 94bb83687216118e2be5928e0f7d3cea76197695 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 7 Jun 2024 13:00:11 +0000 Subject: [PATCH 11/67] chore: fix formatter --- tooling/nargo_fmt/src/visitor/expr.rs | 5 +++++ tooling/nargo_fmt/tests/expected/unsafe.nr | 8 ++++++++ tooling/nargo_fmt/tests/input/unsafe.nr | 8 ++++++++ 3 files changed, 21 insertions(+) create mode 100644 tooling/nargo_fmt/tests/expected/unsafe.nr create mode 100644 tooling/nargo_fmt/tests/input/unsafe.nr diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 18b962a7f85..eabb5d4a0a3 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -112,6 +112,11 @@ impl FmtVisitor<'_> { } pub(crate) fn visit_block(&mut self, block: BlockExpression, block_span: Span) { + if block.is_unsafe { + self.push_str(self.slice(block_span)); + return + } + if block.is_empty() { self.visit_empty_block(block_span); return; diff --git a/tooling/nargo_fmt/tests/expected/unsafe.nr b/tooling/nargo_fmt/tests/expected/unsafe.nr new file mode 100644 index 00000000000..6e12ef975ee --- /dev/null +++ b/tooling/nargo_fmt/tests/expected/unsafe.nr @@ -0,0 +1,8 @@ +fn main(x: pub u8, y: u8) { + unsafe { } + + unsafe { + assert_eq(x, y); + } +} + diff --git a/tooling/nargo_fmt/tests/input/unsafe.nr b/tooling/nargo_fmt/tests/input/unsafe.nr new file mode 100644 index 00000000000..6e12ef975ee --- /dev/null +++ b/tooling/nargo_fmt/tests/input/unsafe.nr @@ -0,0 +1,8 @@ +fn main(x: pub u8, y: u8) { + unsafe { } + + unsafe { + assert_eq(x, y); + } +} + From 6e04e2d66af35823f4561616a7c8bf9962d52825 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 7 Jun 2024 14:00:21 +0000 Subject: [PATCH 12/67] chore: add comment --- tooling/nargo_fmt/src/visitor/expr.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index eabb5d4a0a3..3fb61d1d104 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -113,6 +113,7 @@ impl FmtVisitor<'_> { pub(crate) fn visit_block(&mut self, block: BlockExpression, block_span: Span) { if block.is_unsafe { + // We currently do not format unsafe blocks. self.push_str(self.slice(block_span)); return } From d56e7d21a876138604cad415652bcadf410a5aa4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 15 Jul 2024 15:04:03 -0300 Subject: [PATCH 13/67] Missing is_unsafe --- compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 22763c9cb64..1e0b507e84b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -423,6 +423,6 @@ impl HirBlockExpression { fn to_display_ast(&self, interner: &NodeInterner) -> BlockExpression { let statements = vecmap(self.statements.clone(), |statement| statement.to_display_ast(interner)); - BlockExpression { statements } + BlockExpression { statements, is_unsafe: self.is_unsafe } } } From e280c9e253120ab0f176f478548fc811a37c2038 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 15 Jul 2024 15:05:20 -0300 Subject: [PATCH 14/67] Remove unused in_unconstrained_fn var --- compiler/noirc_frontend/src/elaborator/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 9ffb6b1c1d0..c23e40b6171 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -97,7 +97,6 @@ pub struct Elaborator<'context> { file: FileId, in_unsafe_block: bool, - in_unconstrained_fn: bool, nested_loops: usize, /// Contains a mapping of the current struct or functions's generics to @@ -198,7 +197,6 @@ impl<'context> Elaborator<'context> { def_maps: &mut context.def_maps, file: FileId::dummy(), in_unsafe_block: false, - in_unconstrained_fn: false, nested_loops: 0, generics: Vec::new(), lambda_stack: Vec::new(), From d9618dc035d056a2a1d103ab8155b3acc262b895 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 15 Jul 2024 15:16:59 -0300 Subject: [PATCH 15/67] Improve error message --- compiler/noirc_frontend/src/hir/type_check/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index fb0e525820c..88c37c7fb9d 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -136,7 +136,7 @@ pub enum TypeCheckError { UnconstrainedReferenceToConstrained { span: Span }, #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] UnconstrainedSliceReturnToConstrained { span: Span }, - #[error("unsafe")] + #[error("Call to unconstrained function is unsafe and requires unconstrained function or unsafe block")] Unsafe { span: Span }, #[error("Slices must have constant length")] NonConstantSliceLength { span: Span }, From 8a4c6d7f3debe7f11965ef7335ebe3bef2849cbb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 15 Jul 2024 15:17:43 -0300 Subject: [PATCH 16/67] clippy --- tooling/nargo_fmt/src/visitor/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 3fb61d1d104..201c599f413 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -115,7 +115,7 @@ impl FmtVisitor<'_> { if block.is_unsafe { // We currently do not format unsafe blocks. self.push_str(self.slice(block_span)); - return + return; } if block.is_empty() { From 9a034ff125cad89385a3e83d717d6ade0377bf04 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 15 Jul 2024 15:30:39 -0300 Subject: [PATCH 17/67] Add a test for unconstrained called outside of unsafe --- compiler/noirc_frontend/src/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 321ac3f549d..00ea9ae6daa 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2140,3 +2140,20 @@ fn no_super() { assert_eq!(span.start(), 4); assert_eq!(span.end(), 9); } + +#[test] +fn cannot_call_unconstrained_function_outside_of_unsafe() { + let src = r#" + fn main() { + foo(); + } + + unconstrained fn foo() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &errors[0].0 else { + panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); + }; +} From feb8ed34aa352f8e805b01851e68c21b365b233b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 10:01:00 -0300 Subject: [PATCH 18/67] Fix debugger test case --- test_programs/execution_success/regression_5435/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/regression_5435/src/main.nr b/test_programs/execution_success/regression_5435/src/main.nr index 65f13c5b201..3146fb7c6f9 100644 --- a/test_programs/execution_success/regression_5435/src/main.nr +++ b/test_programs/execution_success/regression_5435/src/main.nr @@ -3,7 +3,7 @@ fn main(input: Field, enable: bool) { let hash = no_predicate_function(input); // `EnableSideEffects` instruction from above instruction leaks out and removes the predicate from this call, // resulting in execution failure. - fail(hash); + unsafe { fail(hash) }; } } From 83de376e1b20d8a2900c458af144c4187efa8d6d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 10:50:43 -0300 Subject: [PATCH 19/67] Keep track of unconstrained status of function type, disallow calling outside of unsafe --- compiler/noirc_driver/src/abi_gen.rs | 2 +- .../src/ssa/ssa_gen/context.rs | 2 +- compiler/noirc_frontend/src/ast/mod.rs | 7 ++- .../src/elaborator/expressions.rs | 2 +- compiler/noirc_frontend/src/elaborator/mod.rs | 7 ++- .../noirc_frontend/src/elaborator/traits.rs | 11 +++- .../noirc_frontend/src/elaborator/types.rs | 35 ++++++++---- .../src/hir/comptime/hir_to_display_ast.rs | 4 +- .../noirc_frontend/src/hir_def/function.rs | 4 +- compiler/noirc_frontend/src/hir_def/traits.rs | 8 +-- compiler/noirc_frontend/src/hir_def/types.rs | 56 +++++++++++++------ .../src/monomorphization/ast.rs | 13 ++++- .../src/monomorphization/mod.rs | 38 ++++++++----- compiler/noirc_frontend/src/node_interner.rs | 19 ++++--- .../noirc_frontend/src/parser/parser/types.rs | 5 +- compiler/noirc_frontend/src/tests.rs | 18 ++++++ compiler/noirc_printable_type/src/lib.rs | 1 + tooling/lsp/src/requests/inlay_hint.rs | 6 +- tooling/nargo_fmt/src/rewrite/typ.rs | 4 +- 19 files changed, 172 insertions(+), 70 deletions(-) diff --git a/compiler/noirc_driver/src/abi_gen.rs b/compiler/noirc_driver/src/abi_gen.rs index d0b33945f40..661b5ef674f 100644 --- a/compiler/noirc_driver/src/abi_gen.rs +++ b/compiler/noirc_driver/src/abi_gen.rs @@ -106,7 +106,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType { | Type::Forall(..) | Type::Quoted(_) | Type::Slice(_) - | Type::Function(_, _, _) => unreachable!("{typ} cannot be used in the abi"), + | Type::Function(_, _, _, _) => unreachable!("{typ} cannot be used in the abi"), Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"), Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 8e55debec1d..dca1679cccf 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -248,7 +248,7 @@ impl<'a> FunctionContext<'a> { } ast::Type::Unit => panic!("convert_non_tuple_type called on a unit type"), ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"), - ast::Type::Function(_, _, _) => Type::Function, + ast::Type::Function(_, _, _, _) => Type::Function, ast::Type::Slice(_) => panic!("convert_non_tuple_type called on a slice: {typ}"), ast::Type::MutableReference(element) => { // Recursive call to panic if element is a tuple diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index dfe4258744a..661c52333f2 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -115,6 +115,7 @@ pub enum UnresolvedTypeData { /*args:*/ Vec, /*ret:*/ Box, /*env:*/ Box, + /*unconstrained:*/ bool, ), // The type of quoted code for metaprogramming @@ -206,7 +207,11 @@ impl std::fmt::Display for UnresolvedTypeData { Bool => write!(f, "bool"), String(len) => write!(f, "str<{len}>"), FormatString(len, elements) => write!(f, "fmt<{len}, {elements}"), - Function(args, ret, env) => { + Function(args, ret, env, unconstrained) => { + if *unconstrained { + write!(f, "unconstrained ")? + } + let args = vecmap(args, ToString::to_string).join(", "); match &env.as_ref().typ { diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 53c80f538a1..43d9ae4570c 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -701,7 +701,7 @@ impl<'context> Elaborator<'context> { let captures = lambda_context.captures; let expr = HirExpression::Lambda(HirLambda { parameters, return_type, body, captures }); - (expr, Type::Function(arg_types, Box::new(body_type), Box::new(env_type))) + (expr, Type::Function(arg_types, Box::new(body_type), Box::new(env_type), false)) } fn elaborate_quote(&mut self, mut tokens: Tokens) -> (HirExpression, Type) { diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 168e9791471..55c751db45a 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -719,7 +719,12 @@ impl<'context> Elaborator<'context> { let return_type = Box::new(self.resolve_type(func.return_type())); - let mut typ = Type::Function(parameter_types, return_type, Box::new(Type::Unit)); + let mut typ = Type::Function( + parameter_types, + return_type, + Box::new(Type::Unit), + func.def.is_unconstrained, + ); if !generics.is_empty() { typ = Type::Forall(generics, Box::new(typ)); diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index 62bac1cc7d0..c0438e16749 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -152,8 +152,9 @@ impl<'context> Elaborator<'context> { }; let no_environment = Box::new(Type::Unit); + // TODO: unconstrained let function_type = - Type::Function(arguments, Box::new(return_type), no_environment); + Type::Function(arguments, Box::new(return_type), no_environment, false); functions.push(TraitFunction { name: name.clone(), @@ -323,9 +324,13 @@ fn check_function_type_matches_expected_type( ) { let mut bindings = TypeBindings::new(); // Shouldn't need to unify envs, they should always be equal since they're both free functions - if let (Type::Function(params_a, ret_a, _env_a), Type::Function(params_b, ret_b, _env_b)) = - (expected, actual) + if let ( + Type::Function(params_a, ret_a, _env_a, unconstrained_a), + Type::Function(params_b, ret_b, _env_b, unconstrained_b), + ) = (expected, actual) { + // TODO(ary): check unconstrained + if params_a.len() == params_b.len() { for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() { if a.try_unify(b, &mut bindings).is_err() { diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 9368fc45961..47862d13d86 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -124,7 +124,7 @@ impl<'context> Elaborator<'context> { Tuple(fields) => { Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field, kind))) } - Function(args, ret, env) => { + Function(args, ret, env, unconstrained) => { let args = vecmap(args, |arg| self.resolve_type_inner(arg, kind)); let ret = Box::new(self.resolve_type_inner(*ret, kind)); @@ -138,7 +138,7 @@ impl<'context> Elaborator<'context> { match *env { Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _, _) => { - Type::Function(args, ret, env) + Type::Function(args, ret, env, unconstrained) } _ => { self.push_err(ResolverError::InvalidClosureEnvironment { @@ -715,8 +715,11 @@ impl<'context> Elaborator<'context> { fn_params: &[Type], fn_ret: &Type, callsite_args: &[(Type, ExprId, Span)], + unconstrained: bool, span: Span, ) -> Type { + // TODO(ary): check unconstrained + if fn_params.len() != callsite_args.len() { self.push_err(TypeCheckError::ParameterCountMismatch { expected: fn_params.len(), @@ -754,7 +757,9 @@ impl<'context> Elaborator<'context> { let ret = self.interner.next_type_variable(); let args = vecmap(args, |(arg, _, _)| arg); let env_type = self.interner.next_type_variable(); - let expected = Type::Function(args, Box::new(ret.clone()), Box::new(env_type)); + let unconstrained = false; // TODO(ary): check unconstrained + let expected = + Type::Function(args, Box::new(ret.clone()), Box::new(env_type), unconstrained); if let Err(error) = binding.try_bind(expected, span) { self.push_err(error); @@ -763,8 +768,8 @@ impl<'context> Elaborator<'context> { } // The closure env is ignored on purpose: call arguments never place // constraints on closure environments. - Type::Function(parameters, ret, _env) => { - self.bind_function_type_impl(¶meters, &ret, &args, span) + Type::Function(parameters, ret, _env, unconstrained) => { + self.bind_function_type_impl(¶meters, &ret, &args, unconstrained, span) } Type::Error => Type::Error, found => { @@ -1114,7 +1119,9 @@ impl<'context> Elaborator<'context> { let (method_type, mut bindings) = method.typ.clone().instantiate(self.interner); match method_type { - Type::Function(args, _, _) => { + Type::Function(args, _, _, unconstrained) => { + // TODO(ary): check unconstrained + // We can cheat a bit and match against only the object type here since no operator // overload uses other generic parameters or return types aside from the object type. let expected_object_type = &args[0]; @@ -1294,7 +1301,15 @@ impl<'context> Elaborator<'context> { let is_current_func_constrained = self.in_constrained_function(); - let is_unconstrained_call = self.is_unconstrained_call(call.func); + let func_type_is_unconstrained = + if let Type::Function(_args, _ret, _env, unconstrained) = &func_type { + *unconstrained + } else { + false + }; + + let is_unconstrained_call = + func_type_is_unconstrained || self.is_unconstrained_call(call.func); let crossing_runtime_boundary = is_current_func_constrained && is_unconstrained_call; if crossing_runtime_boundary { if !self.in_unsafe_block { @@ -1358,9 +1373,9 @@ impl<'context> Elaborator<'context> { object: &mut ExprId, ) { let expected_object_type = match function_type { - Type::Function(args, _, _) => args.first(), + Type::Function(args, _, _, _) => args.first(), Type::Forall(_, typ) => match typ.as_ref() { - Type::Function(args, _, _) => args.first(), + Type::Function(args, _, _, _) => args.first(), typ => unreachable!("Unexpected type for function: {typ}"), }, typ => unreachable!("Unexpected type for function: {typ}"), @@ -1562,7 +1577,7 @@ impl<'context> Elaborator<'context> { } } - Type::Function(parameters, return_type, _env) => { + Type::Function(parameters, return_type, _env, _unconstrained) => { for parameter in parameters { Self::find_numeric_generics_in_type(parameter, found); } diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 1e0b507e84b..5d3c6501eb1 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -334,11 +334,11 @@ impl Type { let name = Path::from_single(name.as_ref().clone(), Span::default()); UnresolvedTypeData::TraitAsType(name, Vec::new()) } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, unconstrained) => { let args = vecmap(args, |arg| arg.to_display_ast()); let ret = Box::new(ret.to_display_ast()); let env = Box::new(env.to_display_ast()); - UnresolvedTypeData::Function(args, ret, env) + UnresolvedTypeData::Function(args, ret, env, *unconstrained) } Type::MutableReference(element) => { let element = Box::new(element.to_display_ast()); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 490ef1028f7..1835b802327 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -184,9 +184,9 @@ impl FuncMeta { /// Gives the (uninstantiated) return type of this function. pub fn return_type(&self) -> &Type { match &self.typ { - Type::Function(_, ret, _env) => ret, + Type::Function(_, ret, _env, _unconstrained) => ret, Type::Forall(_, typ) => match typ.as_ref() { - Type::Function(_, ret, _env) => ret, + Type::Function(_, ret, _env, _unconstrained) => ret, _ => unreachable!(), }, _ => unreachable!(), diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index 0600706922b..b36d59b94a2 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -139,9 +139,9 @@ impl std::fmt::Display for Trait { impl TraitFunction { pub fn arguments(&self) -> &[Type] { match &self.typ { - Type::Function(args, _, _) => args, + Type::Function(args, _, _, _) => args, Type::Forall(_, typ) => match typ.as_ref() { - Type::Function(args, _, _) => args, + Type::Function(args, _, _, _) => args, _ => unreachable!("Trait function does not have a function type"), }, _ => unreachable!("Trait function does not have a function type"), @@ -158,9 +158,9 @@ impl TraitFunction { pub fn return_type(&self) -> &Type { match &self.typ { - Type::Function(_, return_type, _) => return_type, + Type::Function(_, return_type, _, _) => return_type, Type::Forall(_, typ) => match typ.as_ref() { - Type::Function(_, return_type, _) => return_type, + Type::Function(_, return_type, _, _) => return_type, _ => unreachable!("Trait function does not have a function type"), }, _ => unreachable!("Trait function does not have a function type"), diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 91a374a0787..0936ae62eb4 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -88,7 +88,12 @@ pub enum Type { /// the environment should be `Unit` by default, /// for closures it should contain a `Tuple` type with the captured /// variable types. - Function(Vec, /*return_type:*/ Box, /*environment:*/ Box), + Function( + Vec, + /*return_type:*/ Box, + /*environment:*/ Box, + /*unconstrained*/ bool, + ), /// &mut T MutableReference(Box), @@ -627,7 +632,11 @@ impl std::fmt::Display for Type { let typevars = vecmap(typevars, |var| var.id().to_string()); write!(f, "forall {}. {}", typevars.join(" "), typ) } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, unconstrained) => { + if *unconstrained { + write!(f, "unconstrained ")?; + } + let closure_env_text = match **env { Type::Unit => "".to_string(), _ => format!(" with env {env}"), @@ -807,7 +816,7 @@ impl Type { Type::Tuple(fields) => { fields.iter().any(|field| field.contains_numeric_typevar(target_id)) } - Type::Function(parameters, return_type, env) => { + Type::Function(parameters, return_type, env, _unconstrained) => { parameters.iter().any(|parameter| parameter.contains_numeric_typevar(target_id)) || return_type.contains_numeric_typevar(target_id) || env.contains_numeric_typevar(target_id) @@ -884,7 +893,7 @@ impl Type { field.find_numeric_type_vars(found_names); } } - Type::Function(parameters, return_type, env) => { + Type::Function(parameters, return_type, env, _unconstrained) => { for parameter in parameters.iter() { parameter.find_numeric_type_vars(found_names); } @@ -937,7 +946,7 @@ impl Type { Type::FmtString(_, _) | Type::TypeVariable(_, _) | Type::NamedGeneric(_, _, _) - | Type::Function(_, _, _) + | Type::Function(_, _, _, _) | Type::MutableReference(_) | Type::Forall(_, _) | Type::Quoted(_) @@ -984,7 +993,7 @@ impl Type { Type::FmtString(_, _) // To enable this we would need to determine the size of the closure outputs at compile-time. // This is possible as long as the output size is not dependent upon a witness condition. - | Type::Function(_, _, _) + | Type::Function(_, _, _, _) | Type::Slice(_) | Type::MutableReference(_) | Type::Forall(_, _) @@ -1022,7 +1031,7 @@ impl Type { | Type::Slice(_) | Type::TypeVariable(_, _) | Type::NamedGeneric(_, _, _) - | Type::Function(_, _, _) + | Type::Function(_, _, _, _) | Type::FmtString(_, _) | Type::Error => true, @@ -1152,7 +1161,7 @@ impl Type { | Type::TypeVariable(_, _) | Type::TraitAsType(..) | Type::NamedGeneric(_, _, _) - | Type::Function(_, _, _) + | Type::Function(_, _, _, _) | Type::MutableReference(_) | Type::Forall(_, _) | Type::Constant(_) @@ -1509,7 +1518,12 @@ impl Type { } } - (Function(params_a, ret_a, env_a), Function(params_b, ret_b, env_b)) => { + ( + Function(params_a, ret_a, env_a, unconstrained_a), + Function(params_b, ret_b, env_b, unconstrained_b), + ) => { + // TODO(ary): check unconstrained + if params_a.len() == params_b.len() { for (a, b) in params_a.iter().zip(params_b.iter()) { a.try_unify(b, bindings)?; @@ -1876,13 +1890,13 @@ impl Type { let typ = Box::new(typ.substitute_helper(type_bindings, substitute_bound_typevars)); Type::Forall(typevars.clone(), typ) } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, unconstrained) => { let args = vecmap(args, |arg| { arg.substitute_helper(type_bindings, substitute_bound_typevars) }); let ret = Box::new(ret.substitute_helper(type_bindings, substitute_bound_typevars)); let env = Box::new(env.substitute_helper(type_bindings, substitute_bound_typevars)); - Type::Function(args, ret, env) + Type::Function(args, ret, env, *unconstrained) } Type::MutableReference(element) => Type::MutableReference(Box::new( element.substitute_helper(type_bindings, substitute_bound_typevars), @@ -1931,7 +1945,7 @@ impl Type { Type::Forall(typevars, typ) => { !typevars.iter().any(|var| var.id() == target_id) && typ.occurs(target_id) } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, _unconstrained) => { args.iter().any(|arg| arg.occurs(target_id)) || ret.occurs(target_id) || env.occurs(target_id) @@ -1984,11 +1998,11 @@ impl Type { self.clone() } - Function(args, ret, env) => { + Function(args, ret, env, unconstrained) => { let args = vecmap(args, |arg| arg.follow_bindings()); let ret = Box::new(ret.follow_bindings()); let env = Box::new(env.follow_bindings()); - Function(args, ret, env) + Function(args, ret, env, *unconstrained) } MutableReference(element) => MutableReference(Box::new(element.follow_bindings())), @@ -2075,7 +2089,7 @@ impl Type { *self = Type::TypeVariable(var.clone(), TypeVariableKind::Normal); } } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, _unconstrained) => { for arg in args { arg.replace_named_generics_with_type_variables(); } @@ -2118,7 +2132,8 @@ fn convert_array_expression_to_slice( interner.push_expr_location(func, location.span, location.file); interner.push_expr_type(expression, target_type.clone()); - let func_type = Type::Function(vec![array_type], Box::new(target_type), Box::new(Type::Unit)); + let func_type = + Type::Function(vec![array_type], Box::new(target_type), Box::new(Type::Unit), false); interner.push_expr_type(func, func_type); } @@ -2206,10 +2221,11 @@ impl From<&Type> for PrintableType { Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(arguments, return_type, env) => PrintableType::Function { + Type::Function(arguments, return_type, env, unconstrained) => PrintableType::Function { arguments: arguments.iter().map(|arg| arg.into()).collect(), return_type: Box::new(return_type.as_ref().into()), env: Box::new(env.as_ref().into()), + unconstrained: *unconstrained, }, Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } @@ -2293,7 +2309,11 @@ impl std::fmt::Debug for Type { let typevars = vecmap(typevars, |var| format!("{:?}", var)); write!(f, "forall {}. {:?}", typevars.join(" "), typ) } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, unconstrained) => { + if *unconstrained { + write!(f, "unconstrained ")?; + } + let closure_env_text = match **env { Type::Unit => "".to_string(), _ => format!(" with env {env:?}"), diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 47698c5b65c..c0101dc0d17 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -287,7 +287,12 @@ pub enum Type { Tuple(Vec), Slice(Box), MutableReference(Box), - Function(/*args:*/ Vec, /*ret:*/ Box, /*env:*/ Box), + Function( + /*args:*/ Vec, + /*ret:*/ Box, + /*env:*/ Box, + /*unconstrained:*/ bool, + ), } impl Type { @@ -418,7 +423,11 @@ impl std::fmt::Display for Type { let elements = vecmap(elements, ToString::to_string); write!(f, "({})", elements.join(", ")) } - Type::Function(args, ret, env) => { + Type::Function(args, ret, env, unconstrained) => { + if *unconstrained { + write!(f, "unconstrained ")?; + } + let args = vecmap(args, ToString::to_string); let closure_env_text = match **env { Type::Unit => "".to_string(), diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index a46f32e3094..4ae86bb1170 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -989,15 +989,17 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Tuple(fields) } - HirType::Function(args, ret, env) => { + HirType::Function(args, ret, env, unconstrained) => { let args = try_vecmap(args, |x| Self::convert_type(x, location))?; let ret = Box::new(Self::convert_type(ret, location)?); let env = Self::convert_type(env, location)?; match &env { - ast::Type::Unit => ast::Type::Function(args, ret, Box::new(env)), + ast::Type::Unit => { + ast::Type::Function(args, ret, Box::new(env), *unconstrained) + } ast::Type::Tuple(_elements) => ast::Type::Tuple(vec![ env.clone(), - ast::Type::Function(args, ret, Box::new(env)), + ast::Type::Function(args, ret, Box::new(env), *unconstrained), ]), _ => { unreachable!( @@ -1024,7 +1026,7 @@ impl<'interner> Monomorphizer<'interner> { true } else if let ast::Type::Tuple(elements) = t { if elements.len() == 2 { - matches!(elements[1], ast::Type::Function(_, _, _)) + matches!(elements[1], ast::Type::Function(_, _, _, _)) } else { false } @@ -1034,7 +1036,7 @@ impl<'interner> Monomorphizer<'interner> { } fn is_function_closure_type(&self, t: &ast::Type) -> bool { - if let ast::Type::Function(_, _, env) = t { + if let ast::Type::Function(_, _, env, _) = t { let e = (*env).clone(); matches!(*e, ast::Type::Tuple(_captures)) } else { @@ -1401,8 +1403,12 @@ impl<'interner> Monomorphizer<'interner> { }; self.push_function(id, function); - let typ = - ast::Type::Function(parameter_types, Box::new(ret_type), Box::new(ast::Type::Unit)); + let typ = ast::Type::Function( + parameter_types, + Box::new(ret_type), + Box::new(ast::Type::Unit), + false, + ); let name = lambda_name.to_owned(); Ok(ast::Expression::Ident(ast::Ident { @@ -1470,7 +1476,7 @@ impl<'interner> Monomorphizer<'interner> { })?); let expr_type = self.interner.id_type(expr); - let env_typ = if let types::Type::Function(_, _, function_env_type) = expr_type { + let env_typ = if let types::Type::Function(_, _, function_env_type, _) = expr_type { Self::convert_type(&function_env_type, location)? } else { unreachable!("expected a Function type for a Lambda node") @@ -1500,8 +1506,12 @@ impl<'interner> Monomorphizer<'interner> { let body = self.expr(lambda.body)?; self.lambda_envs_stack.pop(); - let lambda_fn_typ: ast::Type = - ast::Type::Function(parameter_types, Box::new(ret_type), Box::new(env_typ.clone())); + let lambda_fn_typ: ast::Type = ast::Type::Function( + parameter_types, + Box::new(ret_type), + Box::new(env_typ.clone()), + false, + ); let lambda_fn = ast::Expression::Ident(ast::Ident { definition: Definition::Function(id), mutable: false, @@ -1591,9 +1601,8 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Tuple(fields) => ast::Expression::Tuple(vecmap(fields, |field| { self.zeroed_value_of_type(field, location) })), - ast::Type::Function(parameter_types, ret_type, env) => { - self.create_zeroed_function(parameter_types, ret_type, env, location) - } + ast::Type::Function(parameter_types, ret_type, env, unconstrained) => self + .create_zeroed_function(parameter_types, ret_type, env, *unconstrained, location), ast::Type::Slice(element_type) => { ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents: vec![], @@ -1625,6 +1634,7 @@ impl<'interner> Monomorphizer<'interner> { parameter_types: &[ast::Type], ret_type: &ast::Type, env_type: &ast::Type, + unconstrained: bool, location: noirc_errors::Location, ) -> ast::Expression { let lambda_name = "zeroed_lambda"; @@ -1639,7 +1649,6 @@ impl<'interner> Monomorphizer<'interner> { let return_type = ret_type.clone(); let name = lambda_name.to_owned(); - let unconstrained = false; let function = ast::Function { id, name, @@ -1661,6 +1670,7 @@ impl<'interner> Monomorphizer<'interner> { parameter_types.to_owned(), Box::new(ret_type.clone()), Box::new(env_type.clone()), + unconstrained, ), }) } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 1c7d0984b2f..6d17c703a68 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1180,14 +1180,19 @@ impl NodeInterner { pub fn id_type_substitute_trait_as_type(&self, def_id: DefinitionId) -> Type { let typ = self.definition_type(def_id); - if let Type::Function(args, ret, env) = &typ { + if let Type::Function(args, ret, env, unconstrained) = &typ { let def = self.definition(def_id); if let Type::TraitAsType(..) = ret.as_ref() { if let DefinitionKind::Function(func_id) = def.kind { let f = self.function(&func_id); let func_body = f.as_expr(); let ret_type = self.id_type(func_body); - let new_type = Type::Function(args.clone(), Box::new(ret_type), env.clone()); + let new_type = Type::Function( + args.clone(), + Box::new(ret_type), + env.clone(), + *unconstrained, + ); return new_type; } } @@ -1748,7 +1753,7 @@ impl NodeInterner { let the_trait = self.get_trait(trait_id); self.ordering_type = match &the_trait.methods[0].typ { Type::Forall(_, typ) => match typ.as_ref() { - Type::Function(_, return_type, _) => Some(return_type.as_ref().clone()), + Type::Function(_, return_type, _, _) => Some(return_type.as_ref().clone()), other => unreachable!("Expected function type for `cmp`, found {}", other), }, other => unreachable!("Expected Forall type for `cmp`, found {}", other), @@ -1946,7 +1951,7 @@ impl NodeInterner { }; let env = Box::new(Type::Unit); - (Type::Function(args, Box::new(ret.clone()), env), ret) + (Type::Function(args, Box::new(ret.clone()), env, false), ret) } /// Returns the type of a prefix operator (which is always a function), along with its return type. @@ -1955,7 +1960,7 @@ impl NodeInterner { let args = vec![rhs_type]; let ret = self.id_type(operator_expr); let env = Box::new(Type::Unit); - (Type::Function(args, Box::new(ret.clone()), env), ret) + (Type::Function(args, Box::new(ret.clone()), env, false), ret) } } @@ -1992,7 +1997,7 @@ impl Methods { // at most 1 matching method in this list. for method in self.iter() { match interner.function_meta(&method).typ.instantiate(interner).0 { - Type::Function(args, _, _) => { + Type::Function(args, _, _, _) => { if let Some(object) = args.first() { let mut bindings = TypeBindings::new(); @@ -2043,7 +2048,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::FmtString(_, _) => Some(FmtString), Type::Unit => Some(Unit), Type::Tuple(_) => Some(Tuple), - Type::Function(_, _, _) => Some(Function), + Type::Function(_, _, _, _) => Some(Function), Type::NamedGeneric(_, _, _) => Some(Generic), Type::Quoted(quoted) => Some(Quoted(*quoted)), Type::MutableReference(element) => get_type_method_key(element), diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs index 7b69b309bae..b6b1c353cb3 100644 --- a/compiler/noirc_frontend/src/parser/parser/types.rs +++ b/compiler/noirc_frontend/src/parser/parser/types.rs @@ -269,13 +269,16 @@ where t.unwrap_or_else(|| UnresolvedTypeData::Unit.with_span(Span::empty(span.end()))) }); + // TODO(ary): allow "unconstrained" keyword here + let unconstrained = false; + keyword(Keyword::Fn) .ignore_then(env) .then(args) .then_ignore(just(Token::Arrow)) .then(type_parser) .map_with_span(|((env, args), ret), span| { - UnresolvedTypeData::Function(args, Box::new(ret), Box::new(env)).with_span(span) + UnresolvedTypeData::Function(args, Box::new(ret), Box::new(env), false).with_span(span) }) } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 1de34456ae1..57e0ecf4244 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2153,3 +2153,21 @@ fn cannot_call_unconstrained_function_outside_of_unsafe() { panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); }; } + +#[test] +fn cannot_call_unconstrained_captured_function_outside_of_unsafe() { + let src = r#" + fn main() { + let func = foo; + func(); + } + + unconstrained fn foo() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &errors[0].0 else { + panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); + }; +} diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index dfecd301b35..5ab04c6f576 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -40,6 +40,7 @@ pub enum PrintableType { arguments: Vec, return_type: Box, env: Box, + unconstrained: bool, }, MutableReference { typ: Box, diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 2d8830d877f..4330317f625 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -411,7 +411,11 @@ fn push_type_parts(typ: &Type, parts: &mut Vec, files: &File parts.push(string_part(">")); } } - Type::Function(args, return_type, _env) => { + Type::Function(args, return_type, _env, unconstrained) => { + if *unconstrained { + parts.push(string_part("unconstrained ")); + } + parts.push(string_part("fn(")); for (index, arg) in args.iter().enumerate() { push_type_parts(arg, parts, files); diff --git a/tooling/nargo_fmt/src/rewrite/typ.rs b/tooling/nargo_fmt/src/rewrite/typ.rs index 3298ed8ae73..e84fd453913 100644 --- a/tooling/nargo_fmt/src/rewrite/typ.rs +++ b/tooling/nargo_fmt/src/rewrite/typ.rs @@ -37,7 +37,9 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) format!("({types})") } } - UnresolvedTypeData::Function(args, return_type, env) => { + UnresolvedTypeData::Function(args, return_type, env, unconstrained) => { + // TODO(ary): format unconstrained here + let env = if span_is_empty(env.span.unwrap()) { "".into() } else { From 0c09df8e5ebee9de187e1b4e329053cf12d35d1e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 11:02:32 -0300 Subject: [PATCH 20/67] `unconstrained fn` and `fn` don't unify --- .../noirc_frontend/src/elaborator/types.rs | 11 ++----- compiler/noirc_frontend/src/hir_def/types.rs | 6 ++-- compiler/noirc_frontend/src/tests.rs | 29 +++++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 47862d13d86..414ac104eb2 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -715,11 +715,8 @@ impl<'context> Elaborator<'context> { fn_params: &[Type], fn_ret: &Type, callsite_args: &[(Type, ExprId, Span)], - unconstrained: bool, span: Span, ) -> Type { - // TODO(ary): check unconstrained - if fn_params.len() != callsite_args.len() { self.push_err(TypeCheckError::ParameterCountMismatch { expected: fn_params.len(), @@ -768,8 +765,8 @@ impl<'context> Elaborator<'context> { } // The closure env is ignored on purpose: call arguments never place // constraints on closure environments. - Type::Function(parameters, ret, _env, unconstrained) => { - self.bind_function_type_impl(¶meters, &ret, &args, unconstrained, span) + Type::Function(parameters, ret, _env, _unconstrained) => { + self.bind_function_type_impl(¶meters, &ret, &args, span) } Type::Error => Type::Error, found => { @@ -1119,9 +1116,7 @@ impl<'context> Elaborator<'context> { let (method_type, mut bindings) = method.typ.clone().instantiate(self.interner); match method_type { - Type::Function(args, _, _, unconstrained) => { - // TODO(ary): check unconstrained - + Type::Function(args, _, _, _) => { // We can cheat a bit and match against only the object type here since no operator // overload uses other generic parameters or return types aside from the object type. let expected_object_type = &args[0]; diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 0936ae62eb4..d09e2a2f20a 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1522,9 +1522,9 @@ impl Type { Function(params_a, ret_a, env_a, unconstrained_a), Function(params_b, ret_b, env_b, unconstrained_b), ) => { - // TODO(ary): check unconstrained - - if params_a.len() == params_b.len() { + if *unconstrained_a != *unconstrained_b { + Err(UnificationError) + } else if params_a.len() == params_b.len() { for (a, b) in params_a.iter().zip(params_b.iter()) { a.try_unify(b, bindings)?; } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 57e0ecf4244..1805d1badd0 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2171,3 +2171,32 @@ fn cannot_call_unconstrained_captured_function_outside_of_unsafe() { panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); }; } + +#[test] +fn cannot_pass_unconstrained_function_to_regular_function() { + let src = r#" + fn main() { + let func = foo; + expect_regular(func); + } + + unconstrained fn foo() {} + + fn expect_regular(_func: fn() -> ()) { + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + if let CompilationError::TypeError(TypeCheckError::TypeMismatch { + expected_typ, + expr_typ, + .. + }) = &errors[0].0 + { + assert_eq!(expected_typ, "fn() -> ()"); + assert_eq!(expr_typ, "unconstrained fn() -> ()"); + } else { + panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); + }; +} From df13d6c95c1fc6a2457fe4867b27df610c33d26f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 11:09:03 -0300 Subject: [PATCH 21/67] Can't check unconstrained match on traits and impls yet --- compiler/noirc_frontend/src/elaborator/traits.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index c0438e16749..e4b0bc98106 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -325,11 +325,13 @@ fn check_function_type_matches_expected_type( let mut bindings = TypeBindings::new(); // Shouldn't need to unify envs, they should always be equal since they're both free functions if let ( - Type::Function(params_a, ret_a, _env_a, unconstrained_a), - Type::Function(params_b, ret_b, _env_b, unconstrained_b), + Type::Function(params_a, ret_a, _env_a, _unconstrained_a), + Type::Function(params_b, ret_b, _env_b, _unconstrained_b), ) = (expected, actual) { - // TODO(ary): check unconstrained + // TODO: we don't yet allow marking an trait function or a trait impl function as unconstrained, + // so both values will always be false here. Once we support that, we should check that both + // match (adding a test for it). if params_a.len() == params_b.len() { for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() { From d2469dab055a9a55586363cb22577b6b589a00f1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 11:13:42 -0300 Subject: [PATCH 22/67] Don't parse `unconstrained fn(...) -> ...` yet --- compiler/noirc_frontend/src/elaborator/types.rs | 3 +-- compiler/noirc_frontend/src/parser/parser/types.rs | 3 --- tooling/nargo_fmt/src/rewrite/typ.rs | 4 +--- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 414ac104eb2..8769251067c 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -754,9 +754,8 @@ impl<'context> Elaborator<'context> { let ret = self.interner.next_type_variable(); let args = vecmap(args, |(arg, _, _)| arg); let env_type = self.interner.next_type_variable(); - let unconstrained = false; // TODO(ary): check unconstrained let expected = - Type::Function(args, Box::new(ret.clone()), Box::new(env_type), unconstrained); + Type::Function(args, Box::new(ret.clone()), Box::new(env_type), false); if let Err(error) = binding.try_bind(expected, span) { self.push_err(error); diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs index b6b1c353cb3..6a3a2afec00 100644 --- a/compiler/noirc_frontend/src/parser/parser/types.rs +++ b/compiler/noirc_frontend/src/parser/parser/types.rs @@ -269,9 +269,6 @@ where t.unwrap_or_else(|| UnresolvedTypeData::Unit.with_span(Span::empty(span.end()))) }); - // TODO(ary): allow "unconstrained" keyword here - let unconstrained = false; - keyword(Keyword::Fn) .ignore_then(env) .then(args) diff --git a/tooling/nargo_fmt/src/rewrite/typ.rs b/tooling/nargo_fmt/src/rewrite/typ.rs index e84fd453913..21d800bb79f 100644 --- a/tooling/nargo_fmt/src/rewrite/typ.rs +++ b/tooling/nargo_fmt/src/rewrite/typ.rs @@ -37,9 +37,7 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) format!("({types})") } } - UnresolvedTypeData::Function(args, return_type, env, unconstrained) => { - // TODO(ary): format unconstrained here - + UnresolvedTypeData::Function(args, return_type, env, _unconstrained) => { let env = if span_is_empty(env.span.unwrap()) { "".into() } else { From b9664e48370c0b3eac197bf33ecf7bfa06564bd0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 11:16:19 -0300 Subject: [PATCH 23/67] clippy --- compiler/noirc_frontend/src/ast/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index 661c52333f2..45ba9a074e2 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -209,7 +209,7 @@ impl std::fmt::Display for UnresolvedTypeData { FormatString(len, elements) => write!(f, "fmt<{len}, {elements}"), Function(args, ret, env, unconstrained) => { if *unconstrained { - write!(f, "unconstrained ")? + write!(f, "unconstrained ")?; } let args = vecmap(args, ToString::to_string).join(", "); From 995ccb305da9df65f9ffafa99abf2130e6c44acd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 11:24:39 -0300 Subject: [PATCH 24/67] Wrap `crate::field::bn254::decompose_hint` in unsafe --- noir_stdlib/src/hash/mod.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index 40d50abc9e5..63b208fd1d6 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -84,7 +84,7 @@ fn __derive_generators( // does not assert the limbs are 128 bits // does not assert the decomposition does not overflow the EmbeddedCurveScalar fn from_field_unsafe(scalar: Field) -> EmbeddedCurveScalar { - let (xlo, xhi) = crate::field::bn254::decompose_hint(scalar); + let (xlo, xhi) = unsafe { crate::field::bn254::decompose_hint(scalar) }; // Check that the decomposition is correct assert_eq(scalar, xlo + crate::field::bn254::TWO_POW_128 * xhi); EmbeddedCurveScalar { lo: xlo, hi: xhi } From d251fb17bd8c67fcd9fc3bd0cc865a27b947d5b9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 14:41:54 -0300 Subject: [PATCH 25/67] Add a test to show that unconstrained and regular fn can't be put in var --- compiler/noirc_frontend/src/tests.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 1805d1badd0..7e889dede99 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2197,6 +2197,31 @@ fn cannot_pass_unconstrained_function_to_regular_function() { assert_eq!(expected_typ, "fn() -> ()"); assert_eq!(expr_typ, "unconstrained fn() -> ()"); } else { - panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); + panic!("Expected a type mismatch error, got {:?}", errors[0].0); + }; +} + +#[test] +fn cannot_assign_unconstrained_and_regular_fn_to_variable() { + let src = r#" + fn main() { + let _func = if true { foo } else { bar }; + } + + fn foo() {} + unconstrained fn bar() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::Context { err, .. }) = &errors[0].0 else { + panic!("Expected a context error, got {:?}", errors[0].0); + }; + + if let TypeCheckError::TypeMismatch { expected_typ, expr_typ, .. } = err.as_ref() { + assert_eq!(expected_typ, "fn() -> ()"); + assert_eq!(expr_typ, "unconstrained fn() -> ()"); + } else { + panic!("Expected a type mismatch error, got {:?}", errors[0].0); }; } From 2910cc53196e087e773be603dc9e1c085c5486b0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 15:17:54 -0300 Subject: [PATCH 26/67] Parse `unconstrained fn` type and special rule for function call arg vs. param --- .../noirc_frontend/src/elaborator/types.rs | 34 ++++++++++++++--- .../noirc_frontend/src/parser/parser/types.rs | 12 ++++-- compiler/noirc_frontend/src/tests.rs | 37 +++++++++++++++++++ 3 files changed, 74 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 8769251067c..0a91875d41f 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -727,11 +727,35 @@ impl<'context> Elaborator<'context> { } for (param, (arg, _, arg_span)) in fn_params.iter().zip(callsite_args) { - self.unify(arg, param, || TypeCheckError::TypeMismatch { - expected_typ: param.to_string(), - expr_typ: arg.to_string(), - expr_span: *arg_span, - }); + let unify = |self_type: &mut Elaborator, actual: &Type, expected: &Type| { + self_type.unify(actual, expected, || TypeCheckError::TypeMismatch { + expected_typ: expected.to_string(), + expr_typ: actual.to_string(), + expr_span: *arg_span, + }); + }; + + // If arg and param are functions, they are incompatible if trying to pass an unconstrained + // function arg to a regular function param (the other way around is fine). + if let ( + Type::Function(params, ret, env, unconstrained_arg), + Type::Function(_, _, _, unconstrained_param), + ) = (arg.follow_bindings(), param.follow_bindings()) + { + if !unconstrained_param && unconstrained_arg { + self.push_err(TypeCheckError::TypeMismatch { + expected_typ: param.to_string(), + expr_typ: arg.to_string(), + expr_span: *arg_span, + }); + } + + // Cast arg's unconstrained status to that of param, so it won't produce an error + let arg = Type::Function(params, ret, env, unconstrained_param); + unify(self, &arg, param); + } else { + unify(self, &arg, param); + }; } fn_ret.clone() diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs index 6a3a2afec00..441c80b1ff5 100644 --- a/compiler/noirc_frontend/src/parser/parser/types.rs +++ b/compiler/noirc_frontend/src/parser/parser/types.rs @@ -269,13 +269,17 @@ where t.unwrap_or_else(|| UnresolvedTypeData::Unit.with_span(Span::empty(span.end()))) }); - keyword(Keyword::Fn) - .ignore_then(env) + keyword(Keyword::Unconstrained) + .or_not() + .then(keyword(Keyword::Fn)) + .map(|(unconstrained_token, _fn_token)| unconstrained_token.is_some()) + .then(env) .then(args) .then_ignore(just(Token::Arrow)) .then(type_parser) - .map_with_span(|((env, args), ret), span| { - UnresolvedTypeData::Function(args, Box::new(ret), Box::new(env), false).with_span(span) + .map_with_span(|(((unconstrained, env), args), ret), span| { + UnresolvedTypeData::Function(args, Box::new(ret), Box::new(env), unconstrained) + .with_span(span) }) } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 7e889dede99..7ef7131871d 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2225,3 +2225,40 @@ fn cannot_assign_unconstrained_and_regular_fn_to_variable() { panic!("Expected a type mismatch error, got {:?}", errors[0].0); }; } + +#[test] +fn can_pass_regular_function_to_unconstrained_function() { + let src = r#" + fn main() { + let func = foo; + expect_unconstrained(func); + } + + fn foo() {} + + fn expect_unconstrained(_func: unconstrained fn() -> ()) { + } + "#; + assert_no_errors(src); +} + +#[test] +fn cannot_pass_unconstrained_function_to_constrained_function() { + let src = r#" + fn main() { + let func = foo; + expect_regular(func); + } + + unconstrained fn foo() {} + + fn expect_regular(_func: fn() -> ()) { + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0].0 else { + panic!("Expected a type mismatch error, got {:?}", errors[0].0); + }; +} From 35b3cfab5eecf0465186f8fb5842a7e5abcde37e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 16:21:49 -0300 Subject: [PATCH 27/67] Fixes in test program --- .../noir_test_success/regression_4561/src/main.nr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test_programs/noir_test_success/regression_4561/src/main.nr b/test_programs/noir_test_success/regression_4561/src/main.nr index ad40941ff51..6a8b4bd61fb 100644 --- a/test_programs/noir_test_success/regression_4561/src/main.nr +++ b/test_programs/noir_test_success/regression_4561/src/main.nr @@ -12,7 +12,7 @@ unconstrained fn simple_nested_return_unconstrained() -> TReturn { } #[test] -fn test_simple_nested_return() { +unconstrained fn test_simple_nested_return() { OracleMock::mock("simple_nested_return").returns([1, 2, 3, 4, 5, 6]); assert_eq(simple_nested_return_unconstrained(), [[1, 2, 3], [4, 5, 6]]); } @@ -25,7 +25,7 @@ unconstrained fn nested_with_fields_return_unconstrained() -> (Field, TReturn, F } #[test] -fn test_nested_with_fields_return() { +unconstrained fn test_nested_with_fields_return() { OracleMock::mock("nested_with_fields_return").returns((0, [1, 2, 3, 4, 5, 6], 7)); assert_eq(nested_with_fields_return_unconstrained(), (0, [[1, 2, 3], [4, 5, 6]], 7)); } @@ -38,7 +38,7 @@ unconstrained fn two_nested_return_unconstrained() -> (Field, TReturn, Field, TR } #[test] -fn two_nested_return() { +unconstrained fn two_nested_return() { OracleMock::mock("two_nested_return").returns((0, [1, 2, 3, 4, 5, 6], 7, [1, 2, 3, 4, 5, 6])); assert_eq(two_nested_return_unconstrained(), (0, [[1, 2, 3], [4, 5, 6]], 7, [[1, 2, 3], [4, 5, 6]])); } @@ -57,7 +57,7 @@ struct TestTypeFoo { } #[test] -fn complexe_struct_return() { +unconstrained fn complexe_struct_return() { OracleMock::mock("foo_return").returns( ( 0, [1, 2, 3, 4, 5, 6], 7, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24], [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12], [1, 2, 3, 4, 5, 6] From 75270203f255f88c4f4b279fe0dfc9d04eca7129 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 16:21:59 -0300 Subject: [PATCH 28/67] Format `unconstrained fn` as type --- tooling/nargo_fmt/src/rewrite/typ.rs | 6 ++++-- tooling/nargo_fmt/tests/expected/fn.nr | 2 ++ tooling/nargo_fmt/tests/input/fn.nr | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_fmt/src/rewrite/typ.rs b/tooling/nargo_fmt/src/rewrite/typ.rs index 21d800bb79f..6ec35564f88 100644 --- a/tooling/nargo_fmt/src/rewrite/typ.rs +++ b/tooling/nargo_fmt/src/rewrite/typ.rs @@ -37,7 +37,9 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) format!("({types})") } } - UnresolvedTypeData::Function(args, return_type, env, _unconstrained) => { + UnresolvedTypeData::Function(args, return_type, env, unconstrained) => { + let unconstrained = if unconstrained { "unconstrained " } else { "" }; + let env = if span_is_empty(env.span.unwrap()) { "".into() } else { @@ -53,7 +55,7 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) let return_type = rewrite(visitor, _shape, *return_type); - format!("fn{env}({args}) -> {return_type}") + format!("{unconstrained}fn{env}({args}) -> {return_type}") } UnresolvedTypeData::Resolved(_) => { unreachable!("Unexpected macro expansion of a type in nargo fmt input") diff --git a/tooling/nargo_fmt/tests/expected/fn.nr b/tooling/nargo_fmt/tests/expected/fn.nr index 4dde9a1b3ec..73248d0c04f 100644 --- a/tooling/nargo_fmt/tests/expected/fn.nr +++ b/tooling/nargo_fmt/tests/expected/fn.nr @@ -69,3 +69,5 @@ fn id_two(x: [Field; I]) -> [Field; I] {} fn whitespace_before_generics(foo: T) {} fn more_whitespace_before_generics(foo: T) {} + +fn with_unconstrained(x: unconstrained fn() -> ()) {} diff --git a/tooling/nargo_fmt/tests/input/fn.nr b/tooling/nargo_fmt/tests/input/fn.nr index 16ed95a540d..8db6022808f 100644 --- a/tooling/nargo_fmt/tests/input/fn.nr +++ b/tooling/nargo_fmt/tests/input/fn.nr @@ -54,3 +54,5 @@ fn whitespace_before_generics < T > (foo: T) {} fn more_whitespace_before_generics < T > (foo: T) {} + +fn with_unconstrained(x: unconstrained fn() -> ()) {} From 171288f55bf4b955b3a2fcb5670cb49219cf471f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 17:17:36 -0300 Subject: [PATCH 29/67] nargo fmt --- noir_stdlib/src/field/bn254.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index 96868fb1184..c1c7a41b963 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -82,7 +82,7 @@ pub fn decompose(x: Field) -> (Field, Field) { if is_unconstrained() { compute_decomposition(x) } else { - unsafe { + unsafe { // Take hints of the decomposition let (xlo, xhi) = decompose_hint(x); @@ -122,7 +122,7 @@ pub fn gt(a: Field, b: Field) -> bool { compute_lt(b, a, 32) } else if a == b { false - } else { + } else { // Take a hint of the comparison and verify it unsafe { if lt_32_hint(a, b) { From e8ae6e8bfe5c5012748b57d809cf72fd64ab5bb8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 17 Jul 2024 17:18:07 -0300 Subject: [PATCH 30/67] clippy --- compiler/noirc_frontend/src/elaborator/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 0a91875d41f..f861ee65e28 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -754,7 +754,7 @@ impl<'context> Elaborator<'context> { let arg = Type::Function(params, ret, env, unconstrained_param); unify(self, &arg, param); } else { - unify(self, &arg, param); + unify(self, arg, param); }; } From d66228c06955191c3eeb883cd590ea95c9f71401 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 09:16:09 -0300 Subject: [PATCH 31/67] `fn` to `unconstrained fn` is now a type coercion, and fn calls coerce args --- .../noirc_frontend/src/elaborator/types.rs | 36 +++----------- compiler/noirc_frontend/src/hir_def/types.rs | 48 +++++++++++++++++++ compiler/noirc_frontend/src/tests.rs | 28 +++++++++++ 3 files changed, 82 insertions(+), 30 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index f861ee65e28..f63de82e9ff 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -726,36 +726,12 @@ impl<'context> Elaborator<'context> { return Type::Error; } - for (param, (arg, _, arg_span)) in fn_params.iter().zip(callsite_args) { - let unify = |self_type: &mut Elaborator, actual: &Type, expected: &Type| { - self_type.unify(actual, expected, || TypeCheckError::TypeMismatch { - expected_typ: expected.to_string(), - expr_typ: actual.to_string(), - expr_span: *arg_span, - }); - }; - - // If arg and param are functions, they are incompatible if trying to pass an unconstrained - // function arg to a regular function param (the other way around is fine). - if let ( - Type::Function(params, ret, env, unconstrained_arg), - Type::Function(_, _, _, unconstrained_param), - ) = (arg.follow_bindings(), param.follow_bindings()) - { - if !unconstrained_param && unconstrained_arg { - self.push_err(TypeCheckError::TypeMismatch { - expected_typ: param.to_string(), - expr_typ: arg.to_string(), - expr_span: *arg_span, - }); - } - - // Cast arg's unconstrained status to that of param, so it won't produce an error - let arg = Type::Function(params, ret, env, unconstrained_param); - unify(self, &arg, param); - } else { - unify(self, arg, param); - }; + for (param, (arg, arg_expr_id, arg_span)) in fn_params.iter().zip(callsite_args) { + self.unify_with_coercions(arg, param, *arg_expr_id, || TypeCheckError::TypeMismatch { + expected_typ: param.to_string(), + expr_typ: arg.to_string(), + expr_span: *arg_span, + }); } fn_ret.clone() diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d09e2a2f20a..94d9c45d337 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1591,6 +1591,13 @@ impl Type { errors: &mut Vec, make_error: impl FnOnce() -> TypeCheckError, ) { + // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` + let Some(make_error) = self.try_fn_to_unconstrained_fn_coercion( + expected, expression, interner, errors, make_error, + ) else { + return; + }; + let mut bindings = TypeBindings::new(); if let Err(UnificationError) = self.try_unify(expected, &mut bindings) { @@ -1602,6 +1609,47 @@ impl Type { } } + // If `self` and `expected` are function types, tries to coerce `self` to `expected`. + // Returns None if this produced an error, otherwise returns Some(make_error) meaning + // that regular unification must still be done (and it could produce an error). + pub fn try_fn_to_unconstrained_fn_coercion( + &self, + expected: &Type, + expression: ExprId, + interner: &mut NodeInterner, + errors: &mut Vec, + make_error: impl FnOnce() -> TypeCheckError, + ) -> Option TypeCheckError> { + // If `self` and `expected` are function types, `self` can be coerced to `expected` + // if `self` is unconstrained and `expected` is not. The other way around is an error, though. + let ( + Type::Function(params, ret, env, unconstrained_self), + Type::Function(_, _, _, unconstrained_expected), + ) = (self.follow_bindings(), expected.follow_bindings()) + else { + // No coercion needed + return Some(make_error); + }; + + // unconstrained status matches: no error + if unconstrained_self == unconstrained_expected { + return Some(make_error); + } + + // unconstrained mismatch: produce an error + if unconstrained_self && !unconstrained_expected { + errors.push(make_error()); + return None; + } + + // Cast self unconstrained status to that of expected, so it won't produce an error + let coerced_self = Type::Function(params, ret, env, unconstrained_expected); + coerced_self.unify_with_coercions(expected, expression, interner, errors, make_error); + + // Return None: an error might have been produced in the previous call + None + } + /// Try to apply the array to slice coercion to this given type pair and expression. /// If self can be converted to target this way, do so and return true to indicate success. fn try_array_to_slice_coercion( diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 7ef7131871d..27ec6e896b6 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2262,3 +2262,31 @@ fn cannot_pass_unconstrained_function_to_constrained_function() { panic!("Expected a type mismatch error, got {:?}", errors[0].0); }; } + +#[test] +fn can_assign_regular_function_to_unconstrained_function_in_explicitly_typed_var() { + let src = r#" + fn main() { + let _func: unconstrained fn() -> () = foo; + } + + fn foo() {} + "#; + assert_no_errors(src); +} + +#[test] +fn can_assign_regular_function_to_unconstrained_function_in_struct_member() { + let src = r#" + fn main() { + let _ = Foo { func: foo }; + } + + fn foo() {} + + struct Foo { + func: unconstrained fn() -> (), + } + "#; + assert_no_errors(src); +} From 1e4973fd3a300ede15ad259bef180bf5d8678514 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 09:23:17 -0300 Subject: [PATCH 32/67] Fix failing program --- .../execution_success/brillig_fns_as_values/src/main.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/brillig_fns_as_values/src/main.nr b/test_programs/execution_success/brillig_fns_as_values/src/main.nr index 8bedef5b818..55b9d307905 100644 --- a/test_programs/execution_success/brillig_fns_as_values/src/main.nr +++ b/test_programs/execution_success/brillig_fns_as_values/src/main.nr @@ -1,5 +1,5 @@ struct MyStruct { - operation: fn (u32) -> u32, + operation: unconstrained fn (u32) -> u32, } fn main(x: u32) { @@ -14,7 +14,7 @@ fn main(x: u32) { } } -unconstrained fn wrapper(func: fn(u32) -> u32, param: u32) -> u32 { +unconstrained fn wrapper(func: unconstrained fn(u32) -> u32, param: u32) -> u32 { func(param) } From 1247917400700b3136ee4f091423cb4a87f6b978 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 09:33:29 -0300 Subject: [PATCH 33/67] Unify if branches --- compiler/noirc_frontend/src/hir_def/types.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 94d9c45d337..96ea373e7af 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1522,9 +1522,7 @@ impl Type { Function(params_a, ret_a, env_a, unconstrained_a), Function(params_b, ret_b, env_b, unconstrained_b), ) => { - if *unconstrained_a != *unconstrained_b { - Err(UnificationError) - } else if params_a.len() == params_b.len() { + if unconstrained_a == unconstrained_b && params_a.len() == params_b.len() { for (a, b) in params_a.iter().zip(params_b.iter()) { a.try_unify(b, bindings)?; } From 211d12000f3d8cf68cb1ee4d3c9fc592c0a0a530 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 09:53:46 -0300 Subject: [PATCH 34/67] Only coerce Array to Slice if `as_slice` builtin is defined --- compiler/noirc_frontend/src/hir_def/types.rs | 29 ++++++++++++-------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 96ea373e7af..d7498008ccd 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1660,13 +1660,23 @@ impl Type { let target = target.follow_bindings(); if let (Type::Array(_size, element1), Type::Slice(element2)) = (&this, &target) { - // Still have to ensure the element types match. - // Don't need to issue an error here if not, it will be done in unify_with_coercions - let mut bindings = TypeBindings::new(); - if element1.try_unify(element2, &mut bindings).is_ok() { - convert_array_expression_to_slice(expression, this, target, interner); - Self::apply_type_bindings(bindings); - return true; + // We can only do the coercion if the `as_slice` method exists. + // This is usually true, but some tests don't have access to the standard library. + if let Some(as_slice_method) = interner.lookup_primitive_method(&this, "as_slice") { + // Still have to ensure the element types match. + // Don't need to issue an error here if not, it will be done in unify_with_coercions + let mut bindings = TypeBindings::new(); + if element1.try_unify(element2, &mut bindings).is_ok() { + convert_array_expression_to_slice( + expression, + this, + target, + as_slice_method, + interner, + ); + Self::apply_type_bindings(bindings); + return true; + } } } false @@ -2153,12 +2163,9 @@ fn convert_array_expression_to_slice( expression: ExprId, array_type: Type, target_type: Type, + as_slice_method: crate::node_interner::FuncId, interner: &mut NodeInterner, ) { - let as_slice_method = interner - .lookup_primitive_method(&array_type, "as_slice") - .expect("Expected 'as_slice' method to be present in Noir's stdlib"); - let as_slice_id = interner.function_definition_id(as_slice_method); let location = interner.expr_location(&expression); let as_slice = HirExpression::Ident(HirIdent::non_trait_method(as_slice_id, location), None); From b7c1d85204ca8ed48f01aea0fc551b0b4d57af1b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 10:09:44 -0300 Subject: [PATCH 35/67] One more unsafe --- .../check_uncostrained_regression/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr b/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr index e93e068f432..8cb6c6f662e 100644 --- a/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr @@ -15,7 +15,7 @@ unconstrained fn convert(trigger: Trigger) -> ResultType { } impl Trigger { fn execute(self) -> ResultType { - let result = convert(self); + let result = unsafe { convert(self) }; assert(result.a == self.x + 1); assert(result.b == self.y - 1 + self.z[2]); assert(result.c[1] == 0); From 7bdf3bfe753bf7cffe5b1bcb3bdb5cc62c2fd57d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 10:28:22 -0300 Subject: [PATCH 36/67] nargo fmt --- noir_stdlib/src/uint128.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index c341000bd36..6359737b9d6 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -465,7 +465,7 @@ mod tests { let (c,d) = b.unconstrained_div(a); assert_eq((c, d), (U128::zero(), b)); } - + // Dividing by zero returns 0,0 let a = U128::from_u64s_le(0x1, 0x0); let b = U128::zero(); From 7081c856ec45378a04e3fcc9241a0524a76fb86a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 12:45:56 -0300 Subject: [PATCH 37/67] Format --- test_programs/execution_success/aes128_encrypt/src/main.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/aes128_encrypt/src/main.nr b/test_programs/execution_success/aes128_encrypt/src/main.nr index d86e22cf169..9cf07841b9e 100644 --- a/test_programs/execution_success/aes128_encrypt/src/main.nr +++ b/test_programs/execution_success/aes128_encrypt/src/main.nr @@ -31,7 +31,7 @@ unconstrained fn cipher(plaintext: [u8; 12], iv: [u8; 16], key: [u8; 16]) -> [u8 fn main(inputs: str<12>, iv: str<16>, key: str<16>, output: str<32>) { let result = std::aes128::aes128_encrypt(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()); - + let output_bytes: [u8; 16] = unsafe { let output_bytes: [u8; 16] = decode_hex(output); for i in 0..16 { @@ -39,7 +39,7 @@ fn main(inputs: str<12>, iv: str<16>, key: str<16>, output: str<32>) { } output_bytes }; - + unsafe { let unconstrained_result = cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()); for i in 0..16 { From c065b15e94c86cb601031c201405ffa103c07b17 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 12:50:46 -0300 Subject: [PATCH 38/67] Mention `unsafe` in docs --- docs/docs/noir/concepts/unconstrained.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/docs/noir/concepts/unconstrained.md b/docs/docs/noir/concepts/unconstrained.md index 96f824c5e42..b5221b8d2dd 100644 --- a/docs/docs/noir/concepts/unconstrained.md +++ b/docs/docs/noir/concepts/unconstrained.md @@ -62,11 +62,13 @@ Those are some nice savings already but we can do better. This code is all const It turns out that truncating a u72 into a u8 is hard to do inside a snark, each time we do as u8 we lay down 4 ACIR opcodes which get converted into multiple gates. It's actually much easier to calculate num from out than the other way around. All we need to do is multiply each element of out by a constant and add them all together, both relatively easy operations inside a snark. -We can then run u72_to_u8 as unconstrained brillig code in order to calculate out, then use that result in our constrained function and assert that if we were to do the reverse calculation we'd get back num. This looks a little like the below: +We can then run `u72_to_u8` as unconstrained brillig code in order to calculate out, then use that result in our constrained function and assert that if we were to do the reverse calculation we'd get back num. This looks a little like the below: ```rust fn main(num: u72) -> pub [u8; 8] { - let out = u72_to_u8(num); + let out = unsafe { + u72_to_u8(num) + }; let mut reconstructed_num: u72 = 0; for i in 0..8 { @@ -92,6 +94,9 @@ Backend circuit size: 2902 This ends up taking off another ~250 gates from our circuit! We've ended up with more ACIR opcodes than before but they're easier for the backend to prove (resulting in fewer gates). +Note that in order to invoke unconstrained functions we need to wrap them in an `unsafe` block, +to make it clear that the call is unconstrained. + Generally we want to use brillig whenever there's something that's easy to verify but hard to compute within the circuit. For example, if you wanted to calculate a square root of a number it'll be a much better idea to calculate this in brillig and then assert that if you square the result you get back your number. ## Break and Continue From 84c246c6fb14ab1d83701dc1339beb0d3afc5320 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 15:24:44 -0300 Subject: [PATCH 39/67] Apply suggestions from code review Co-authored-by: jfecher --- compiler/noirc_frontend/src/elaborator/traits.rs | 2 +- compiler/noirc_frontend/src/hir/type_check/errors.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 10 ++-------- compiler/noirc_frontend/src/lexer/token.rs | 1 - compiler/noirc_frontend/src/tests.rs | 2 +- noir_stdlib/src/array.nr | 3 +-- 6 files changed, 6 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index e4b0bc98106..d9268a33b9d 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -329,7 +329,7 @@ fn check_function_type_matches_expected_type( Type::Function(params_b, ret_b, _env_b, _unconstrained_b), ) = (expected, actual) { - // TODO: we don't yet allow marking an trait function or a trait impl function as unconstrained, + // TODO: we don't yet allow marking a trait function or a trait impl function as unconstrained, // so both values will always be false here. Once we support that, we should check that both // match (adding a test for it). diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 88c37c7fb9d..d88e98a9898 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -136,7 +136,7 @@ pub enum TypeCheckError { UnconstrainedReferenceToConstrained { span: Span }, #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] UnconstrainedSliceReturnToConstrained { span: Span }, - #[error("Call to unconstrained function is unsafe and requires unconstrained function or unsafe block")] + #[error("Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block")] Unsafe { span: Span }, #[error("Slices must have constant length")] NonConstantSliceLength { span: Span }, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d7498008ccd..151b59af3a4 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1662,18 +1662,12 @@ impl Type { if let (Type::Array(_size, element1), Type::Slice(element2)) = (&this, &target) { // We can only do the coercion if the `as_slice` method exists. // This is usually true, but some tests don't have access to the standard library. - if let Some(as_slice_method) = interner.lookup_primitive_method(&this, "as_slice") { + if let Some(as_slice) = interner.lookup_primitive_method(&this, "as_slice") { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); if element1.try_unify(element2, &mut bindings).is_ok() { - convert_array_expression_to_slice( - expression, - this, - target, - as_slice_method, - interner, - ); + convert_array_expression_to_slice(expression, this, target, as_slice, interner); Self::apply_type_bindings(bindings); return true; } diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 88b6d252afa..e6ff497dd13 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -1027,7 +1027,6 @@ impl Keyword { "unchecked" => Keyword::Unchecked, "unconstrained" => Keyword::Unconstrained, "unsafe" => Keyword::Unsafe, - "use" => Keyword::Use, "where" => Keyword::Where, "while" => Keyword::While, diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 27ec6e896b6..803a7da7d04 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2155,7 +2155,7 @@ fn cannot_call_unconstrained_function_outside_of_unsafe() { } #[test] -fn cannot_call_unconstrained_captured_function_outside_of_unsafe() { +fn cannot_call_unconstrained_first_class_function_outside_of_unsafe() { let src = r#" fn main() { let func = foo; diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index be1daa283bb..c735d6f3cb3 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -12,10 +12,9 @@ impl [T; N] { pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { let sorted_index = unsafe { - // Safety: This is safe we we enforce proper ordering using these indices + // Safety: These indices are asserted to be the sorted element indices via `find_index` let sorted_index: [u32; N] = self.get_sorting_index(ordering); - // Ensure the indexes are correct for i in 0..N { let pos = find_index(sorted_index, i); assert(sorted_index[pos] == i); From 7b334a5682f45e6041ff1fc7f68f85bbf040b02e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 18 Jul 2024 16:22:36 -0300 Subject: [PATCH 40/67] Introduce an Unsafe block expression instead of a bool in BlockExpression --- aztec_macros/src/transforms/functions.rs | 6 +-- aztec_macros/src/transforms/storage.rs | 2 +- compiler/noirc_frontend/src/ast/expression.rs | 7 ++- compiler/noirc_frontend/src/ast/statement.rs | 2 - compiler/noirc_frontend/src/debug/mod.rs | 3 -- .../src/elaborator/expressions.rs | 24 ++++++---- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- .../noirc_frontend/src/elaborator/traits.rs | 2 +- .../src/hir/comptime/hir_to_display_ast.rs | 5 +- .../src/hir/comptime/interpreter.rs | 1 + .../noirc_frontend/src/hir/comptime/scan.rs | 1 + compiler/noirc_frontend/src/hir_def/expr.rs | 2 +- .../noirc_frontend/src/hir_def/function.rs | 3 +- .../src/monomorphization/mod.rs | 1 + compiler/noirc_frontend/src/parser/parser.rs | 47 ++++++++++--------- tooling/lsp/src/requests/inlay_hint.rs | 3 ++ tooling/nargo_fmt/src/rewrite/expr.rs | 3 ++ tooling/nargo_fmt/src/visitor/expr.rs | 6 --- 18 files changed, 63 insertions(+), 57 deletions(-) diff --git a/aztec_macros/src/transforms/functions.rs b/aztec_macros/src/transforms/functions.rs index 04f23be24e2..4d8b6ef7cdf 100644 --- a/aztec_macros/src/transforms/functions.rs +++ b/aztec_macros/src/transforms/functions.rs @@ -749,10 +749,8 @@ fn create_loop_over(var: Expression, loop_body: Vec) -> Statement { // What will be looped over // - `hasher.add({ident}[i] as Field)` - let for_loop_block = expression(ExpressionKind::Block(BlockExpression { - is_unsafe: false, - statements: loop_body, - })); + let for_loop_block = + expression(ExpressionKind::Block(BlockExpression { statements: loop_body })); // `for i in 0..{ident}.len()` make_statement(StatementKind::For(ForLoopStatement { diff --git a/aztec_macros/src/transforms/storage.rs b/aztec_macros/src/transforms/storage.rs index e883dabfab4..c302dd87aa5 100644 --- a/aztec_macros/src/transforms/storage.rs +++ b/aztec_macros/src/transforms/storage.rs @@ -228,7 +228,7 @@ pub fn generate_storage_implementation( &ident("init"), &vec![], &[(ident("context"), generic_context_type.clone())], - &BlockExpression { is_unsafe: false, statements: vec![storage_constructor_statement] }, + &BlockExpression { statements: vec![storage_constructor_statement] }, &[], &return_type(chained_path!("Self")), )); diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index d338f76ab1e..023b76073ec 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -38,6 +38,7 @@ pub enum ExpressionKind { Quote(Tokens), Unquote(Box), Comptime(BlockExpression, Span), + Unsafe(BlockExpression, Span), // This variant is only emitted when inlining the result of comptime // code. It is used to translate function values back into the AST while @@ -270,7 +271,6 @@ impl Expression { // as a sequence of { if, tuple } rather than a function call. This behavior matches rust. let kind = if matches!(&lhs.kind, ExpressionKind::If(..)) { ExpressionKind::Block(BlockExpression { - is_unsafe: false, statements: vec![ Statement { kind: StatementKind::Expression(lhs), span }, Statement { @@ -548,7 +548,6 @@ pub struct IndexExpression { #[derive(Debug, PartialEq, Eq, Clone)] pub struct BlockExpression { - pub is_unsafe: bool, pub statements: Vec, } @@ -602,6 +601,7 @@ impl Display for ExpressionKind { Lambda(lambda) => lambda.fmt(f), Parenthesized(sub_expr) => write!(f, "({sub_expr})"), Comptime(block, _) => write!(f, "comptime {block}"), + Unsafe(block, _) => write!(f, "unsafe {block}"), Error => write!(f, "Error"), Resolved(_) => write!(f, "?Resolved"), Unquote(expr) => write!(f, "$({expr})"), @@ -652,8 +652,7 @@ impl Display for Literal { impl Display for BlockExpression { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let safety = if self.is_unsafe { "unsafe " } else { "" }; - writeln!(f, "{safety}{{")?; + writeln!(f, "{{")?; for statement in &self.statements { let statement = statement.kind.to_string(); for line in statement.lines() { diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index d37fc9b87e6..7d86ae95742 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -651,7 +651,6 @@ impl ForRange { let block_span = block.span; let new_block = BlockExpression { - is_unsafe: false, statements: vec![ let_elem, Statement { kind: StatementKind::Expression(block), span: block_span }, @@ -669,7 +668,6 @@ impl ForRange { }; let block = ExpressionKind::Block(BlockExpression { - is_unsafe: false, statements: vec![let_array, for_loop], }); Statement { diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index af9e04b6009..af2810626ce 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -250,7 +250,6 @@ impl DebugInstrumenter { comptime: false, expression: ast::Expression { kind: ast::ExpressionKind::Block(ast::BlockExpression { - is_unsafe: true, statements: block_stmts, }), span: let_stmt.expression.span, @@ -343,7 +342,6 @@ impl DebugInstrumenter { lvalue: assign_stmt.lvalue.clone(), expression: ast::Expression { kind: ast::ExpressionKind::Block(ast::BlockExpression { - is_unsafe: false, statements: vec![ ast::Statement { kind: let_kind, span: expression_span }, new_assign_stmt, @@ -431,7 +429,6 @@ impl DebugInstrumenter { self.walk_expr(&mut for_stmt.block); for_stmt.block = ast::Expression { kind: ast::ExpressionKind::Block(ast::BlockExpression { - is_unsafe: false, statements: vec![ set_stmt, ast::Statement { diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 43d9ae4570c..3b5c1e66d38 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -60,6 +60,9 @@ impl<'context> Elaborator<'context> { ExpressionKind::Comptime(comptime, _) => { return self.elaborate_comptime_block(comptime, expr.span) } + ExpressionKind::Unsafe(block_expression, _) => { + self.elaborate_unsafe_block(block_expression) + } ExpressionKind::Resolved(id) => return (id, self.interner.id_type(id)), ExpressionKind::Error => (HirExpression::Error, Type::Error), ExpressionKind::Unquote(_) => { @@ -83,13 +86,6 @@ impl<'context> Elaborator<'context> { let mut block_type = Type::Unit; let mut statements = Vec::with_capacity(block.statements.len()); - // Before entering the block we cache the old value of `in_unsafe_block` so it can be restored. - let old_in_unsafe_block = self.in_unsafe_block; - - // If we're already in an unsafe block then entering a new block should preserve this even if - // the inner block isn't marked as unsafe. - self.in_unsafe_block |= block.is_unsafe; - for (i, statement) in block.statements.into_iter().enumerate() { let (id, stmt_type) = self.elaborate_statement(statement); statements.push(id); @@ -109,11 +105,21 @@ impl<'context> Elaborator<'context> { } } + self.pop_scope(); + (HirBlockExpression { statements }, block_type) + } + + fn elaborate_unsafe_block(&mut self, block: BlockExpression) -> (HirExpression, Type) { + // Before entering the block we cache the old value of `in_unsafe_block` so it can be restored. + let old_in_unsafe_block = self.in_unsafe_block; + self.in_unsafe_block = true; + + let (hir_block_expression, typ) = self.elaborate_block_expression(block); + // Finally, we restore the original value of `self.in_unsafe_block`. self.in_unsafe_block = old_in_unsafe_block; - self.pop_scope(); - (HirBlockExpression { is_unsafe: block.is_unsafe, statements }, block_type) + (HirExpression::Unsafe(hir_block_expression), typ) } fn elaborate_literal(&mut self, literal: Literal, span: Span) -> (HirExpression, Type) { diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 55c751db45a..ffc91a47fb5 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -738,7 +738,7 @@ impl<'context> Elaborator<'context> { .collect(); let statements = std::mem::take(&mut func.def.body.statements); - let body = BlockExpression { is_unsafe: false, statements }; + let body = BlockExpression { statements }; let struct_id = if let Some(Type::Struct(struct_type, _)) = &self.self_type { Some(struct_type.borrow().id) diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index d9268a33b9d..c0e57ce84be 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -196,7 +196,7 @@ impl<'context> Elaborator<'context> { typ: typ.clone(), span: name.span(), }), - body: BlockExpression { is_unsafe: false, statements: Vec::new() }, + body: BlockExpression { statements: Vec::new() }, span: name.span(), where_clause: where_clause.to_vec(), return_type: return_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 5d3c6501eb1..7fee9a7703b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -196,6 +196,9 @@ impl HirExpression { HirExpression::Comptime(block) => { ExpressionKind::Comptime(block.to_display_ast(interner), span) } + HirExpression::Unsafe(block) => { + ExpressionKind::Unsafe(block.to_display_ast(interner), span) + } HirExpression::Quote(block) => ExpressionKind::Quote(block.clone()), // A macro was evaluated here: return the quoted result @@ -423,6 +426,6 @@ impl HirBlockExpression { fn to_display_ast(&self, interner: &NodeInterner) -> BlockExpression { let statements = vecmap(self.statements.clone(), |statement| statement.to_display_ast(interner)); - BlockExpression { statements, is_unsafe: self.is_unsafe } + BlockExpression { statements } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index cbea5cd158e..ef861c8fc70 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -370,6 +370,7 @@ impl<'a> Interpreter<'a> { HirExpression::Lambda(lambda) => self.evaluate_lambda(lambda, id), HirExpression::Quote(tokens) => self.evaluate_quote(tokens, id), HirExpression::Comptime(block) => self.evaluate_block(block), + HirExpression::Unsafe(block) => self.evaluate_block(block), HirExpression::Unquote(tokens) => { // An Unquote expression being found is indicative of a macro being // expanded within another comptime fn which we don't currently support. diff --git a/compiler/noirc_frontend/src/hir/comptime/scan.rs b/compiler/noirc_frontend/src/hir/comptime/scan.rs index 2ce22ab51e3..295939ff68d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/scan.rs +++ b/compiler/noirc_frontend/src/hir/comptime/scan.rs @@ -90,6 +90,7 @@ impl<'interner> Interpreter<'interner> { self.interner.replace_expr(&expr, new_expr); Ok(()) } + HirExpression::Unsafe(block) => self.scan_block(block), HirExpression::Quote(_) => { // This error could be detected much earlier in the compiler pipeline but // it just makes sense for the comptime code to handle comptime things. diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index d7e3339c2e3..1388170fb36 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -37,6 +37,7 @@ pub enum HirExpression { Quote(Tokens), Unquote(Tokens), Comptime(HirBlockExpression), + Unsafe(HirBlockExpression), Error, } @@ -258,7 +259,6 @@ pub struct HirIndexExpression { #[derive(Debug, Clone)] pub struct HirBlockExpression { - pub is_unsafe: bool, pub statements: Vec, } diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 1835b802327..7377a2c6f9b 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -199,10 +199,9 @@ impl FuncMeta { match &mut self.function_body { FunctionBody::Unresolved(kind, block, span) => { let statements = std::mem::take(&mut block.statements); - let is_unsafe = block.is_unsafe; let (kind, span) = (*kind, *span); self.function_body = FunctionBody::Resolving; - FunctionBody::Unresolved(kind, BlockExpression { is_unsafe, statements }, span) + FunctionBody::Unresolved(kind, BlockExpression { statements }, span) } FunctionBody::Resolving => FunctionBody::Resolving, FunctionBody::Resolved => FunctionBody::Resolved, diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 4ae86bb1170..4759379bef2 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -442,6 +442,7 @@ impl<'interner> Monomorphizer<'interner> { }, HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]), HirExpression::Block(block) => self.block(block.statements)?, + HirExpression::Unsafe(block) => self.block(block.statements)?, HirExpression::Prefix(prefix) => { let rhs = self.expr(prefix.rhs)?; diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 15fcfcf593e..9a5945e076f 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -425,28 +425,22 @@ fn block_expr<'a>( fn block<'a>( statement: impl NoirParser + 'a, ) -> impl NoirParser + 'a { - use crate::token::Keyword; use Token::*; - keyword(Keyword::Unsafe) - .or_not() - .map(|unsafe_token| unsafe_token.is_some()) - .then( - statement - .recover_via(statement_recovery()) - .then(just(Semicolon).or_not().map_with_span(|s, span| (s, span))) - .map_with_span(|(kind, rest), span| (Statement { kind, span }, rest)) - .repeated() - .validate(check_statements_require_semicolon) - .delimited_by(just(LeftBrace), just(RightBrace)) - .recover_with(nested_delimiters( - LeftBrace, - RightBrace, - [(LeftParen, RightParen), (LeftBracket, RightBracket)], - |span| vec![Statement { kind: StatementKind::Error, span }], - )), - ) - .map(|(is_unsafe, statements)| BlockExpression { is_unsafe, statements }) + statement + .recover_via(statement_recovery()) + .then(just(Semicolon).or_not().map_with_span(|s, span| (s, span))) + .map_with_span(|(kind, rest), span| (Statement { kind, span }, rest)) + .repeated() + .validate(check_statements_require_semicolon) + .delimited_by(just(LeftBrace), just(RightBrace)) + .recover_with(nested_delimiters( + LeftBrace, + RightBrace, + [(LeftParen, RightParen), (LeftBracket, RightBracket)], + |span| vec![Statement { kind: StatementKind::Error, span }], + )) + .map(|statements| BlockExpression { statements }) } fn check_statements_require_semicolon( @@ -573,6 +567,15 @@ where .map(|(block, span)| ExpressionKind::Comptime(block, span)) } +fn unsafe_expr<'a, S>(statement: S) -> impl NoirParser + 'a +where + S: NoirParser + 'a, +{ + keyword(Keyword::Unsafe) + .ignore_then(spanned(block(statement))) + .map(|(block, span)| ExpressionKind::Unsafe(block, span)) +} + fn declaration<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, @@ -958,7 +961,6 @@ where // i.e. rewrite the sugared form `if cond1 {} else if cond2 {}` as `if cond1 {} else { if cond2 {} }`. let if_expression = Expression::new(kind, span); let desugared_else = BlockExpression { - is_unsafe: false, statements: vec![Statement { kind: StatementKind::Expression(if_expression), span, @@ -1096,7 +1098,8 @@ where }, lambdas::lambda(expr_parser.clone()), block(statement.clone()).map(ExpressionKind::Block), - comptime_expr(statement), + comptime_expr(statement.clone()), + unsafe_expr(statement), quote(), unquote(expr_parser.clone()), variable(), diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 4330317f625..bd3907cb858 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -258,6 +258,9 @@ impl<'a> InlayHintCollector<'a> { ExpressionKind::Comptime(block_expression, _span) => { self.collect_in_block_expression(block_expression); } + ExpressionKind::Unsafe(block_expression, _span) => { + self.collect_in_block_expression(block_expression); + } ExpressionKind::Literal(..) | ExpressionKind::Variable(..) | ExpressionKind::Quote(..) diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 015644c15cb..b6248dc4774 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -172,6 +172,9 @@ pub(crate) fn rewrite( ExpressionKind::Comptime(block, block_span) => { format!("comptime {}", rewrite_block(visitor, block, block_span)) } + ExpressionKind::Unsafe(block, block_span) => { + format!("unsafe {}", rewrite_block(visitor, block, block_span)) + } ExpressionKind::Error => unreachable!(), ExpressionKind::Resolved(_) => { unreachable!("ExpressionKind::Resolved should only emitted by the comptime interpreter") diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 201c599f413..18b962a7f85 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -112,12 +112,6 @@ impl FmtVisitor<'_> { } pub(crate) fn visit_block(&mut self, block: BlockExpression, block_span: Span) { - if block.is_unsafe { - // We currently do not format unsafe blocks. - self.push_str(self.slice(block_span)); - return; - } - if block.is_empty() { self.visit_empty_block(block_span); return; From 1adc679c9e33e233fabacb0c6850952863188b07 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 11:33:11 -0300 Subject: [PATCH 41/67] Fix after merge --- tooling/lsp/src/requests/inlay_hint.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index f407aa589c0..28076ff284f 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -635,6 +635,7 @@ fn get_expression_name(expression: &Expression) -> Option { | ExpressionKind::Comptime(..) | ExpressionKind::Resolved(..) | ExpressionKind::Literal(..) + | ExpressionKind::Unsafe(..) | ExpressionKind::Error => None, } } From bda7e7c6249df15942851a2caf847cadf238e16c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 13:04:46 -0300 Subject: [PATCH 42/67] Don't require semicolon after unsafe statement --- compiler/noirc_frontend/src/ast/statement.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 7d86ae95742..fbeb8e64885 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -100,7 +100,9 @@ impl StatementKind { StatementKind::Expression(expr) => { match (&expr.kind, semi, last_statement_in_block) { // Semicolons are optional for these expressions - (ExpressionKind::Block(_), semi, _) | (ExpressionKind::If(_), semi, _) => { + (ExpressionKind::Block(_), semi, _) + | (ExpressionKind::Unsafe(..), semi, _) + | (ExpressionKind::If(_), semi, _) => { if semi.is_some() { StatementKind::Semi(expr) } else { From cf264420214ac0adb880de49003ac289a2c1f2c7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 13:27:55 -0300 Subject: [PATCH 43/67] Add a couple more unsafe --- noir_stdlib/src/collections/umap.nr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/noir_stdlib/src/collections/umap.nr b/noir_stdlib/src/collections/umap.nr index fe16ef6bca2..86ae79ea644 100644 --- a/noir_stdlib/src/collections/umap.nr +++ b/noir_stdlib/src/collections/umap.nr @@ -119,7 +119,9 @@ impl UHashMap { B: BuildHasher, H: Hasher { // docs:end:contains_key - self.get(key).is_some() + unsafe { + self.get(key) + }.is_some() } // Returns true if the map contains no elements. @@ -438,7 +440,7 @@ where // Not marked as deleted and has key-value. if equal & slot.is_valid(){ let (key, value) = slot.key_value_unchecked(); - let other_value = other.get(key); + let other_value = unsafe { other.get(key) }; if other_value.is_none(){ equal = false; From e7e687b79ff71759a0169517c62b51042703c161 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 13:53:26 -0300 Subject: [PATCH 44/67] Fix format --- tooling/nargo_fmt/tests/expected/unsafe.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/tests/expected/unsafe.nr b/tooling/nargo_fmt/tests/expected/unsafe.nr index 6e12ef975ee..7d733c203de 100644 --- a/tooling/nargo_fmt/tests/expected/unsafe.nr +++ b/tooling/nargo_fmt/tests/expected/unsafe.nr @@ -1,5 +1,5 @@ fn main(x: pub u8, y: u8) { - unsafe { } + unsafe {} unsafe { assert_eq(x, y); From b4e8f348956ec4dbf7e95f31dc2da750d1bfb91f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 13:54:52 -0300 Subject: [PATCH 45/67] More unsafe --- test_programs/execution_success/uhashmap/src/main.nr | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/uhashmap/src/main.nr b/test_programs/execution_success/uhashmap/src/main.nr index 395ed21b6b0..f2ca6813713 100644 --- a/test_programs/execution_success/uhashmap/src/main.nr +++ b/test_programs/execution_success/uhashmap/src/main.nr @@ -294,7 +294,9 @@ unconstrained fn doc_tests() { // docs:start:get_example fn get_example(map: UHashMap>) { - let x = map.get(12); + let x = unsafe { + map.get(12) + }; if x.is_some() { assert(x.unwrap() == 42); @@ -320,7 +322,9 @@ fn entries_examples(map: UHashMap {value}"); } // docs:end:keys_example From 293c71077a706123951508730a49878709a5e629 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 14:34:08 -0300 Subject: [PATCH 46/67] nargo fmt --- noir_stdlib/src/array.nr | 2 +- noir_stdlib/src/collections/map.nr | 4 +++- noir_stdlib/src/hash/mod.nr | 4 +++- .../check_uncostrained_regression/src/main.nr | 4 +++- test_programs/execution_success/regression_5435/src/main.nr | 4 +++- 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index c735d6f3cb3..04bb747dd3b 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -18,7 +18,7 @@ impl [T; N] { for i in 0..N { let pos = find_index(sorted_index, i); assert(sorted_index[pos] == i); - } + } sorted_index }; diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index 08a38bb9dd3..bd50f345356 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -168,7 +168,9 @@ impl HashMap { for slot in self._table { if slot.is_valid() { - let (_, value) = unsafe {slot.key_value_unchecked()}; + let (_, value) = unsafe { + slot.key_value_unchecked() + }; values.push(value); } } diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index 63b208fd1d6..09c2c424025 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -84,7 +84,9 @@ fn __derive_generators( // does not assert the limbs are 128 bits // does not assert the decomposition does not overflow the EmbeddedCurveScalar fn from_field_unsafe(scalar: Field) -> EmbeddedCurveScalar { - let (xlo, xhi) = unsafe { crate::field::bn254::decompose_hint(scalar) }; + let (xlo, xhi) = unsafe { + crate::field::bn254::decompose_hint(scalar) + }; // Check that the decomposition is correct assert_eq(scalar, xlo + crate::field::bn254::TWO_POW_128 * xhi); EmbeddedCurveScalar { lo: xlo, hi: xhi } diff --git a/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr b/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr index 8cb6c6f662e..33b84c2b702 100644 --- a/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr @@ -15,7 +15,9 @@ unconstrained fn convert(trigger: Trigger) -> ResultType { } impl Trigger { fn execute(self) -> ResultType { - let result = unsafe { convert(self) }; + let result = unsafe { + convert(self) + }; assert(result.a == self.x + 1); assert(result.b == self.y - 1 + self.z[2]); assert(result.c[1] == 0); diff --git a/test_programs/execution_success/regression_5435/src/main.nr b/test_programs/execution_success/regression_5435/src/main.nr index 3146fb7c6f9..d8aeb76356b 100644 --- a/test_programs/execution_success/regression_5435/src/main.nr +++ b/test_programs/execution_success/regression_5435/src/main.nr @@ -3,7 +3,9 @@ fn main(input: Field, enable: bool) { let hash = no_predicate_function(input); // `EnableSideEffects` instruction from above instruction leaks out and removes the predicate from this call, // resulting in execution failure. - unsafe { fail(hash) }; + unsafe { + fail(hash) + }; } } From de1267387ca4c6f5d251d13338a25458d9b684d5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 14:44:48 -0300 Subject: [PATCH 47/67] More nargo fmt --- noir_stdlib/src/field/bn254.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index c1c7a41b963..ed0053694c7 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -85,7 +85,7 @@ pub fn decompose(x: Field) -> (Field, Field) { unsafe { // Take hints of the decomposition let (xlo, xhi) = decompose_hint(x); - + // Range check the limbs xlo.assert_max_bit_size(128); xhi.assert_max_bit_size(128); From f9b6ae03cb52cc0bc6c713facb89548e4d8759c6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 22 Jul 2024 14:58:40 -0300 Subject: [PATCH 48/67] More formatting --- test_programs/execution_success/bigint/src/main.nr | 4 +++- .../execution_success/brillig_nested_arrays/src/main.nr | 4 ++-- test_programs/execution_success/databus/src/main.nr | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/test_programs/execution_success/bigint/src/main.nr b/test_programs/execution_success/bigint/src/main.nr index 2455d8a7c54..188ccd01131 100644 --- a/test_programs/execution_success/bigint/src/main.nr +++ b/test_programs/execution_success/bigint/src/main.nr @@ -17,7 +17,9 @@ fn main(mut x: [u8; 5], y: [u8; 5]) { let c = if x[0] != 0 { test_unconstrained1(a, b) } else { - unsafe {test_unconstrained2(a, b)} + unsafe { + test_unconstrained2(a, b) + } }; assert(c.array[0] == std::wrapping_mul(x[0], y[0])); diff --git a/test_programs/execution_success/brillig_nested_arrays/src/main.nr b/test_programs/execution_success/brillig_nested_arrays/src/main.nr index 80171d45a87..77ab4ea19a6 100644 --- a/test_programs/execution_success/brillig_nested_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_nested_arrays/src/main.nr @@ -32,9 +32,9 @@ fn main(x: Field, y: Field) { let header = Header { params: [1, 2, 3] }; let note0 = MyNote { array: [1, 2], plain: 3, header }; let note1 = MyNote { array: [4, 5], plain: 6, header }; - + assert(access_nested([note0, note1], x, y) == (2 + 4 + 3 + 1)); - + let notes = create_inside_brillig(); assert_inside_brillig(notes, x, y); create_and_assert_inside_brillig(x, y); diff --git a/test_programs/execution_success/databus/src/main.nr b/test_programs/execution_success/databus/src/main.nr index 1075241f020..ad0b55aa476 100644 --- a/test_programs/execution_success/databus/src/main.nr +++ b/test_programs/execution_success/databus/src/main.nr @@ -1,8 +1,8 @@ fn main(mut x: u32, y: call_data u32, z: call_data [u32; 4]) -> return_data u32 { let a = z[x]; - unsafe { - a + foo(y) - } + unsafe { + a + foo(y) + } } // Use an unconstrained function to force the compiler to avoid inlining From df281dd829a1f81ee24ece607b0893bb49c726a0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 15:28:00 -0300 Subject: [PATCH 49/67] Use unsafe --- noir_stdlib/src/meta/mod.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index e00c8d41d11..288f45d9444 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -30,7 +30,9 @@ pub comptime fn derive(s: StructDefinition, traits: [TraitDefinition]) -> Quoted let mut result = quote {}; for trait_to_derive in traits { - let handler = HANDLERS.get(trait_to_derive); + let handler = unsafe { + HANDLERS.get(trait_to_derive) + }; assert(handler.is_some(), f"No derive function registered for `{trait_to_derive}`"); let trait_impl = handler.unwrap()(s); From 828f4326e08cd4a9e2f347f81f2821bfce1fd4cc Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 9 Aug 2024 16:06:38 -0300 Subject: [PATCH 50/67] std::unsafe -> std::unsafe_func --- compiler/noirc_frontend/src/monomorphization/mod.rs | 4 ++-- .../compile_success_empty/arithmetic_generics/src/main.nr | 4 ++-- .../comptime_fmt_strings/src/main.nr | 2 +- .../compile_success_empty/zeroed_slice/src/main.nr | 2 +- test_programs/execution_success/slice_regex/src/main.nr | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 54a681cf467..510b81d9acb 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1660,7 +1660,7 @@ impl<'interner> Monomorphizer<'interner> { Ok((block_let_stmt, closure_ident)) } - /// Implements std::unsafe::zeroed by returning an appropriate zeroed + /// Implements std::unsafe_func::zeroed by returning an appropriate zeroed /// ast literal or collection node for the given type. Note that for functions /// there is no obvious zeroed value so this should be considered unsafe to use. fn zeroed_value_of_type( @@ -1723,7 +1723,7 @@ impl<'interner> Monomorphizer<'interner> { } // Creating a zeroed function value is almost always an error if it is used later, - // Hence why std::unsafe::zeroed is unsafe. + // Hence why std::unsafe_func::zeroed is unsafe. // // To avoid confusing later passes, we arbitrarily choose to construct a function // that satisfies the input type by discarding all its parameters and returning a diff --git a/test_programs/compile_success_empty/arithmetic_generics/src/main.nr b/test_programs/compile_success_empty/arithmetic_generics/src/main.nr index d4f71d38413..bb9ede9e476 100644 --- a/test_programs/compile_success_empty/arithmetic_generics/src/main.nr +++ b/test_programs/compile_success_empty/arithmetic_generics/src/main.nr @@ -11,7 +11,7 @@ fn main() { fn split_first(array: [T; N]) -> (T, [T; N - 1]) { std::static_assert(N != 0, "split_first called on empty array"); - let mut new_array: [T; N - 1] = std::unsafe::zeroed(); + let mut new_array: [T; N - 1] = std::unsafe_func::zeroed(); for i in 0..N - 1 { new_array[i] = array[i + 1]; @@ -21,7 +21,7 @@ fn split_first(array: [T; N]) -> (T, [T; N - 1]) { } fn push(array: [Field; N], element: Field) -> [Field; N + 1] { - let mut result: [_; N + 1] = std::unsafe::zeroed(); + let mut result: [_; N + 1] = std::unsafe_func::zeroed(); result[array.len()] = element; for i in 0..array.len() { diff --git a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr index 19572fd15a1..040162e5fc5 100644 --- a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr +++ b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr @@ -7,7 +7,7 @@ fn main() { // Can't print these at compile-time here since printing to stdout while // compiling breaks the test runner. let s1 = f"x is {x}, fake interpolation: \{y}, y is {y}"; - let s2 = std::unsafe::zeroed::>(); + let s2 = std::unsafe_func::zeroed::>(); (s1, s2) }; assert_eq(s1, "x is 4, fake interpolation: {y}, y is 5"); diff --git a/test_programs/compile_success_empty/zeroed_slice/src/main.nr b/test_programs/compile_success_empty/zeroed_slice/src/main.nr index 44ccb2bd595..689da080347 100644 --- a/test_programs/compile_success_empty/zeroed_slice/src/main.nr +++ b/test_programs/compile_success_empty/zeroed_slice/src/main.nr @@ -1,3 +1,3 @@ fn main() { - let _: [u8] = std::unsafe::zeroed(); + let _: [u8] = std::unsafe_func::zeroed(); } diff --git a/test_programs/execution_success/slice_regex/src/main.nr b/test_programs/execution_success/slice_regex/src/main.nr index 43bd4433c69..e0c57dd8dc9 100644 --- a/test_programs/execution_success/slice_regex/src/main.nr +++ b/test_programs/execution_success/slice_regex/src/main.nr @@ -327,7 +327,7 @@ fn main() { // // impl Bvec { // fn empty() -> Self { -// Self { inner: [std::unsafe::zeroed(); N], offset: 0, len: 0 } +// Self { inner: [std::unsafe_func::zeroed(); N], offset: 0, len: 0 } // } // // fn new(array: [T; N]) -> Self { @@ -559,7 +559,7 @@ fn main() { // } // // fn reverse_array(input: [T; N]) -> [T; N] { -// let mut output = [std::unsafe::zeroed(); N]; +// let mut output = [std::unsafe_func::zeroed(); N]; // for i in 0..N { // output[i] = input[N - (i + 1)]; // } @@ -587,7 +587,7 @@ fn main() { // assert_eq(xs, ys); // // // test that pop_front gives all contents, in order, -// // followed by std::unsafe::zeroed() +// // followed by std::unsafe_func::zeroed() // println(xs); // let (x, new_xs) = xs.pop_front(); // assert_eq(x, 0); @@ -606,7 +606,7 @@ fn main() { // println(xs); // if xs.len != 0 { // let (x, _new_xs) = xs.pop_front(); -// assert_eq(x, std::unsafe::zeroed()); +// assert_eq(x, std::unsafe_func::zeroed()); // } // // assert_eq(new_xs, Bvec { inner: [0, 1, 2], offset: 3, len: 0 }); From 0d92ce128b259b40a0da2323773b70acdfb54a97 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 12 Aug 2024 12:18:21 -0300 Subject: [PATCH 51/67] Fix tests --- compiler/noirc_frontend/src/tests.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index fe22eb0d590..ba39980a0ef 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2559,7 +2559,7 @@ fn can_pass_regular_function_to_unconstrained_function() { fn foo() {} - fn expect_unconstrained(_func: unconstrained fn() -> ()) { + fn expect_unconstrained(_func: unconstrained fn() -> ()) {} "#; assert_no_errors(src); } @@ -2574,8 +2574,7 @@ fn cannot_pass_unconstrained_function_to_constrained_function() { unconstrained fn foo() {} - fn expect_regular(_func: fn() -> ()) { - } + fn expect_regular(_func: fn() -> ()) {} "#; let errors = get_program_errors(src); assert_eq!(errors.len(), 1); From ea82c01889ced4c698c1ac48d144a24000b80f7d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 12 Aug 2024 12:35:46 -0300 Subject: [PATCH 52/67] Move condition further down the line --- compiler/noirc_frontend/src/hir_def/types.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 92ad8ae3ff5..3ed9600ff01 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1778,18 +1778,16 @@ impl Type { errors: &mut Vec, make_error: impl FnOnce() -> TypeCheckError, ) { - // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` - let Some(make_error) = self.try_fn_to_unconstrained_fn_coercion( - expected, expression, interner, errors, make_error, - ) else { - return; - }; - let mut bindings = TypeBindings::new(); if let Err(UnificationError) = self.try_unify(expected, &mut bindings) { if !self.try_array_to_slice_coercion(expected, expression, interner) { - errors.push(make_error()); + // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` + if let Some(make_error) = self.try_fn_to_unconstrained_fn_coercion( + expected, expression, interner, errors, make_error, + ) { + errors.push(make_error()); + } } } else { Type::apply_type_bindings(bindings); From ca52a1ac25f76d4091da574c6ea64a1cb1babb8c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 12 Aug 2024 12:52:18 -0300 Subject: [PATCH 53/67] Simpler `try_fn_to_unconstrained_fn_coercion` --- compiler/noirc_frontend/src/hir_def/types.rs | 37 +++++++------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 3ed9600ff01..d5c4f316d4d 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1783,11 +1783,12 @@ impl Type { if let Err(UnificationError) = self.try_unify(expected, &mut bindings) { if !self.try_array_to_slice_coercion(expected, expression, interner) { // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` - if let Some(make_error) = self.try_fn_to_unconstrained_fn_coercion( - expected, expression, interner, errors, make_error, - ) { + if let Some(coerced_self) = self.try_fn_to_unconstrained_fn_coercion(expected) { + coerced_self + .unify_with_coercions(expected, expression, interner, errors, make_error); + } else { errors.push(make_error()); - } + }; } } else { Type::apply_type_bindings(bindings); @@ -1795,16 +1796,8 @@ impl Type { } // If `self` and `expected` are function types, tries to coerce `self` to `expected`. - // Returns None if this produced an error, otherwise returns Some(make_error) meaning - // that regular unification must still be done (and it could produce an error). - pub fn try_fn_to_unconstrained_fn_coercion( - &self, - expected: &Type, - expression: ExprId, - interner: &mut NodeInterner, - errors: &mut Vec, - make_error: impl FnOnce() -> TypeCheckError, - ) -> Option TypeCheckError> { + // Returns None if no coercion can be applied, otherwise returns `self` coerced to `expected`. + fn try_fn_to_unconstrained_fn_coercion(&self, expected: &Type) -> Option { // If `self` and `expected` are function types, `self` can be coerced to `expected` // if `self` is unconstrained and `expected` is not. The other way around is an error, though. let ( @@ -1812,27 +1805,21 @@ impl Type { Type::Function(_, _, _, unconstrained_expected), ) = (self.follow_bindings(), expected.follow_bindings()) else { - // No coercion needed - return Some(make_error); + return None; }; - // unconstrained status matches: no error + // Nothing to coerce if unconstrained_self == unconstrained_expected { - return Some(make_error); + return None; } - // unconstrained mismatch: produce an error + // unconstrained mismatch: can't coerce if unconstrained_self && !unconstrained_expected { - errors.push(make_error()); return None; } // Cast self unconstrained status to that of expected, so it won't produce an error - let coerced_self = Type::Function(params, ret, env, unconstrained_expected); - coerced_self.unify_with_coercions(expected, expression, interner, errors, make_error); - - // Return None: an error might have been produced in the previous call - None + Some(Type::Function(params, ret, env, unconstrained_expected)) } /// Try to apply the array to slice coercion to this given type pair and expression. From 057438ca9a5b573f5ff1083776b261eaef875c63 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 12 Aug 2024 12:56:19 -0300 Subject: [PATCH 54/67] Use early returns to avoid too much nesting --- compiler/noirc_frontend/src/hir_def/types.rs | 25 +++++++++++--------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d5c4f316d4d..4f345afadca 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1780,19 +1780,22 @@ impl Type { ) { let mut bindings = TypeBindings::new(); - if let Err(UnificationError) = self.try_unify(expected, &mut bindings) { - if !self.try_array_to_slice_coercion(expected, expression, interner) { - // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` - if let Some(coerced_self) = self.try_fn_to_unconstrained_fn_coercion(expected) { - coerced_self - .unify_with_coercions(expected, expression, interner, errors, make_error); - } else { - errors.push(make_error()); - }; - } - } else { + if let Ok(()) = self.try_unify(expected, &mut bindings) { Type::apply_type_bindings(bindings); + return; } + + if self.try_array_to_slice_coercion(expected, expression, interner) { + return; + } + + // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` + if let Some(coerced_self) = self.try_fn_to_unconstrained_fn_coercion(expected) { + coerced_self.unify_with_coercions(expected, expression, interner, errors, make_error); + return; + } + + errors.push(make_error()); } // If `self` and `expected` are function types, tries to coerce `self` to `expected`. From 52e02d184cf8f7ca0dfdd862b830e16ea15c7beb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 12 Aug 2024 13:10:36 -0300 Subject: [PATCH 55/67] Add a required unsafe --- .../compile_success_empty/macros_in_comptime/src/main.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/macros_in_comptime/src/main.nr b/test_programs/compile_success_empty/macros_in_comptime/src/main.nr index 52567025e23..31d08d2762d 100644 --- a/test_programs/compile_success_empty/macros_in_comptime/src/main.nr +++ b/test_programs/compile_success_empty/macros_in_comptime/src/main.nr @@ -4,7 +4,9 @@ use std::meta::unquote; fn main() { comptime { - foo::<3>(5); + unsafe { + foo::<3>(5) + }; submodule::bar(); } } From a0ec87251f975dd9fe2bc699c3ad6b788449c86c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 13:06:40 -0300 Subject: [PATCH 56/67] Incorrect unsafe usage is now a warning --- compiler/noirc_frontend/src/hir/type_check/errors.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 941a45bf1f4..13e29046309 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -265,7 +265,7 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { | TypeCheckError::FieldNot { span } | TypeCheckError::ConstrainedReferenceToUnconstrained { span } | TypeCheckError::UnconstrainedReferenceToConstrained { span } - | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } | TypeCheckError::Unsafe { span } + | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } | TypeCheckError::NonConstantSliceLength { span } | TypeCheckError::StringIndexAssign { span } | TypeCheckError::InvalidShiftSize { span } => { @@ -358,6 +358,9 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { let msg = "turbofish (`::<_>`) usage at this position isn't supported yet"; Diagnostic::simple_error(msg.to_string(), "".to_string(), *span) }, + TypeCheckError::Unsafe { span } => { + Diagnostic::simple_warning(error.to_string(), String::new(), *span) + } } } } From b06f9785a0aba38fcee7917fb6a4c800d74077f6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 13:15:14 -0300 Subject: [PATCH 57/67] Fixes after merge --- tooling/lsp/src/requests/completion.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 48616c0f52d..5c6c2149afd 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -513,6 +513,9 @@ impl<'a> NodeFinder<'a> { self.local_variables = old_local_variables; } + noirc_frontend::ast::ExpressionKind::Unsafe(block_expression, _) => { + self.find_in_block_expression(block_expression); + } noirc_frontend::ast::ExpressionKind::AsTraitPath(as_trait_path) => { self.find_in_as_trait_path(as_trait_path); } @@ -663,7 +666,7 @@ impl<'a> NodeFinder<'a> { noirc_frontend::ast::UnresolvedTypeData::Tuple(unresolved_types) => { self.find_in_unresolved_types(unresolved_types); } - noirc_frontend::ast::UnresolvedTypeData::Function(args, ret, env) => { + noirc_frontend::ast::UnresolvedTypeData::Function(args, ret, env, _) => { self.find_in_unresolved_types(args); self.find_in_unresolved_type(ret); self.find_in_unresolved_type(env); From 5552d63679f4468d7e49087f7d80f2c0d3325d8f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 13:29:04 -0300 Subject: [PATCH 58/67] More fixes after merge --- tooling/lsp/src/requests/completion/builtins.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tooling/lsp/src/requests/completion/builtins.rs b/tooling/lsp/src/requests/completion/builtins.rs index 070be109f13..0b21355407e 100644 --- a/tooling/lsp/src/requests/completion/builtins.rs +++ b/tooling/lsp/src/requests/completion/builtins.rs @@ -52,6 +52,7 @@ pub(super) fn keyword_builtin_type(keyword: &Keyword) -> Option<&'static str> { | Keyword::Type | Keyword::Unchecked | Keyword::Unconstrained + | Keyword::Unsafe | Keyword::Use | Keyword::Where | Keyword::While => None, @@ -120,6 +121,7 @@ pub(super) fn keyword_builtin_function(keyword: &Keyword) -> Option None, From adddc9dc4c0aaa0073b7eda97f759bbab4e20eef Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 13:33:28 -0300 Subject: [PATCH 59/67] cargo fmt --- compiler/noirc_frontend/src/hir/type_check/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 13e29046309..380753d8198 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -265,7 +265,7 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { | TypeCheckError::FieldNot { span } | TypeCheckError::ConstrainedReferenceToUnconstrained { span } | TypeCheckError::UnconstrainedReferenceToConstrained { span } - | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } + | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } | TypeCheckError::NonConstantSliceLength { span } | TypeCheckError::StringIndexAssign { span } | TypeCheckError::InvalidShiftSize { span } => { From 2d01c5c1d12e5d32baee660e93c67c4fd06bbbfd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 13:49:08 -0300 Subject: [PATCH 60/67] Put more unsafes --- .../compile_failure/unconstrained_ref/src/main.nr | 4 +++- .../execution_failure/brillig_assert_fail/src/main.nr | 6 +++++- .../execution_failure/regression_5202/src/main.nr | 8 ++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/test_programs/compile_failure/unconstrained_ref/src/main.nr b/test_programs/compile_failure/unconstrained_ref/src/main.nr index 1491badaa49..9a4f42469b5 100644 --- a/test_programs/compile_failure/unconstrained_ref/src/main.nr +++ b/test_programs/compile_failure/unconstrained_ref/src/main.nr @@ -4,5 +4,7 @@ unconstrained fn uncon_ref() -> &mut Field { } fn main() { - let e = uncon_ref(); + let e = unsafe { + uncon_ref() + }; } diff --git a/test_programs/execution_failure/brillig_assert_fail/src/main.nr b/test_programs/execution_failure/brillig_assert_fail/src/main.nr index da9d4ec1ac8..07256f0c398 100644 --- a/test_programs/execution_failure/brillig_assert_fail/src/main.nr +++ b/test_programs/execution_failure/brillig_assert_fail/src/main.nr @@ -2,7 +2,11 @@ // // The features being tested is using assert on brillig fn main(x: Field) { - assert(1 == conditional(x as bool)); + assert( + 1 == unsafe { + conditional(x as bool) + } + ); } unconstrained fn conditional(x: bool) -> Field { diff --git a/test_programs/execution_failure/regression_5202/src/main.nr b/test_programs/execution_failure/regression_5202/src/main.nr index 45ffafca4c7..fbcdba43355 100644 --- a/test_programs/execution_failure/regression_5202/src/main.nr +++ b/test_programs/execution_failure/regression_5202/src/main.nr @@ -13,11 +13,15 @@ unconstrained fn should_i_assert() -> bool { } fn get_magical_boolean() -> bool { - let option = get_unconstrained_option(); + let option = unsafe { + get_unconstrained_option() + }; let pre_assert = option.is_some().to_field(); - if should_i_assert() { + if unsafe { + should_i_assert() + } { // Note that `should_i_assert` is unconstrained, so Noir should not be able to infer // any behavior from the contents of this block. In this case it is actually false, so the // assertion below is not even executed (if it did it'd fail since the values are not equal). From 7008143f838e6a3d15d765ad85079a6786e45b71 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 13:50:15 -0300 Subject: [PATCH 61/67] And more --- .../compile_failure/brillig_mut_ref_from_acir/src/main.nr | 4 +++- .../brillig_assert_msg_runtime/src/main.nr | 6 +++++- .../fold_nested_brillig_assert_fail/src/main.nr | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr b/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr index 473ad8e8d6a..f262635e508 100644 --- a/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr +++ b/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr @@ -3,6 +3,8 @@ unconstrained fn mut_ref_identity(value: &mut Field) -> Field { } fn main(mut x: Field, y: pub Field) { - let returned_x = mut_ref_identity(&mut x); + let returned_x = unsafe { + mut_ref_identity(&mut x) + }; assert(returned_x == x); } diff --git a/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr b/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr index bd77551e304..4dd06ceb743 100644 --- a/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr +++ b/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr @@ -1,5 +1,9 @@ fn main(x: Field) { - assert(1 == conditional(x)); + assert( + 1 == unsafe { + conditional(x) + } + ); } unconstrained fn conditional(x: Field) -> Field { diff --git a/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr b/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr index 0a5038c179b..21fd4006c2a 100644 --- a/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr +++ b/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr @@ -13,7 +13,9 @@ fn fold_conditional_wrapper(x: bool) -> Field { #[fold] fn fold_conditional(x: bool) -> Field { - conditional_wrapper(x) + unsafe { + conditional_wrapper(x) + } } unconstrained fn conditional_wrapper(x: bool) -> Field { From f5e2a6065059bd43155a9d6997c1aad455d598e2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 14:03:46 -0300 Subject: [PATCH 62/67] And more unsafe --- test_programs/compile_failure/regression_5008/src/main.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test_programs/compile_failure/regression_5008/src/main.nr b/test_programs/compile_failure/regression_5008/src/main.nr index 6d9645ee6eb..cf79c22683f 100644 --- a/test_programs/compile_failure/regression_5008/src/main.nr +++ b/test_programs/compile_failure/regression_5008/src/main.nr @@ -13,5 +13,7 @@ impl Foo { fn main() { let foo = Foo { bar: &mut Bar { value: 0 } }; - foo.crash_fn(); + unsafe { + foo.crash_fn() + }; } From cf5e3c09ce746dfe9351960fb2619f29b47b5692 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 17:10:25 -0300 Subject: [PATCH 63/67] Simplify --- compiler/noirc_frontend/src/hir_def/types.rs | 26 ++++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 4f345afadca..bb8a8ed08a1 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1803,26 +1803,20 @@ impl Type { fn try_fn_to_unconstrained_fn_coercion(&self, expected: &Type) -> Option { // If `self` and `expected` are function types, `self` can be coerced to `expected` // if `self` is unconstrained and `expected` is not. The other way around is an error, though. - let ( + if let ( Type::Function(params, ret, env, unconstrained_self), Type::Function(_, _, _, unconstrained_expected), ) = (self.follow_bindings(), expected.follow_bindings()) - else { - return None; - }; - - // Nothing to coerce - if unconstrained_self == unconstrained_expected { - return None; - } - - // unconstrained mismatch: can't coerce - if unconstrained_self && !unconstrained_expected { - return None; + { + (!unconstrained_self && unconstrained_expected).then_some(Type::Function( + params, + ret, + env, + unconstrained_expected, + )) + } else { + None } - - // Cast self unconstrained status to that of expected, so it won't produce an error - Some(Type::Function(params, ret, env, unconstrained_expected)) } /// Try to apply the array to slice coercion to this given type pair and expression. From 4ef02ff5d3018f120d6f30280c875f73ddf81907 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 17:11:53 -0300 Subject: [PATCH 64/67] unsafe_func -> mem --- noir_stdlib/src/collections/bounded_vec.nr | 4 ++-- noir_stdlib/src/lib.nr | 2 +- noir_stdlib/src/{unsafe_func.nr => mem.nr} | 2 +- noir_stdlib/src/option.nr | 2 +- noir_stdlib/src/slice.nr | 2 +- .../compile_failure/array_length_defaulting/src/main.nr | 2 +- .../compile_failure/turbofish_generic_count/src/main.nr | 2 +- .../compile_success_empty/arithmetic_generics/src/main.nr | 4 ++-- .../comptime_fmt_strings/src/main.nr | 2 +- .../compile_success_empty/zeroed_slice/src/main.nr | 2 +- .../brillig_block_parameter_liveness/src/main.nr | 4 ++-- test_programs/execution_success/generics/src/main.nr | 2 +- .../execution_success/regression_4124/src/main.nr | 2 +- test_programs/execution_success/slice_regex/src/main.nr | 8 ++++---- test_programs/execution_success/unit_value/src/main.nr | 2 +- 15 files changed, 21 insertions(+), 21 deletions(-) rename noir_stdlib/src/{unsafe_func.nr => mem.nr} (89%) diff --git a/noir_stdlib/src/collections/bounded_vec.nr b/noir_stdlib/src/collections/bounded_vec.nr index 99a8d76aca9..a4c0f642a82 100644 --- a/noir_stdlib/src/collections/bounded_vec.nr +++ b/noir_stdlib/src/collections/bounded_vec.nr @@ -7,7 +7,7 @@ struct BoundedVec { impl BoundedVec { pub fn new() -> Self { - let zeroed = crate::unsafe_func::zeroed(); + let zeroed = crate::mem::zeroed(); BoundedVec { storage: [zeroed; MaxLen], len: 0 } } @@ -106,7 +106,7 @@ impl BoundedVec { self.len -= 1; let elem = self.storage[self.len]; - self.storage[self.len] = crate::unsafe_func::zeroed(); + self.storage[self.len] = crate::mem::zeroed(); elem } diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index a766bfefa71..5c4587b69ac 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -12,7 +12,6 @@ mod sha256; mod sha512; mod field; mod ec; -mod unsafe_func; mod collections; mod compat; mod convert; @@ -28,6 +27,7 @@ mod bigint; mod runtime; mod meta; mod append; +mod mem; // Oracle calls are required to be wrapped in an unconstrained function // Thus, the only argument to the `println` oracle is expected to always be an ident diff --git a/noir_stdlib/src/unsafe_func.nr b/noir_stdlib/src/mem.nr similarity index 89% rename from noir_stdlib/src/unsafe_func.nr rename to noir_stdlib/src/mem.nr index 542bd31fa84..d0418ed69a9 100644 --- a/noir_stdlib/src/unsafe_func.nr +++ b/noir_stdlib/src/mem.nr @@ -2,4 +2,4 @@ /// all of its fields to 0. This is considered to be unsafe since there /// is no guarantee that all zeroes is a valid bit pattern for every type. #[builtin(zeroed)] -pub fn zeroed() -> T {} +pub fn zeroed() -> T {} \ No newline at end of file diff --git a/noir_stdlib/src/option.nr b/noir_stdlib/src/option.nr index b6b4f56defe..8d6d9ef970d 100644 --- a/noir_stdlib/src/option.nr +++ b/noir_stdlib/src/option.nr @@ -10,7 +10,7 @@ struct Option { impl Option { /// Constructs a None value pub fn none() -> Self { - Self { _is_some: false, _value: crate::unsafe_func::zeroed() } + Self { _is_some: false, _value: crate::mem::zeroed() } } /// Constructs a Some wrapper around the given value diff --git a/noir_stdlib/src/slice.nr b/noir_stdlib/src/slice.nr index 55688241622..f9aa98a9ecd 100644 --- a/noir_stdlib/src/slice.nr +++ b/noir_stdlib/src/slice.nr @@ -49,7 +49,7 @@ impl [T] { pub fn as_array(self) -> [T; N] { assert(self.len() == N); - let mut array = [crate::unsafe_func::zeroed(); N]; + let mut array = [crate::mem::zeroed(); N]; for i in 0..N { array[i] = self[i]; } diff --git a/test_programs/compile_failure/array_length_defaulting/src/main.nr b/test_programs/compile_failure/array_length_defaulting/src/main.nr index 4e9f7e304b6..16902860a16 100644 --- a/test_programs/compile_failure/array_length_defaulting/src/main.nr +++ b/test_programs/compile_failure/array_length_defaulting/src/main.nr @@ -1,5 +1,5 @@ fn main() { - let x = std::unsafe_func::zeroed(); + let x = std::mem::zeroed(); foo(x); } diff --git a/test_programs/compile_failure/turbofish_generic_count/src/main.nr b/test_programs/compile_failure/turbofish_generic_count/src/main.nr index 98629f6a6cd..5dc05a3a5c3 100644 --- a/test_programs/compile_failure/turbofish_generic_count/src/main.nr +++ b/test_programs/compile_failure/turbofish_generic_count/src/main.nr @@ -6,7 +6,7 @@ struct Bar { impl Bar { fn zeroed(_self: Self) -> A { - std::unsafe_func::zeroed() + std::mem::zeroed() } } diff --git a/test_programs/compile_success_empty/arithmetic_generics/src/main.nr b/test_programs/compile_success_empty/arithmetic_generics/src/main.nr index bb9ede9e476..6cd13ab0e2f 100644 --- a/test_programs/compile_success_empty/arithmetic_generics/src/main.nr +++ b/test_programs/compile_success_empty/arithmetic_generics/src/main.nr @@ -11,7 +11,7 @@ fn main() { fn split_first(array: [T; N]) -> (T, [T; N - 1]) { std::static_assert(N != 0, "split_first called on empty array"); - let mut new_array: [T; N - 1] = std::unsafe_func::zeroed(); + let mut new_array: [T; N - 1] = std::mem::zeroed(); for i in 0..N - 1 { new_array[i] = array[i + 1]; @@ -21,7 +21,7 @@ fn split_first(array: [T; N]) -> (T, [T; N - 1]) { } fn push(array: [Field; N], element: Field) -> [Field; N + 1] { - let mut result: [_; N + 1] = std::unsafe_func::zeroed(); + let mut result: [_; N + 1] = std::mem::zeroed(); result[array.len()] = element; for i in 0..array.len() { diff --git a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr index 040162e5fc5..705a1b2ab4e 100644 --- a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr +++ b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr @@ -7,7 +7,7 @@ fn main() { // Can't print these at compile-time here since printing to stdout while // compiling breaks the test runner. let s1 = f"x is {x}, fake interpolation: \{y}, y is {y}"; - let s2 = std::unsafe_func::zeroed::>(); + let s2 = std::mem::zeroed::>(); (s1, s2) }; assert_eq(s1, "x is 4, fake interpolation: {y}, y is 5"); diff --git a/test_programs/compile_success_empty/zeroed_slice/src/main.nr b/test_programs/compile_success_empty/zeroed_slice/src/main.nr index 689da080347..b35afdbc6c6 100644 --- a/test_programs/compile_success_empty/zeroed_slice/src/main.nr +++ b/test_programs/compile_success_empty/zeroed_slice/src/main.nr @@ -1,3 +1,3 @@ fn main() { - let _: [u8] = std::unsafe_func::zeroed(); + let _: [u8] = std::mem::zeroed(); } diff --git a/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr b/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr index c4983de6d38..6ddfc03622a 100644 --- a/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr +++ b/test_programs/execution_success/brillig_block_parameter_liveness/src/main.nr @@ -38,11 +38,11 @@ struct Outer { // If we don't take into account block parameter liveness, this function will need 5*500=2500 stack items unconstrained fn main(conditions: [bool; 5]) -> pub Outer { let out0 = if conditions[0] { - let mut outer: Outer = std::unsafe_func::zeroed(); + let mut outer: Outer = std::mem::zeroed(); outer.middle_a.inner_a.a = 1; outer } else { - let mut outer: Outer = std::unsafe_func::zeroed(); + let mut outer: Outer = std::mem::zeroed(); outer.middle_f.inner_c.d = 2; outer }; diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index 573e44584ff..75a7f8a3154 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -34,7 +34,7 @@ impl Bar { impl Bar { // This is to test that we can use turbofish on methods as well fn zeroed(_self: Self) -> A { - std::unsafe_func::zeroed() + std::mem::zeroed() } } diff --git a/test_programs/execution_success/regression_4124/src/main.nr b/test_programs/execution_success/regression_4124/src/main.nr index 9e55c214646..7b5060062da 100644 --- a/test_programs/execution_success/regression_4124/src/main.nr +++ b/test_programs/execution_success/regression_4124/src/main.nr @@ -11,7 +11,7 @@ impl MyDeserialize<1> for Field { } pub fn storage_read() -> [Field; N] { - std::unsafe_func::zeroed() + std::mem::zeroed() } struct PublicMutable { diff --git a/test_programs/execution_success/slice_regex/src/main.nr b/test_programs/execution_success/slice_regex/src/main.nr index e0c57dd8dc9..2382f5b0f2f 100644 --- a/test_programs/execution_success/slice_regex/src/main.nr +++ b/test_programs/execution_success/slice_regex/src/main.nr @@ -327,7 +327,7 @@ fn main() { // // impl Bvec { // fn empty() -> Self { -// Self { inner: [std::unsafe_func::zeroed(); N], offset: 0, len: 0 } +// Self { inner: [std::mem::zeroed(); N], offset: 0, len: 0 } // } // // fn new(array: [T; N]) -> Self { @@ -559,7 +559,7 @@ fn main() { // } // // fn reverse_array(input: [T; N]) -> [T; N] { -// let mut output = [std::unsafe_func::zeroed(); N]; +// let mut output = [std::mem::zeroed(); N]; // for i in 0..N { // output[i] = input[N - (i + 1)]; // } @@ -587,7 +587,7 @@ fn main() { // assert_eq(xs, ys); // // // test that pop_front gives all contents, in order, -// // followed by std::unsafe_func::zeroed() +// // followed by std::mem::zeroed() // println(xs); // let (x, new_xs) = xs.pop_front(); // assert_eq(x, 0); @@ -606,7 +606,7 @@ fn main() { // println(xs); // if xs.len != 0 { // let (x, _new_xs) = xs.pop_front(); -// assert_eq(x, std::unsafe_func::zeroed()); +// assert_eq(x, std::mem::zeroed()); // } // // assert_eq(new_xs, Bvec { inner: [0, 1, 2], offset: 3, len: 0 }); diff --git a/test_programs/execution_success/unit_value/src/main.nr b/test_programs/execution_success/unit_value/src/main.nr index d28e99a0a64..18c7e5f4f7d 100644 --- a/test_programs/execution_success/unit_value/src/main.nr +++ b/test_programs/execution_success/unit_value/src/main.nr @@ -1,5 +1,5 @@ fn get_transaction() { - std::unsafe_func::zeroed() + std::mem::zeroed() } fn main() { From 5aea602dace8675b0e789019ac3a4515c8a9b3ce Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 17:31:45 -0300 Subject: [PATCH 65/67] then_some -> then --- compiler/noirc_frontend/src/hir_def/types.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index bb8a8ed08a1..852fb582528 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1808,12 +1808,8 @@ impl Type { Type::Function(_, _, _, unconstrained_expected), ) = (self.follow_bindings(), expected.follow_bindings()) { - (!unconstrained_self && unconstrained_expected).then_some(Type::Function( - params, - ret, - env, - unconstrained_expected, - )) + (!unconstrained_self && unconstrained_expected) + .then(|| Type::Function(params, ret, env, unconstrained_expected)) } else { None } From c615bb9a27af8a343536ee99ebae3cdea9689f18 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 13 Aug 2024 17:54:01 -0300 Subject: [PATCH 66/67] nargo fmt --- noir_stdlib/src/mem.nr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/noir_stdlib/src/mem.nr b/noir_stdlib/src/mem.nr index d0418ed69a9..88d17e20ee3 100644 --- a/noir_stdlib/src/mem.nr +++ b/noir_stdlib/src/mem.nr @@ -2,4 +2,5 @@ /// all of its fields to 0. This is considered to be unsafe since there /// is no guarantee that all zeroes is a valid bit pattern for every type. #[builtin(zeroed)] -pub fn zeroed() -> T {} \ No newline at end of file +pub fn zeroed() -> T {} + From 42d05601cf019740ce2b65d8fe17a243ecda6b1c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 14 Aug 2024 11:04:07 -0300 Subject: [PATCH 67/67] Fix after merge --- tooling/lsp/src/requests/completion.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 6c117fc01a9..78b49a55be7 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -1015,7 +1015,7 @@ impl<'a> NodeFinder<'a> { | Type::TypeVariable(_, _) | Type::TraitAsType(_, _, _) | Type::NamedGeneric(_, _, _) - | Type::Function(_, _, _) + | Type::Function(..) | Type::Forall(_, _) | Type::Constant(_) | Type::Quoted(_) @@ -1560,8 +1560,11 @@ fn func_meta_type_to_string(func_meta: &FuncMeta, has_self_type: bool) -> String typ = typ_; } - if let Type::Function(args, ret, _env) = typ { + if let Type::Function(args, ret, _env, unconstrained) = typ { let mut string = String::new(); + if *unconstrained { + string.push_str("unconstrained "); + } string.push_str("fn("); for (index, arg) in args.iter().enumerate() { if index > 0 {