From 5c00a79d2c93056d07330c350bf7b6efbf81d477 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 20 Dec 2024 11:21:17 +0100 Subject: [PATCH] feat: add a warning when using unsafe blocks without safety comments (#6860) --- compiler/noirc_frontend/src/debug/mod.rs | 21 +++++++++--- compiler/noirc_frontend/src/lexer/lexer.rs | 23 ++++++++++--- compiler/noirc_frontend/src/lexer/token.rs | 3 ++ compiler/noirc_frontend/src/parser/errors.rs | 13 +++++++- .../src/parser/parser/expression.rs | 19 +++++++++-- compiler/noirc_frontend/src/tests.rs | 5 ++- .../noirc_frontend/src/tests/references.rs | 1 + docs/docs/noir/concepts/unconstrained.md | 2 ++ noir_stdlib/src/array/check_shuffle.nr | 3 ++ noir_stdlib/src/array/mod.nr | 7 ++-- noir_stdlib/src/collections/umap.nr | 11 +++++-- noir_stdlib/src/field/bn254.nr | 15 +++++++-- noir_stdlib/src/field/mod.nr | 1 + noir_stdlib/src/hash/mod.nr | 5 ++- noir_stdlib/src/hash/sha256.nr | 32 ++++++++++++++----- noir_stdlib/src/lib.nr | 2 ++ noir_stdlib/src/meta/expr.nr | 7 +++- noir_stdlib/src/uint128.nr | 21 ++++++++++-- .../brillig_mut_ref_from_acir/src/main.nr | 1 + .../regression_5008/src/main.nr | 1 + .../unconstrained_ref/src/main.nr | 1 + .../brillig_cast/src/main.nr | 1 + .../src/main.nr | 1 + .../src/main.nr | 1 + .../brillig_modulo/src/main.nr | 1 + .../brillig_slice_input/src/main.nr | 2 ++ .../macros_in_comptime/src/main.nr | 5 ++- .../check_uncostrained_regression/src/main.nr | 1 + .../src/main.nr | 1 + .../brillig_assert_fail/src/main.nr | 1 + .../brillig_assert_msg_runtime/src/main.nr | 1 + .../src/main.nr | 1 + .../regression_5202/src/main.nr | 2 ++ .../acir_inside_brillig_recursion/src/main.nr | 1 + .../aes128_encrypt/src/main.nr | 10 ++++-- .../src/main.nr | 1 + .../execution_success/bigint/src/main.nr | 1 + .../brillig_acir_as_brillig/src/main.nr | 1 + .../brillig_arrays/src/main.nr | 1 + .../brillig_blake2s/src/main.nr | 1 + .../brillig_calls/src/main.nr | 1 + .../brillig_calls_array/src/main.nr | 1 + .../brillig_calls_conditionals/src/main.nr | 1 + .../brillig_conditional/src/main.nr | 1 + .../brillig_fns_as_values/src/main.nr | 1 + .../brillig_identity_function/src/main.nr | 1 + .../brillig_nested_arrays/src/main.nr | 1 + .../execution_success/brillig_not/src/main.nr | 1 + .../brillig_oracle/src/main.nr | 1 + .../brillig_recursion/src/main.nr | 1 + .../brillig_uninitialized_arrays/src/main.nr | 1 + .../global_consts/src/main.nr | 5 ++- .../hint_black_box/src/main.nr | 5 ++- .../is_unconstrained/src/main.nr | 1 + .../nested_arrays_from_brillig/src/main.nr | 5 ++- .../reference_only_used_as_alias/src/main.nr | 1 + .../regression_5435/src/main.nr | 5 ++- .../regression_6451/src/main.nr | 5 ++- .../regression_6674_3/src/main.nr | 5 ++- .../src/main.nr | 5 ++- .../execution_success/u16_support/src/main.nr | 1 + .../execution_success/uhashmap/src/main.nr | 11 +++++-- .../comptime_expr/src/main.nr | 11 +++++-- .../noir_test_success/mock_oracle/src/main.nr | 9 ++++++ .../out_of_bounds_alignment/src/main.nr | 1 + tooling/nargo_fmt/src/formatter.rs | 12 +++---- .../src/formatter/comments_and_whitespace.rs | 8 +++-- tooling/nargo_fmt/src/formatter/expression.rs | 14 ++++++-- tooling/nargo_fmt/src/formatter/statement.rs | 5 ++- tooling/nargo_fmt/tests/expected/unsafe.nr | 4 ++- tooling/nargo_fmt/tests/input/unsafe.nr | 5 +-- 71 files changed, 294 insertions(+), 63 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index f05fc721581..c9c2cdcfea1 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -523,7 +523,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) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_var_assign_inner(var_id, value); }} } @@ -534,7 +536,9 @@ pub fn build_debug_crate_file() -> String { __debug_var_drop_oracle(var_id); } pub fn __debug_var_drop(var_id: u32) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_var_drop_inner(var_id); }} } @@ -545,7 +549,9 @@ pub fn build_debug_crate_file() -> String { __debug_fn_enter_oracle(fn_id); } pub fn __debug_fn_enter(fn_id: u32) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_fn_enter_inner(fn_id); }} } @@ -556,7 +562,9 @@ pub fn build_debug_crate_file() -> String { __debug_fn_exit_oracle(fn_id); } pub fn __debug_fn_exit(fn_id: u32) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_fn_exit_inner(fn_id); }} } @@ -567,7 +575,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) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_dereference_assign_inner(var_id, value); }} } @@ -594,6 +604,7 @@ pub fn build_debug_crate_file() -> String { }} pub fn __debug_member_assign_{n}(var_id: u32, value: T, {var_sig}) {{ unsafe {{ + //@safety: debug context __debug_inner_member_assign_{n}(var_id, value, {vars}); }} }} diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index a5c4b2cd772..99799b9a1c0 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -726,7 +726,7 @@ impl<'a> Lexer<'a> { } fn parse_comment(&mut self, start: u32) -> SpannedTokenResult { - let doc_style = match self.peek_char() { + let mut doc_style = match self.peek_char() { Some('!') => { self.next_char(); Some(DocStyle::Inner) @@ -735,9 +735,17 @@ impl<'a> Lexer<'a> { self.next_char(); Some(DocStyle::Outer) } + Some('@') => Some(DocStyle::Safety), _ => None, }; - let comment = self.eat_while(None, |ch| ch != '\n'); + let mut comment = self.eat_while(None, |ch| ch != '\n'); + if doc_style == Some(DocStyle::Safety) { + if comment.starts_with("@safety") { + comment = comment["@safety".len()..].to_string(); + } else { + doc_style = None; + } + } if !comment.is_ascii() { let span = Span::from(start..self.position); @@ -752,7 +760,7 @@ impl<'a> Lexer<'a> { } fn parse_block_comment(&mut self, start: u32) -> SpannedTokenResult { - let doc_style = match self.peek_char() { + let mut doc_style = match self.peek_char() { Some('!') => { self.next_char(); Some(DocStyle::Inner) @@ -761,6 +769,7 @@ impl<'a> Lexer<'a> { self.next_char(); Some(DocStyle::Outer) } + Some('@') => Some(DocStyle::Safety), _ => None, }; @@ -787,7 +796,13 @@ impl<'a> Lexer<'a> { ch => content.push(ch), } } - + if doc_style == Some(DocStyle::Safety) { + if content.starts_with("@safety") { + content = content["@safety".len()..].to_string(); + } else { + doc_style = None; + } + } if depth == 0 { if !content.is_ascii() { let span = Span::from(start..self.position); diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index f35515045db..ffd755e606f 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -345,6 +345,7 @@ impl Display for FmtStrFragment { pub enum DocStyle { Outer, Inner, + Safety, } #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -424,11 +425,13 @@ impl fmt::Display for Token { Token::LineComment(ref s, style) => match style { Some(DocStyle::Inner) => write!(f, "//!{s}"), Some(DocStyle::Outer) => write!(f, "///{s}"), + Some(DocStyle::Safety) => write!(f, "//@safety{s}"), None => write!(f, "//{s}"), }, Token::BlockComment(ref s, style) => match style { Some(DocStyle::Inner) => write!(f, "/*!{s}*/"), Some(DocStyle::Outer) => write!(f, "/**{s}*/"), + Some(DocStyle::Safety) => write!(f, "/*@safety{s}*/"), None => write!(f, "/*{s}*/"), }, Token::Quote(ref stream) => { diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index 899928528e6..7cb8731593b 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -108,6 +108,8 @@ pub enum ParserErrorReason { WrongNumberOfAttributeArguments { name: String, min: usize, max: usize, found: usize }, #[error("The `deprecated` attribute expects a string argument")] DeprecatedAttributeExpectsAStringArgument, + #[error("Unsafe block must start with a safety comment")] + MissingSafetyComment, } /// Represents a parsing error, or a parsing error in the making. @@ -181,7 +183,11 @@ impl ParserError { } pub fn is_warning(&self) -> bool { - matches!(self.reason(), Some(ParserErrorReason::ExperimentalFeature(_))) + matches!( + self.reason(), + Some(ParserErrorReason::ExperimentalFeature(_)) + | Some(ParserErrorReason::MissingSafetyComment) + ) } } @@ -272,6 +278,11 @@ impl<'a> From<&'a ParserError> for Diagnostic { "Noir doesn't have immutable references, only mutable references".to_string(), error.span, ), + ParserErrorReason::MissingSafetyComment => Diagnostic::simple_warning( + "Missing Safety Comment".into(), + "Unsafe block must start with a safety comment: //@safety".into(), + error.span, + ), other => Diagnostic::simple_error(format!("{other}"), String::new(), error.span), }, None => { diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index 03c22684459..6b059bb0c8c 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -8,7 +8,7 @@ use crate::{ MemberAccessExpression, MethodCallExpression, Statement, TypePath, UnaryOp, UnresolvedType, }, parser::{labels::ParsingRuleLabel, parser::parse_many::separated_by_comma, ParserErrorReason}, - token::{Keyword, Token, TokenKind}, + token::{DocStyle, Keyword, SpannedToken, Token, TokenKind}, }; use super::{ @@ -375,6 +375,20 @@ impl<'a> Parser<'a> { return None; } + let next_token = self.next_token.token(); + if matches!( + next_token, + Token::LineComment(_, Some(DocStyle::Safety)) + | Token::BlockComment(_, Some(DocStyle::Safety)) + ) { + //Checks the safety comment is there, and skip it + let span = self.current_token_span; + self.eat_left_brace(); + self.token = SpannedToken::new(Token::LeftBrace, span); + } else { + self.push_error(ParserErrorReason::MissingSafetyComment, self.current_token_span); + } + let start_span = self.current_token_span; if let Some(block) = self.parse_block() { Some(ExpressionKind::Unsafe(block, self.span_since(start_span))) @@ -957,7 +971,8 @@ mod tests { #[test] fn parses_unsafe_expression() { - let src = "unsafe { 1 }"; + let src = "unsafe { //@safety: test + 1 }"; let expr = parse_expression_no_errors(src); let ExpressionKind::Unsafe(block, _) = expr.kind else { panic!("Expected unsafe expression"); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 5bfcf2a65a4..3d908d1aa0c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3880,7 +3880,8 @@ fn errors_on_cyclic_globals() { fn warns_on_unneeded_unsafe() { let src = r#" fn main() { - unsafe { + unsafe { + //@safety: test foo() } } @@ -3900,7 +3901,9 @@ fn warns_on_nested_unsafe() { let src = r#" fn main() { unsafe { + //@safety: test unsafe { + //@safety: test foo() } } diff --git a/compiler/noirc_frontend/src/tests/references.rs b/compiler/noirc_frontend/src/tests/references.rs index ce72240d146..7ae3974a125 100644 --- a/compiler/noirc_frontend/src/tests/references.rs +++ b/compiler/noirc_frontend/src/tests/references.rs @@ -88,6 +88,7 @@ fn constrained_reference_to_unconstrained() { let x_ref = &mut x; if x == 5 { unsafe { + //@safety: test context mut_ref_input(x_ref, y); } } diff --git a/docs/docs/noir/concepts/unconstrained.md b/docs/docs/noir/concepts/unconstrained.md index b5221b8d2dd..8e07d719f4b 100644 --- a/docs/docs/noir/concepts/unconstrained.md +++ b/docs/docs/noir/concepts/unconstrained.md @@ -67,6 +67,7 @@ We can then run `u72_to_u8` as unconstrained brillig code in order to calculate ```rust fn main(num: u72) -> pub [u8; 8] { let out = unsafe { + //@safety: 'out' is properly constrained below in 'assert(num == reconstructed_num);' u72_to_u8(num) }; @@ -96,6 +97,7 @@ This ends up taking off another ~250 gates from our circuit! We've ended up with 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. +Furthermore, a warning is emitted unless the `unsafe` block starts with a `//@safety: ...` comment explaining why it is fine to call the unconstrained function. 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. diff --git a/noir_stdlib/src/array/check_shuffle.nr b/noir_stdlib/src/array/check_shuffle.nr index 2e8c227feee..828ac733290 100644 --- a/noir_stdlib/src/array/check_shuffle.nr +++ b/noir_stdlib/src/array/check_shuffle.nr @@ -43,6 +43,9 @@ where T: Eq, { unsafe { + /*@safety : shuffle_indices is ensured to be a permutation of 0..N, and then + shuffle_indices is ensured to map lhs to rhs: assert(lhs[i] == rhs[shuffle_indices[i]]), for all i in 0..N + */ let shuffle_indices = __get_shuffle_indices(lhs, rhs); for i in 0..N { diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 8bb425854f2..1309a15d571 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -185,9 +185,10 @@ where /// ``` pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { unsafe { - // Safety: `sorted` array is checked to be: - // a. a permutation of `input`'s elements - // b. satisfying the predicate `ordering` + /*@safety: `sorted` array is checked to be: + a. a permutation of `input`'s elements + b. satisfying the predicate `ordering` + */ let sorted = quicksort::quicksort(self, ordering); if !is_unconstrained() { diff --git a/noir_stdlib/src/collections/umap.nr b/noir_stdlib/src/collections/umap.nr index 7aebeb437cf..27af14caa24 100644 --- a/noir_stdlib/src/collections/umap.nr +++ b/noir_stdlib/src/collections/umap.nr @@ -113,7 +113,11 @@ impl UHashMap { H: Hasher, { // docs:end:contains_key - unsafe { self.get(key) }.is_some() + unsafe { + //@safety : unconstrained context + self.get(key) + } + .is_some() } // Returns true if the map contains no elements. @@ -432,7 +436,10 @@ where // Not marked as deleted and has key-value. if equal & slot.is_valid() { let (key, value) = slot.key_value_unchecked(); - let other_value = unsafe { other.get(key) }; + let other_value = unsafe { + //@safety : unconstrained context + other.get(key) + }; if other_value.is_none() { equal = false; diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index a7ca7d77373..f3cbdca2762 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -39,6 +39,10 @@ fn assert_gt_limbs(a: (Field, Field), b: (Field, Field)) { let (alo, ahi) = a; let (blo, bhi) = b; unsafe { + /*@safety: borrow is enforced to be boolean due to its type. + if borrow is 0, it asserts that (alo > blo && ahi >= bhi) + if borrow is 1, it asserts that (alo <= blo && ahi > bhi) + */ let borrow = lte_hint(alo, blo); let rlo = alo - blo - 1 + (borrow as Field) * TWO_POW_128; @@ -55,6 +59,7 @@ pub fn decompose(x: Field) -> (Field, Field) { compute_decomposition(x) } else { unsafe { + /*@safety: decomposition is properly checked below*/ // Take hints of the decomposition let (xlo, xhi) = decompose_hint(x); @@ -74,7 +79,12 @@ pub fn decompose(x: Field) -> (Field, Field) { pub fn assert_gt(a: Field, b: Field) { if is_unconstrained() { - assert(unsafe { field_less_than(b, a) }); + assert( + unsafe { + //@safety: already unconstrained + field_less_than(b, a) + }, + ); } else { // Decompose a and b let a_limbs = decompose(a); @@ -92,13 +102,14 @@ pub fn assert_lt(a: Field, b: Field) { pub fn gt(a: Field, b: Field) -> bool { if is_unconstrained() { unsafe { + //@safety: unsafe in unconstrained field_less_than(b, a) } } else if a == b { false } else { - // Take a hint of the comparison and verify it unsafe { + //@safety: Take a hint of the comparison and verify it if field_less_than(a, b) { assert_gt(b, a); false diff --git a/noir_stdlib/src/field/mod.nr b/noir_stdlib/src/field/mod.nr index a5c1ff7bb62..7e5ad97d64f 100644 --- a/noir_stdlib/src/field/mod.nr +++ b/noir_stdlib/src/field/mod.nr @@ -238,6 +238,7 @@ pub fn bytes32_to_field(bytes32: [u8; 32]) -> Field { fn lt_fallback(x: Field, y: Field) -> bool { if is_unconstrained() { unsafe { + //@safety : unconstrained context field_less_than(x, y) } } else { diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index 5f7017b60b7..89f6d2a1363 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -96,7 +96,10 @@ 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 { + //@safety : xlo and xhi decomposition is checked below + 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/noir_stdlib/src/hash/sha256.nr b/noir_stdlib/src/hash/sha256.nr index b9a2b02c9d9..a4e21304912 100644 --- a/noir_stdlib/src/hash/sha256.nr +++ b/noir_stdlib/src/hash/sha256.nr @@ -74,8 +74,10 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { let mut msg_byte_ptr = 0; for i in 0..num_blocks { let msg_start = BLOCK_SIZE * i; - let (new_msg_block, new_msg_byte_ptr) = - unsafe { build_msg_block(msg, message_size, msg_start) }; + let (new_msg_block, new_msg_byte_ptr) = unsafe { + /*@safety : the msg_block is checked below in verify_msg_block*/ + build_msg_block(msg, message_size, msg_start) + }; if msg_start < message_size { msg_block = new_msg_block; @@ -104,8 +106,10 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { // or our message cannot be evenly split into blocks. if modulo != 0 { let msg_start = BLOCK_SIZE * num_blocks; - let (new_msg_block, new_msg_byte_ptr) = - unsafe { build_msg_block(msg, message_size, msg_start) }; + let (new_msg_block, new_msg_byte_ptr) = unsafe { + //@safety : the msg_block is checked below in verify_msg_block + build_msg_block(msg, message_size, msg_start) + }; if msg_start < message_size { msg_block = new_msg_block; @@ -146,7 +150,10 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { msg_byte_ptr = 0; } - msg_block = unsafe { attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size) }; + msg_block = unsafe { + //@safety : the msg_len is checked below in verify_msg_len + attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size) + }; if !is_unconstrained() { verify_msg_len(msg_block, last_block, msg_byte_ptr, message_size); @@ -798,7 +805,10 @@ mod tests { 101, 115, 46, 48, ]; assert_eq(input.len(), 22); - let (msg_block, msg_byte_ptr) = unsafe { build_msg_block(input, input.len(), 0) }; + let (msg_block, msg_byte_ptr) = unsafe { + //@safety : testing context + build_msg_block(input, input.len(), 0) + }; assert_eq(msg_byte_ptr, input.len()); assert_eq(msg_block[0], make_item(input[0], input[1], input[2], input[3])); assert_eq(msg_block[1], make_item(input[4], input[5], input[6], input[7])); @@ -815,7 +825,10 @@ mod tests { 108, 97, 105, 110, 59, 32, 99, 104, 97, 114, 115, 101, 116, ]; assert_eq(input.len(), 68); - let (msg_block, msg_byte_ptr) = unsafe { build_msg_block(input, input.len(), 64) }; + let (msg_block, msg_byte_ptr) = unsafe { + //@safety : testing context + build_msg_block(input, input.len(), 64) + }; assert_eq(msg_byte_ptr, 4); assert_eq(msg_block[0], make_item(input[64], input[65], input[66], input[67])); assert_eq(msg_block[1], 0); @@ -828,7 +841,10 @@ mod tests { 1919905082, 1131376244, 1701737517, 1417244773, 978151789, 1697470053, 1920166255, 1849316213, 1651139939, ]; - let msg_block = unsafe { attach_len_to_msg_block(input, 1, 448) }; + let msg_block = unsafe { + //@safety : testing context + attach_len_to_msg_block(input, 1, 448) + }; assert_eq(msg_block[0], ((1 << 7) as u32) * 256 * 256 * 256); assert_eq(msg_block[1], 0); assert_eq(msg_block[15], 3584); diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index 1cb8f7fc2dd..cdc82b20509 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -41,12 +41,14 @@ unconstrained fn print_unconstrained(with_newline: bool, input: T) { pub fn println(input: T) { unsafe { + //@safety: a print statement cannot be constrained print_unconstrained(true, input); } } pub fn print(input: T) { unsafe { + //@safety: a print statement cannot be constrained print_unconstrained(false, input); } } diff --git a/noir_stdlib/src/meta/expr.nr b/noir_stdlib/src/meta/expr.nr index bb795a520d3..a6bb4630e81 100644 --- a/noir_stdlib/src/meta/expr.nr +++ b/noir_stdlib/src/meta/expr.nr @@ -669,7 +669,12 @@ comptime fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr { comptime fn new_unsafe(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { ; }); - quote { unsafe { $exprs }}.as_expr().unwrap() + quote { unsafe { + //@safety: generated by macro + $exprs + }} + .as_expr() + .unwrap() } comptime fn join_expressions(exprs: [Expr], separator: Quoted) -> Quoted { diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index b448e11acb1..fab8cacc055 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -104,8 +104,16 @@ impl U128 { if ascii < 58 { ascii - 48 } else { - let ascii = - ascii + 32 * (unsafe { U128::uconstrained_check_is_upper_ascii(ascii) as u8 }); + let ascii = ascii + + 32 + * ( + unsafe { + /*@safety : optionally adds 32 and then check (below) the result is in 'a..f' range + This enforces that the input is in either 'A..F' or 'a..f' range + */ + U128::uconstrained_check_is_upper_ascii(ascii) as u8 + } + ); assert(ascii >= 97); // enforce >= 'a' assert(ascii <= 102); // enforce <= 'f' ascii - 87 @@ -213,6 +221,9 @@ impl Mul for U128 { impl Div for U128 { fn div(self: Self, b: U128) -> U128 { unsafe { + /*@safety : euclidian division is asserted to be correct: assert(a == b * q + r); and assert(r < b); + Furthermore, U128 addition and multiplication ensures that b * q + r does not overflow + */ let (q, r) = self.unconstrained_div(b); let a = b * q + r; assert_eq(self, a); @@ -225,6 +236,7 @@ impl Div for U128 { impl Rem for U128 { fn rem(self: Self, b: U128) -> U128 { unsafe { + //@safety : cf div() above let (q, r) = self.unconstrained_div(b); let a = b * q + r; assert_eq(self, a); @@ -444,6 +456,7 @@ mod tests { let c = U128::one(); let d = U128::from_u64s_le(0x0, 0x1); unsafe { + //@safety: testing context let (q, r) = a.unconstrained_div(b); assert_eq(q, c); assert_eq(r, d); @@ -453,12 +466,14 @@ mod tests { let b = U128::one(); // Check the case where a is a multiple of b unsafe { + //@safety: testing context let (c, d) = a.unconstrained_div(b); assert_eq((c, d), (a, U128::zero())); } // Check where b is a multiple of a unsafe { + //@safety: testing context let (c, d) = b.unconstrained_div(a); assert_eq((c, d), (U128::zero(), b)); } @@ -467,6 +482,7 @@ mod tests { let a = U128::from_u64s_le(0x1, 0x0); let b = U128::zero(); unsafe { + //@safety: testing context let (c, d) = a.unconstrained_div(b); assert_eq((c, d), (U128::zero(), U128::zero())); } @@ -474,6 +490,7 @@ mod tests { let a = U128::from_u64s_le(0x0, pow63 as u64); let b = U128::from_u64s_le(0x0, pow63 as u64); unsafe { + //@safety: testing context let (c, d) = a.unconstrained_div(b); assert_eq((c, d), (U128::one(), U128::zero())); } 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 f262635e508..29431005c29 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 @@ -4,6 +4,7 @@ unconstrained fn mut_ref_identity(value: &mut Field) -> Field { fn main(mut x: Field, y: pub Field) { let returned_x = unsafe { + //@safety: testing context mut_ref_identity(&mut x) }; assert(returned_x == x); diff --git a/test_programs/compile_failure/regression_5008/src/main.nr b/test_programs/compile_failure/regression_5008/src/main.nr index cf79c22683f..be1adb119aa 100644 --- a/test_programs/compile_failure/regression_5008/src/main.nr +++ b/test_programs/compile_failure/regression_5008/src/main.nr @@ -14,6 +14,7 @@ fn main() { let foo = Foo { bar: &mut Bar { value: 0 } }; unsafe { + //@safety: testing context foo.crash_fn() }; } diff --git a/test_programs/compile_failure/unconstrained_ref/src/main.nr b/test_programs/compile_failure/unconstrained_ref/src/main.nr index 9a4f42469b5..282d598b484 100644 --- a/test_programs/compile_failure/unconstrained_ref/src/main.nr +++ b/test_programs/compile_failure/unconstrained_ref/src/main.nr @@ -5,6 +5,7 @@ unconstrained fn uncon_ref() -> &mut Field { fn main() { let e = unsafe { + //@safety: testing context uncon_ref() }; } 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 f1a6814f4e7..581ca8dc16a 100644 --- a/test_programs/compile_success_empty/brillig_cast/src/main.nr +++ b/test_programs/compile_success_empty/brillig_cast/src/main.nr @@ -3,6 +3,7 @@ // The features being tested are cast operations on brillig fn main() { unsafe { + //@safety: testing context bool_casts(); field_casts(); uint_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 fedfe48cb0d..15f1b23853d 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,6 +1,7 @@ // Tests arithmetic operations on fields fn main() { unsafe { + //@safety: testing context let x = 4; let y = 2; assert((x + y) == add(x, y)); 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 7ecc21dbd2f..4ee04807355 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 @@ -4,6 +4,7 @@ fn main() { let y: u32 = 2; unsafe { + //@safety: testing context assert((x + y) == add(x, y)); assert((x - y) == sub(x, y)); 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 2ad43bdb7eb..47f2466f453 100644 --- a/test_programs/compile_success_empty/brillig_modulo/src/main.nr +++ b/test_programs/compile_success_empty/brillig_modulo/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is modulo operations on brillig fn main() { unsafe { + //@safety: testing context assert(modulo(47, 3) == 2); assert(modulo(2, 3) == 2); assert(signed_modulo(5, 3) == 2); 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 436a939767e..41bd2d8c3ef 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 @@ -17,12 +17,14 @@ fn main() { let mut slice = &[]; slice = slice.push_back([Point { x: 13, y: 14 }, Point { x: 20, y: 8 }]); unsafe { + //@safety: testing context let brillig_sum = sum_slice(slice); assert_eq(brillig_sum, 55); } slice = slice.push_back([Point { x: 15, y: 5 }, Point { x: 12, y: 13 }]); unsafe { + //@safety: testing context let brillig_sum = sum_slice(slice); assert_eq(brillig_sum, 100); } 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 799091fca09..9e4439e67cd 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 @@ -6,7 +6,10 @@ global three_field: Field = 3; fn main() { comptime { - unsafe { foo::(5) }; + unsafe { + //@safety: testing context + foo::(5) + }; submodule::bar(); } } 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 33b84c2b702..b8c4137aa74 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 @@ -16,6 +16,7 @@ unconstrained fn convert(trigger: Trigger) -> ResultType { impl Trigger { fn execute(self) -> ResultType { let result = unsafe { + //@safety: testing context convert(self) }; assert(result.a == self.x + 1); diff --git a/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr b/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr index 1d9269eebd5..7047be3a577 100644 --- a/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr +++ b/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr @@ -10,6 +10,7 @@ unconstrained fn maximum_price(options: [u32; 3]) -> u32 { fn main(sandwiches: pub [u32; 3], drinks: pub [u32; 3], snacks: pub [u32; 3], best_value: u32) { unsafe { + //@safety: testing context let meal_deal_cost: u32 = 390; let most_expensive_sandwich = maximum_price(sandwiches); let mut sandwich_exists = false; 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 07256f0c398..4ca70baae2e 100644 --- a/test_programs/execution_failure/brillig_assert_fail/src/main.nr +++ b/test_programs/execution_failure/brillig_assert_fail/src/main.nr @@ -4,6 +4,7 @@ fn main(x: Field) { assert( 1 == unsafe { + //@safety: testing context conditional(x as bool) } ); 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 4dd06ceb743..d726e699bab 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,6 +1,7 @@ fn main(x: Field) { assert( 1 == unsafe { + //@safety: testing context conditional(x) } ); 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 1c1563ffe1d..b87744399c6 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 @@ -15,6 +15,7 @@ fn fold_conditional_wrapper(x: bool) -> Field { #[fold] fn fold_conditional(x: bool) -> Field { unsafe { + //@safety: testing context conditional_wrapper(x) } } diff --git a/test_programs/execution_failure/regression_5202/src/main.nr b/test_programs/execution_failure/regression_5202/src/main.nr index fbcdba43355..a270a9fccad 100644 --- a/test_programs/execution_failure/regression_5202/src/main.nr +++ b/test_programs/execution_failure/regression_5202/src/main.nr @@ -14,12 +14,14 @@ unconstrained fn should_i_assert() -> bool { fn get_magical_boolean() -> bool { let option = unsafe { + //@safety: testing context get_unconstrained_option() }; let pre_assert = option.is_some().to_field(); if unsafe { + //@safety: testing context should_i_assert() } { // Note that `should_i_assert` is unconstrained, so Noir should not be able to infer 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 49b7c00b6b9..b4cc69bb00a 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,6 @@ fn main() { unsafe { + //@safety: testing context assert_eq(fibonacci(3), fibonacci_hint(3)); } } diff --git a/test_programs/execution_success/aes128_encrypt/src/main.nr b/test_programs/execution_success/aes128_encrypt/src/main.nr index 3d3992971f7..7738bed5255 100644 --- a/test_programs/execution_success/aes128_encrypt/src/main.nr +++ b/test_programs/execution_success/aes128_encrypt/src/main.nr @@ -29,9 +29,15 @@ fn main(inputs: str<12>, iv: str<16>, key: str<16>, output: str<32>) { let result: [u8; 16] = std::aes128::aes128_encrypt(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()).as_array(); - let output_bytes: [u8; 16] = unsafe { decode_hex(output) }; + let output_bytes: [u8; 16] = unsafe { + //@safety: testing context + decode_hex(output) + }; assert(result == output_bytes); - let unconstrained_result = unsafe { cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()) }; + let unconstrained_result = unsafe { + //@safety: testing context + cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()) + }; assert(unconstrained_result == output_bytes); } 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 09f99622939..425c48c21f1 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,7 @@ unconstrained fn return_array(val: Field) -> [Field; 1] { fn main(val: Field) { unsafe { + //@safety: testing context 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 5a8b9fb67ef..65cf990dcaf 100644 --- a/test_programs/execution_success/bigint/src/main.nr +++ b/test_programs/execution_success/bigint/src/main.nr @@ -18,6 +18,7 @@ fn main(mut x: [u8; 5], y: [u8; 5]) { test_unconstrained1(a, b) } else { unsafe { + //@safety: testing context test_unconstrained2(a, b) } }; 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 1a595ecfb38..bcb995591c7 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,5 +1,6 @@ fn main(x: u32) { unsafe { + //@safety: testing context assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); diff --git a/test_programs/execution_success/brillig_arrays/src/main.nr b/test_programs/execution_success/brillig_arrays/src/main.nr index 00d43e2b10d..b4cd13e1091 100644 --- a/test_programs/execution_success/brillig_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_arrays/src/main.nr @@ -3,6 +3,7 @@ // The features being tested are array reads and writes fn main(x: [Field; 3]) { unsafe { + //@safety: testing context read_array(x); read_write_array(x); } diff --git a/test_programs/execution_success/brillig_blake2s/src/main.nr b/test_programs/execution_success/brillig_blake2s/src/main.nr index c4412bd2bba..58f45e8d667 100644 --- a/test_programs/execution_success/brillig_blake2s/src/main.nr +++ b/test_programs/execution_success/brillig_blake2s/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is blake2s in brillig fn main(x: [u8; 5], result: [u8; 32]) { unsafe { + //@safety: testing context assert(blake2s(x) == result); } } diff --git a/test_programs/execution_success/brillig_calls/src/main.nr b/test_programs/execution_success/brillig_calls/src/main.nr index c76d7651b0a..5bd3458da51 100644 --- a/test_programs/execution_success/brillig_calls/src/main.nr +++ b/test_programs/execution_success/brillig_calls/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is brillig calls fn main(x: u32) { unsafe { + /*@safety : testing context*/ assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); 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 90f588ab988..8d5e01dd7c4 100644 --- a/test_programs/execution_success/brillig_calls_array/src/main.nr +++ b/test_programs/execution_success/brillig_calls_array/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is brillig calls passing arrays around fn main(x: [u32; 3]) { unsafe { + //@safety: testing context assert(entry_point(x) == 9); another_entry_point(x); } 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 0fbfd84968f..e03f04a0cbc 100644 --- a/test_programs/execution_success/brillig_calls_conditionals/src/main.nr +++ b/test_programs/execution_success/brillig_calls_conditionals/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is brillig calls with conditionals fn main(x: [u32; 3]) { unsafe { + //@safety: testing context assert(entry_point(x[0]) == 7); assert(entry_point(x[1]) == 8); assert(entry_point(x[2]) == 9); diff --git a/test_programs/execution_success/brillig_conditional/src/main.nr b/test_programs/execution_success/brillig_conditional/src/main.nr index 17484f6e8c5..5934f982f88 100644 --- a/test_programs/execution_success/brillig_conditional/src/main.nr +++ b/test_programs/execution_success/brillig_conditional/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is basic conditonal on brillig fn main(x: Field) { unsafe { + //@safety: testing context assert(4 == conditional(x == 1)); } } 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 03a92b4b10d..b56542fd0ae 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 @@ -4,6 +4,7 @@ struct MyStruct { fn main(x: u32) { unsafe { + //@safety: testing context assert(wrapper(increment, x) == x + 1); assert(wrapper(increment_acir, x) == x + 1); assert(wrapper(decrement, x) == x - 1); 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 f3b45d0de4e..7bf584579d3 100644 --- a/test_programs/execution_success/brillig_identity_function/src/main.nr +++ b/test_programs/execution_success/brillig_identity_function/src/main.nr @@ -7,6 +7,7 @@ struct myStruct { // The features being tested is the identity function in Brillig fn main(x: Field) { unsafe { + //@safety: testing context assert(x == identity(x)); // TODO: add support for array comparison let arr = identity_array([x, x]); 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 77ab4ea19a6..e97db35f800 100644 --- a/test_programs/execution_success/brillig_nested_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_nested_arrays/src/main.nr @@ -29,6 +29,7 @@ unconstrained fn create_and_assert_inside_brillig(x: Field, y: Field) { fn main(x: Field, y: Field) { unsafe { + //@safety: testing context 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 }; diff --git a/test_programs/execution_success/brillig_not/src/main.nr b/test_programs/execution_success/brillig_not/src/main.nr index 4e61b6a54ea..38641f5ee5f 100644 --- a/test_programs/execution_success/brillig_not/src/main.nr +++ b/test_programs/execution_success/brillig_not/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is not instruction on brillig fn main(x: Field, y: Field) { unsafe { + //@safety: testing context assert(false == not_operator(x as bool)); assert(true == not_operator(y as bool)); } diff --git a/test_programs/execution_success/brillig_oracle/src/main.nr b/test_programs/execution_success/brillig_oracle/src/main.nr index 8f5b2fa7566..e59c908fd6f 100644 --- a/test_programs/execution_success/brillig_oracle/src/main.nr +++ b/test_programs/execution_success/brillig_oracle/src/main.nr @@ -4,6 +4,7 @@ use std::test::OracleMock; // Tests oracle usage in brillig/unconstrained functions fn main(_x: Field) { unsafe { + //@safety: testing context let size = 20; // TODO: Add a method along the lines of `(0..size).to_array()`. let mut mock_oracle_response = [0; 20]; diff --git a/test_programs/execution_success/brillig_recursion/src/main.nr b/test_programs/execution_success/brillig_recursion/src/main.nr index 5354777a1de..fa048c30abe 100644 --- a/test_programs/execution_success/brillig_recursion/src/main.nr +++ b/test_programs/execution_success/brillig_recursion/src/main.nr @@ -3,6 +3,7 @@ // The feature being tested is brillig recursion fn main(x: u32) { unsafe { + //@safety: testing context assert(fibonacci(x) == 55); } } diff --git a/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr b/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr index d4b74162cfb..839ac489f2d 100644 --- a/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr +++ b/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr @@ -1,5 +1,6 @@ fn main(x: Field, y: Field) -> pub Field { unsafe { + //@safety: testing context let notes = create_notes(x, y); sum_x(notes, x, y) } diff --git a/test_programs/execution_success/global_consts/src/main.nr b/test_programs/execution_success/global_consts/src/main.nr index 2eaab810d6a..70423810327 100644 --- a/test_programs/execution_success/global_consts/src/main.nr +++ b/test_programs/execution_success/global_consts/src/main.nr @@ -25,7 +25,10 @@ unconstrained fn calculate_global_value() -> Field { } // Regression test for https://github.com/noir-lang/noir/issues/4318 -global CALCULATED_GLOBAL: Field = unsafe { calculate_global_value() }; +global CALCULATED_GLOBAL: Field = unsafe { + //@safety: testing context + calculate_global_value() +}; fn main( a: [Field; M + N - N], diff --git a/test_programs/execution_success/hint_black_box/src/main.nr b/test_programs/execution_success/hint_black_box/src/main.nr index 1109c54301f..d53cf335878 100644 --- a/test_programs/execution_success/hint_black_box/src/main.nr +++ b/test_programs/execution_success/hint_black_box/src/main.nr @@ -23,7 +23,10 @@ fn main(a: u32, b: u32) { //assert_eq(slice_sum(black_box(arr.as_slice())), b); // But we can pass a blackboxed slice to Brillig. - let s = unsafe { brillig_slice_sum(black_box(arr.as_slice())) }; + let s = unsafe { + //@safety: testing context + brillig_slice_sum(black_box(arr.as_slice())) + }; assert_eq(s, b); let mut d = b; diff --git a/test_programs/execution_success/is_unconstrained/src/main.nr b/test_programs/execution_success/is_unconstrained/src/main.nr index d06366cf642..dddf2dfec2e 100644 --- a/test_programs/execution_success/is_unconstrained/src/main.nr +++ b/test_programs/execution_success/is_unconstrained/src/main.nr @@ -10,6 +10,7 @@ unconstrained fn unconstrained_intermediate() { fn main() { unsafe { + //@safety: testing context unconstrained_intermediate(); } check(false); 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 f8070aa3ef4..aec0ba3be33 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,10 @@ unconstrained fn create_inside_brillig(values: [Field; 6]) -> [MyNote; 2] { } fn main(values: [Field; 6]) { - let notes = unsafe { create_inside_brillig(values) }; + let notes = unsafe { + //@safety: testing context + create_inside_brillig(values) + }; assert(access_nested(notes) == (2 + 4 + 3 + 1)); } diff --git a/test_programs/execution_success/reference_only_used_as_alias/src/main.nr b/test_programs/execution_success/reference_only_used_as_alias/src/main.nr index 8b5b804cf94..c310f052fce 100644 --- a/test_programs/execution_success/reference_only_used_as_alias/src/main.nr +++ b/test_programs/execution_success/reference_only_used_as_alias/src/main.nr @@ -62,6 +62,7 @@ where { |e: Event| { unsafe { + //@safety: testing context func(context.a); } emit_with_keys(context, randomness, e, compute); diff --git a/test_programs/execution_success/regression_5435/src/main.nr b/test_programs/execution_success/regression_5435/src/main.nr index 895a6983ad9..4e90002e2a2 100644 --- a/test_programs/execution_success/regression_5435/src/main.nr +++ b/test_programs/execution_success/regression_5435/src/main.nr @@ -3,7 +3,10 @@ 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 { + //@safety: testing context + fail(hash) + }; } } diff --git a/test_programs/execution_success/regression_6451/src/main.nr b/test_programs/execution_success/regression_6451/src/main.nr index b13b6c25a7e..192e09fe3f5 100644 --- a/test_programs/execution_success/regression_6451/src/main.nr +++ b/test_programs/execution_success/regression_6451/src/main.nr @@ -10,7 +10,10 @@ fn main(x: Field) { value.assert_max_bit_size::<1>(); // Regression test for #6447 (Aztec Packages issue #9488) - let y = unsafe { empty(x + 1) }; + let y = unsafe { + //@safety: testing context + empty(x + 1) + }; let z = y + x + 1; let z1 = z + y; assert(z + z1 != 3); diff --git a/test_programs/execution_success/regression_6674_3/src/main.nr b/test_programs/execution_success/regression_6674_3/src/main.nr index 2c87a4c679c..a0f2d0668d5 100644 --- a/test_programs/execution_success/regression_6674_3/src/main.nr +++ b/test_programs/execution_success/regression_6674_3/src/main.nr @@ -166,7 +166,10 @@ pub unconstrained fn sort_by_counter_desc(array: [T; N]) -> [T; N pub unconstrained fn sort_by(array: [T; N]) -> [T; N] { let mut result = array; - unsafe { get_sorting_index(array) }; + unsafe { + //@safety: testing context + get_sorting_index(array) + }; result } diff --git a/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr b/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr index fc1e55ee641..c0d8b336a04 100644 --- a/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr +++ b/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr @@ -7,7 +7,10 @@ fn main(x: u8, nest: bool) { #[no_predicates] pub fn unsafe_assert(msg: [u8; N]) -> u8 { - let block = unsafe { get_block(msg) }; + let block = unsafe { + //@safety: testing context + get_block(msg) + }; verify_block(msg, block); block[0] } diff --git a/test_programs/execution_success/u16_support/src/main.nr b/test_programs/execution_success/u16_support/src/main.nr index ca41c708077..c6a212f908e 100644 --- a/test_programs/execution_success/u16_support/src/main.nr +++ b/test_programs/execution_success/u16_support/src/main.nr @@ -1,6 +1,7 @@ fn main(x: u16) { test_u16(x); unsafe { + //@safety: testing context test_u16_unconstrained(x); } } diff --git a/test_programs/execution_success/uhashmap/src/main.nr b/test_programs/execution_success/uhashmap/src/main.nr index 689ba9d4a04..d1ba1c6175d 100644 --- a/test_programs/execution_success/uhashmap/src/main.nr +++ b/test_programs/execution_success/uhashmap/src/main.nr @@ -293,7 +293,10 @@ unconstrained fn doc_tests() { // docs:start:get_example fn get_example(map: UHashMap>) { - let x = unsafe { map.get(12) }; + let x = unsafe { + //@safety: testing context + map.get(12) + }; if x.is_some() { assert(x.unwrap() == 42); @@ -318,7 +321,11 @@ fn entries_examples(map: UHashMap {value}"); } // docs:end:keys_example diff --git a/test_programs/noir_test_success/comptime_expr/src/main.nr b/test_programs/noir_test_success/comptime_expr/src/main.nr index 709180879a0..39167242a7b 100644 --- a/test_programs/noir_test_success/comptime_expr/src/main.nr +++ b/test_programs/noir_test_success/comptime_expr/src/main.nr @@ -597,7 +597,12 @@ mod tests { fn test_expr_as_unsafe() { comptime { - let expr = quote { unsafe { 1; 4; 23 } }.as_expr().unwrap(); + let expr = quote { + unsafe + { + //@safety" test + 1; 4; 23 } + }.as_expr().unwrap(); let exprs = expr.as_unsafe().unwrap(); assert_eq(exprs.len(), 3); } @@ -607,7 +612,9 @@ mod tests { fn test_expr_modify_for_unsafe() { comptime { - let expr = quote { unsafe { 1; 4; 23 } }.as_expr().unwrap(); + let expr = quote { unsafe { + //@safety: test + 1; 4; 23 } }.as_expr().unwrap(); let expr = expr.modify(times_two); let exprs = expr.as_unsafe().unwrap(); assert_eq(exprs.len(), 3); 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 1b427043e91..b026f510418 100644 --- a/test_programs/noir_test_success/mock_oracle/src/main.nr +++ b/test_programs/noir_test_success/mock_oracle/src/main.nr @@ -35,6 +35,7 @@ unconstrained fn struct_field(point: Point, array: [Field; 4]) -> Field { #[test(should_fail)] fn test_mock_no_returns() { unsafe { + //@safety: testing context OracleMock::mock("void_field"); void_field(); // Some return value must be set } @@ -43,6 +44,7 @@ fn test_mock_no_returns() { #[test] fn test_mock() { unsafe { + //@safety: testing context OracleMock::mock("void_field").returns(10); assert_eq(void_field(), 10); } @@ -51,6 +53,7 @@ fn test_mock() { #[test] fn test_multiple_mock() { unsafe { + //@safety: testing context let first_mock = OracleMock::mock("void_field").returns(10); OracleMock::mock("void_field").returns(42); @@ -65,6 +68,7 @@ fn test_multiple_mock() { #[test] fn test_multiple_mock_times() { unsafe { + //@safety: testing context OracleMock::mock("void_field").returns(10).times(2); OracleMock::mock("void_field").returns(42); @@ -77,6 +81,7 @@ fn test_multiple_mock_times() { #[test] fn test_mock_with_params() { unsafe { + //@safety: testing context OracleMock::mock("field_field").with_params((5,)).returns(10); assert_eq(field_field(5), 10); } @@ -85,6 +90,7 @@ fn test_mock_with_params() { #[test] fn test_multiple_mock_with_params() { unsafe { + //@safety: testing context OracleMock::mock("field_field").with_params((5,)).returns(10); OracleMock::mock("field_field").with_params((7,)).returns(14); @@ -96,6 +102,7 @@ fn test_multiple_mock_with_params() { #[test] fn test_mock_last_params() { unsafe { + //@safety: testing context let mock = OracleMock::mock("field_field").returns(10); assert_eq(field_field(5), 10); @@ -106,6 +113,7 @@ fn test_mock_last_params() { #[test] fn test_mock_last_params_many_calls() { unsafe { + //@safety: testing context let mock = OracleMock::mock("field_field").returns(10); assert_eq(field_field(5), 10); assert_eq(field_field(7), 10); @@ -123,6 +131,7 @@ fn test_mock_struct_field() { let point = Point { x: 14, y: 27 }; unsafe { + //@safety: testing context OracleMock::mock("struct_field").returns(42).times(2); let timeless_mock = OracleMock::mock("struct_field").returns(0); assert_eq(42, struct_field(point, array)); 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 fb90e3d96c5..99277b62939 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 @@ -14,6 +14,7 @@ fn test_acir() { #[test(should_fail)] fn test_brillig() { unsafe { + //@safety: testing context assert_eq(out_of_bounds_unconstrained_wrapper([0; 50], [0; 50]), 0); } } diff --git a/tooling/nargo_fmt/src/formatter.rs b/tooling/nargo_fmt/src/formatter.rs index 9a9386e1911..4184ff288d7 100644 --- a/tooling/nargo_fmt/src/formatter.rs +++ b/tooling/nargo_fmt/src/formatter.rs @@ -228,13 +228,11 @@ impl<'a> Formatter<'a> { /// Writes the current indentation to the buffer, but only if the buffer /// is empty or it ends with a newline (otherwise we'd be indenting when not needed). pub(crate) fn write_indentation(&mut self) { - if !(self.buffer.is_empty() || self.buffer.ends_with_newline()) { - return; - } - - for _ in 0..self.indentation { - for _ in 0..self.config.tab_spaces { - self.write(" "); + if self.buffer.is_empty() || self.buffer.ends_with_newline() { + for _ in 0..self.indentation { + for _ in 0..self.config.tab_spaces { + self.write(" "); + } } } } diff --git a/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs b/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs index e20eb4291d1..57512d049df 100644 --- a/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs +++ b/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs @@ -1,4 +1,4 @@ -use noirc_frontend::token::Token; +use noirc_frontend::token::{DocStyle, Token}; use super::Formatter; @@ -113,7 +113,8 @@ impl<'a> Formatter<'a> { last_was_block_comment = false; } - Token::LineComment(comment, None) => { + Token::LineComment(comment, None) + | Token::LineComment(comment, Some(DocStyle::Safety)) => { if comment.trim() == "noir-fmt:ignore" { ignore_next = true; } @@ -142,7 +143,8 @@ impl<'a> Formatter<'a> { last_was_block_comment = false; self.written_comments_count += 1; } - Token::BlockComment(comment, None) => { + Token::BlockComment(comment, None) + | Token::BlockComment(comment, Some(DocStyle::Safety)) => { if comment.trim() == "noir-fmt:ignore" { ignore_next = true; } diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index ecc9fab18ce..2f71d3209a7 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -1910,15 +1910,21 @@ global y = 1; #[test] fn format_unsafe_one_expression() { - let src = "global x = unsafe { 1 } ;"; - let expected = "global x = unsafe { 1 };\n"; + let src = "global x = unsafe { //@safety: testing + 1 } ;"; + let expected = "global x = unsafe { + //@safety: testing + 1 +};\n"; assert_format(src, expected); } #[test] fn format_unsafe_two_expressions() { - let src = "global x = unsafe { 1; 2 } ;"; + let src = "global x = unsafe { //@safety: testing + 1; 2 } ;"; let expected = "global x = unsafe { + //@safety: testing 1; 2 }; @@ -2198,6 +2204,7 @@ global y = 1; let src = "mod moo { fn foo() { let mut sorted_write_tuples = unsafe { + //@safety: testing get_sorted_tuple( final_public_data_writes.storage, |(_, leaf_a): (u32, PublicDataTreeLeaf), (_, leaf_b): (u32, PublicDataTreeLeaf)| full_field_less_than( @@ -2211,6 +2218,7 @@ global y = 1; let expected = "mod moo { fn foo() { let mut sorted_write_tuples = unsafe { + //@safety: testing get_sorted_tuple( final_public_data_writes.storage, |(_, leaf_a): (u32, PublicDataTreeLeaf), (_, leaf_b): (u32, PublicDataTreeLeaf)| { diff --git a/tooling/nargo_fmt/src/formatter/statement.rs b/tooling/nargo_fmt/src/formatter/statement.rs index 50c286ff161..116d68e308f 100644 --- a/tooling/nargo_fmt/src/formatter/statement.rs +++ b/tooling/nargo_fmt/src/formatter/statement.rs @@ -489,9 +489,12 @@ mod tests { #[test] fn format_unsafe_statement() { - let src = " fn foo() { unsafe { 1 } } "; + let src = " fn foo() { unsafe { + //@safety: testing context + 1 } } "; let expected = "fn foo() { unsafe { + //@safety: testing context 1 } } diff --git a/tooling/nargo_fmt/tests/expected/unsafe.nr b/tooling/nargo_fmt/tests/expected/unsafe.nr index 7d733c203de..ec99d90c662 100644 --- a/tooling/nargo_fmt/tests/expected/unsafe.nr +++ b/tooling/nargo_fmt/tests/expected/unsafe.nr @@ -1,7 +1,9 @@ fn main(x: pub u8, y: u8) { - unsafe {} + unsafe { //@safety: testing + } unsafe { + //@safety: testing assert_eq(x, y); } } diff --git a/tooling/nargo_fmt/tests/input/unsafe.nr b/tooling/nargo_fmt/tests/input/unsafe.nr index 6e12ef975ee..65ed061835f 100644 --- a/tooling/nargo_fmt/tests/input/unsafe.nr +++ b/tooling/nargo_fmt/tests/input/unsafe.nr @@ -1,7 +1,8 @@ fn main(x: pub u8, y: u8) { - unsafe { } + unsafe {//@safety: testing + } - unsafe { + unsafe {//@safety: testing assert_eq(x, y); } }