From bf4176f9fc2ae13ddc3f3ca534bc0611f85d7aa7 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 14 Nov 2024 21:07:50 +0000 Subject: [PATCH] chore: pull changes out of sync PR (#9966) Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. --- noir/noir-repo/Cargo.lock | 11 + .../src/brillig/brillig_ir/procedures/mod.rs | 5 +- .../compiler/noirc_evaluator/src/lib.rs | 1 - .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 22 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 3 +- .../noirc_evaluator/src/ssa/opt/mod.rs | 4 +- .../noirc_evaluator/src/ssa/parser.rs | 9 +- .../noirc_evaluator/src/ssa/parser/lexer.rs | 1 + .../noirc_evaluator/src/ssa/parser/tests.rs | 11 + .../noirc_evaluator/src/ssa/parser/token.rs | 3 + .../noirc_frontend/src/elaborator/comptime.rs | 9 +- .../src/hir/comptime/interpreter/builtin.rs | 3 +- .../src/hir/comptime/interpreter/foreign.rs | 26 ++- .../src/hir/def_collector/dc_mod.rs | 38 +++- .../src/hir/def_collector/errors.rs | 8 + .../compiler/noirc_frontend/src/tests.rs | 48 +++++ .../Nargo.toml | 6 + .../src/main.nr | 5 + .../attribute_args/src/main.nr | 2 +- .../Nargo.toml | 5 + .../src/main.nr | 5 + noir/noir-repo/tooling/nargo_cli/Cargo.toml | 1 + noir/noir-repo/tooling/nargo_cli/build.rs | 201 ++++++++++++++---- 23 files changed, 356 insertions(+), 71 deletions(-) create mode 100644 noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/Nargo.toml create mode 100644 noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/src/main.nr create mode 100644 noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/Nargo.toml create mode 100644 noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/src/main.nr diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index 35ff97f55e3..15ad7806a17 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -1510,6 +1510,16 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "file-lock" +version = "2.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "040b48f80a749da50292d0f47a1e2d5bf1d772f52836c07f64bfccc62ba6e664" +dependencies = [ + "cc", + "libc", +] + [[package]] name = "filetime" version = "0.2.25" @@ -2777,6 +2787,7 @@ dependencies = [ "criterion", "dap", "dirs", + "file-lock", "fm", "iai", "iter-extended", diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs index 2fa51f3db59..0955142e414 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs @@ -1,3 +1,6 @@ +use noirc_errors::debug_info::ProcedureDebugId; +use serde::{Deserialize, Serialize}; + mod array_copy; mod array_reverse; mod check_max_stack_depth; @@ -14,11 +17,9 @@ use array_copy::compile_array_copy_procedure; use array_reverse::compile_array_reverse_procedure; use check_max_stack_depth::compile_check_max_stack_depth_procedure; use mem_copy::compile_mem_copy_procedure; -use noirc_errors::debug_info::ProcedureDebugId; use prepare_vector_insert::compile_prepare_vector_insert_procedure; use prepare_vector_push::compile_prepare_vector_push_procedure; use revert_with_string::compile_revert_with_string_procedure; -use serde::{Deserialize, Serialize}; use vector_copy::compile_vector_copy_procedure; use vector_pop_back::compile_vector_pop_back_procedure; use vector_pop_front::compile_vector_pop_front_procedure; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs b/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs index b2b4762723f..5f0c7a5bbb8 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs @@ -12,7 +12,6 @@ pub mod ssa; pub mod brillig; pub use ssa::create_program; - pub use ssa::ir::instruction::ErrorType; /// Trims leading whitespace from each line of the input string, according to diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index e7d298558c4..c6e4a261897 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -13,6 +13,7 @@ use acvm::acir::circuit::opcodes::{ }; use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode}; use acvm::brillig_vm::{MemoryValue, VMStatus, VM}; +use acvm::BlackBoxFunctionSolver; use acvm::{ acir::AcirField, acir::{ @@ -107,7 +108,9 @@ impl From for AcirType { /// Context object which holds the relationship between /// `Variables`(AcirVar) and types such as `Expression` and `Witness` /// which are placed into ACIR. -pub(crate) struct AcirContext { +pub(crate) struct AcirContext> { + blackbox_solver: B, + /// Two-way map that links `AcirVar` to `AcirVarData`. /// /// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`. @@ -132,7 +135,7 @@ pub(crate) struct AcirContext { pub(crate) warnings: Vec, } -impl AcirContext { +impl> AcirContext { pub(crate) fn set_expression_width(&mut self, expression_width: ExpressionWidth) { self.expression_width = expression_width; } @@ -1758,8 +1761,8 @@ impl AcirContext { brillig_stdlib_func, ); - fn range_constraint_value( - context: &mut AcirContext, + fn range_constraint_value>( + context: &mut AcirContext, value: &AcirValue, ) -> Result<(), RuntimeError> { match value { @@ -1878,7 +1881,7 @@ impl AcirContext { inputs: &[BrilligInputs], outputs_types: &[AcirType], ) -> Option> { - let mut memory = (execute_brillig(code, inputs)?).into_iter(); + let mut memory = (execute_brillig(code, &self.blackbox_solver, inputs)?).into_iter(); let outputs_var = vecmap(outputs_types.iter(), |output| match output { AcirType::NumericType(_) => { @@ -2171,8 +2174,9 @@ pub(crate) struct AcirVar(usize); /// Returns the finished state of the Brillig VM if execution can complete. /// /// Returns `None` if complete execution of the Brillig bytecode is not possible. -fn execute_brillig( +fn execute_brillig>( code: &[BrilligOpcode], + blackbox_solver: &B, inputs: &[BrilligInputs], ) -> Option>> { // Set input values @@ -2198,12 +2202,8 @@ fn execute_brillig( } // Instantiate a Brillig VM given the solved input registers and memory, along with the Brillig bytecode. - // - // We pass a stubbed solver here as a concrete solver implies a field choice which conflicts with this function - // being generic. - let solver = acvm::blackbox_solver::StubbedBlackBoxSolver; let profiling_active = false; - let mut vm = VM::new(calldata, code, Vec::new(), &solver, profiling_active); + let mut vm = VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active); // Run the Brillig VM on these inputs, bytecode, etc! let vm_status = vm.process_opcodes(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index ecf7561321f..33fdf2abc82 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -30,6 +30,7 @@ use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunction use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::acir::circuit::opcodes::{AcirFunctionId, BlockType}; +use bn254_blackbox_solver::Bn254BlackBoxSolver; use noirc_frontend::monomorphization::ast::InlineType; use acvm::acir::circuit::brillig::{BrilligBytecode, BrilligFunctionId}; @@ -157,7 +158,7 @@ struct Context<'a> { current_side_effects_enabled_var: AcirVar, /// Manages and builds the `AcirVar`s to which the converted SSA values refer. - acir_context: AcirContext, + acir_context: AcirContext, /// Track initialized acir dynamic arrays /// diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs index 5576b494570..098f62bceba 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -44,7 +44,7 @@ pub(crate) fn assert_normalized_ssa_equals(mut ssa: super::Ssa, expected: &str) let expected = trim_leading_whitespace_from_lines(expected); if ssa != expected { - println!("Got:\n~~~\n{}\n~~~\nExpected:\n~~~\n{}\n~~~", ssa, expected); - similar_asserts::assert_eq!(ssa, expected); + println!("Expected:\n~~~\n{expected}\n~~~\nGot:\n~~~\n{ssa}\n~~~"); + similar_asserts::assert_eq!(expected, ssa); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser.rs index 717d2691b2f..11d43284786 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser.rs @@ -735,10 +735,17 @@ impl<'a> Parser<'a> { } fn eat_int(&mut self) -> ParseResult> { + let negative = self.eat(Token::Dash)?; + if matches!(self.token.token(), Token::Int(..)) { let token = self.bump()?; match token.into_token() { - Token::Int(int) => Ok(Some(int)), + Token::Int(mut int) => { + if negative { + int = -int; + } + Ok(Some(int)) + } _ => unreachable!(), } } else { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs index ac4c3b77205..4c90475be74 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs @@ -60,6 +60,7 @@ impl<'a> Lexer<'a> { Some(']') => self.single_char_token(Token::RightBracket), Some('&') => self.single_char_token(Token::Ampersand), Some('-') if self.peek_char() == Some('>') => self.double_char_token(Token::Arrow), + Some('-') => self.single_char_token(Token::Dash), Some(ch) if ch.is_ascii_alphanumeric() || ch == '_' => self.eat_alpha_numeric(ch), Some(char) => Err(LexerError::UnexpectedCharacter { char, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 3ed6be57b5e..9205353151e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -425,3 +425,14 @@ fn test_slice() { "; assert_ssa_roundtrip(src); } + +#[test] +fn test_negative() { + let src = " + acir(inline) fn main f0 { + b0(): + return Field -1 + } + "; + assert_ssa_roundtrip(src); +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs index 41c4f9ca164..d648f58de41 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs @@ -57,6 +57,8 @@ pub(crate) enum Token { Equal, /// & Ampersand, + /// - + Dash, Eof, } @@ -90,6 +92,7 @@ impl Display for Token { Token::Arrow => write!(f, "->"), Token::Equal => write!(f, "=="), Token::Ampersand => write!(f, "&"), + Token::Dash => write!(f, "-"), Token::Eof => write!(f, "(end of stream)"), } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs index 279adc331ea..a27e2bf0163 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -452,7 +452,14 @@ impl<'context> Elaborator<'context> { } ItemKind::Impl(r#impl) => { let module = self.module_id(); - dc_mod::collect_impl(self.interner, generated_items, r#impl, self.file, module); + dc_mod::collect_impl( + self.interner, + generated_items, + r#impl, + self.file, + module, + &mut self.errors, + ); } ItemKind::ModuleDecl(_) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 8a6c46ca50c..80c1ee217c2 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -42,7 +42,7 @@ use crate::{ }; use self::builtin_helpers::{eq_item, get_array, get_ctstring, get_str, get_u8, hash_item, lex}; -use super::Interpreter; +use super::{foreign, Interpreter}; pub(crate) mod builtin_helpers; @@ -57,6 +57,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { let interner = &mut self.elaborator.interner; let call_stack = &self.elaborator.interpreter_call_stack; match name { + "apply_range_constraint" => foreign::apply_range_constraint(arguments, location), "array_as_str_unchecked" => array_as_str_unchecked(interner, arguments, location), "array_len" => array_len(interner, arguments, location), "assert_constant" => Ok(Value::Bool(true)), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs index d1ab6a1dabd..3de72969cab 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs @@ -1,4 +1,6 @@ -use acvm::blackbox_solver::BlackBoxFunctionSolver; +use acvm::{ + acir::BlackBoxFunc, blackbox_solver::BlackBoxFunctionSolver, AcirField, BlackBoxResolutionError, +}; use bn254_blackbox_solver::Bn254BlackBoxSolver; use im::Vector; use iter_extended::try_vecmap; @@ -29,6 +31,28 @@ pub(super) fn call_foreign( } } +pub(super) fn apply_range_constraint( + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let (value, num_bits) = check_two_arguments(arguments, location)?; + + let input = get_field(value)?; + let num_bits = get_u32(num_bits)?; + + if input.num_bits() < num_bits { + Ok(Value::Unit) + } else { + Err(InterpreterError::BlackBoxError( + BlackBoxResolutionError::Failed( + BlackBoxFunc::RANGE, + "value exceeds range check bounds".to_owned(), + ), + location, + )) + } +} + // poseidon2_permutation(_input: [Field; N], _state_length: u32) -> [Field; N] fn poseidon2_permutation( interner: &mut NodeInterner, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index a373441b4e0..bae57daae15 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -97,9 +97,9 @@ pub fn collect_defs( errors.extend(collector.collect_functions(context, ast.functions, crate_id)); - collector.collect_trait_impls(context, ast.trait_impls, crate_id); + errors.extend(collector.collect_trait_impls(context, ast.trait_impls, crate_id)); - collector.collect_impls(context, ast.impls, crate_id); + errors.extend(collector.collect_impls(context, ast.impls, crate_id)); collector.collect_attributes( ast.inner_attributes, @@ -163,7 +163,13 @@ impl<'a> ModCollector<'a> { errors } - fn collect_impls(&mut self, context: &mut Context, impls: Vec, krate: CrateId) { + fn collect_impls( + &mut self, + context: &mut Context, + impls: Vec, + krate: CrateId, + ) -> Vec<(CompilationError, FileId)> { + let mut errors = Vec::new(); let module_id = ModuleId { krate, local_id: self.module_id }; for r#impl in impls { @@ -173,8 +179,11 @@ impl<'a> ModCollector<'a> { r#impl, self.file_id, module_id, + &mut errors, ); } + + errors } fn collect_trait_impls( @@ -182,7 +191,9 @@ impl<'a> ModCollector<'a> { context: &mut Context, impls: Vec, krate: CrateId, - ) { + ) -> Vec<(CompilationError, FileId)> { + let mut errors = Vec::new(); + for mut trait_impl in impls { let trait_name = trait_impl.trait_name.clone(); @@ -198,6 +209,13 @@ impl<'a> ModCollector<'a> { let module = ModuleId { krate, local_id: self.module_id }; for (_, func_id, noir_function) in &mut unresolved_functions.functions { + if noir_function.def.attributes.is_test_function() { + let error = DefCollectorErrorKind::TestOnAssociatedFunction { + span: noir_function.name_ident().span(), + }; + errors.push((error.into(), self.file_id)); + } + let location = Location::new(noir_function.def.span, self.file_id); context.def_interner.push_function(*func_id, &noir_function.def, module, location); } @@ -224,6 +242,8 @@ impl<'a> ModCollector<'a> { self.def_collector.items.trait_impls.push(unresolved_trait_impl); } + + errors } fn collect_functions( @@ -1051,6 +1071,7 @@ pub fn collect_impl( r#impl: TypeImpl, file_id: FileId, module_id: ModuleId, + errors: &mut Vec<(CompilationError, FileId)>, ) { let mut unresolved_functions = UnresolvedFunctions { file_id, functions: Vec::new(), trait_id: None, self_type: None }; @@ -1058,6 +1079,15 @@ pub fn collect_impl( for (method, _) in r#impl.methods { let doc_comments = method.doc_comments; let mut method = method.item; + + if method.def.attributes.is_test_function() { + let error = DefCollectorErrorKind::TestOnAssociatedFunction { + span: method.name_ident().span(), + }; + errors.push((error.into(), file_id)); + continue; + } + let func_id = interner.push_empty_fn(); method.def.where_clause.extend(r#impl.where_clause.clone()); let location = Location::new(method.span(), file_id); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/errors.rs index d72f493092d..c08b4ff2062 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -82,6 +82,8 @@ pub enum DefCollectorErrorKind { }, #[error("{0}")] UnsupportedNumericGenericType(#[from] UnsupportedNumericGenericType), + #[error("The `#[test]` attribute may only be used on a non-associated function")] + TestOnAssociatedFunction { span: Span }, } impl DefCollectorErrorKind { @@ -291,6 +293,12 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { diag } DefCollectorErrorKind::UnsupportedNumericGenericType(err) => err.into(), + DefCollectorErrorKind::TestOnAssociatedFunction { span } => Diagnostic::simple_error( + "The `#[test]` attribute is disallowed on `impl` methods".into(), + String::new(), + *span, + ), + } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index f72741721d8..20a5bac49f6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -3692,3 +3692,51 @@ fn allows_struct_with_generic_infix_type_as_main_input_3() { "#; assert_no_errors(src); } + +#[test] +fn disallows_test_attribute_on_impl_method() { + let src = r#" + pub struct Foo {} + impl Foo { + #[test] + fn foo() {} + } + + fn main() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + errors[0].0, + CompilationError::DefinitionError(DefCollectorErrorKind::TestOnAssociatedFunction { + span: _ + }) + )); +} + +#[test] +fn disallows_test_attribute_on_trait_impl_method() { + let src = r#" + pub trait Trait { + fn foo() {} + } + + pub struct Foo {} + impl Trait for Foo { + #[test] + fn foo() {} + } + + fn main() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + errors[0].0, + CompilationError::DefinitionError(DefCollectorErrorKind::TestOnAssociatedFunction { + span: _ + }) + )); +} diff --git a/noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/Nargo.toml b/noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/Nargo.toml new file mode 100644 index 00000000000..e14cfeae5d6 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "comptime_apply_failing_range_constraint" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/src/main.nr b/noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/src/main.nr new file mode 100644 index 00000000000..752f13a2e51 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_failure/comptime_apply_failing_range_constraint/src/main.nr @@ -0,0 +1,5 @@ +fn main() { + comptime { + 256.assert_max_bit_size::<8>() + } +} diff --git a/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr index 5fc193150db..29690ba36c7 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr @@ -13,7 +13,7 @@ comptime fn attr_with_args(s: StructDefinition, a: Field, b: Field) { #[varargs] comptime fn attr_with_varargs(s: StructDefinition, t: [Field]) { - let _: StructDefinition = s; + let _ = s; for _ in t {} assert(t.len() < 5); } diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/Nargo.toml new file mode 100644 index 00000000000..bfd6fa75728 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/Nargo.toml @@ -0,0 +1,5 @@ +[package] +name = "comptime_apply_range_constraint" +type = "bin" +authors = [""] +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/src/main.nr new file mode 100644 index 00000000000..ff5e0ba9511 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_apply_range_constraint/src/main.nr @@ -0,0 +1,5 @@ +fn main() { + comptime { + 2.assert_max_bit_size::<8>() + } +} diff --git a/noir/noir-repo/tooling/nargo_cli/Cargo.toml b/noir/noir-repo/tooling/nargo_cli/Cargo.toml index 317706bb237..02e669f5c68 100644 --- a/noir/noir-repo/tooling/nargo_cli/Cargo.toml +++ b/noir/noir-repo/tooling/nargo_cli/Cargo.toml @@ -78,6 +78,7 @@ dirs.workspace = true assert_cmd = "2.0.8" assert_fs = "1.0.10" predicates = "2.1.5" +file-lock = "2.1.11" fm.workspace = true criterion.workspace = true pprof.workspace = true diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index 438eef687b8..ce46a717113 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -59,6 +59,12 @@ const IGNORED_BRILLIG_TESTS: [&str; 11] = [ "is_unconstrained", ]; +/// Tests which aren't expected to work with the default inliner cases. +const INLINER_MIN_OVERRIDES: [(&str, i64); 1] = [ + // 0 works if PoseidonHasher::write is tagged as `inline_always`, otherwise 22. + ("eddsa", 0), +]; + /// Some tests are expected to have warnings /// These should be fixed and removed from this list. const TESTS_WITH_EXPECTED_WARNINGS: [&str; 2] = [ @@ -88,22 +94,119 @@ fn read_test_cases( }) } -fn generate_test_case( +#[derive(Default)] +struct MatrixConfig { + // Only used with execution, and only on selected tests. + vary_brillig: bool, + // Only seems to have an effect on the `execute_success` cases. + vary_inliner: bool, + // If there is a non-default minimum inliner aggressiveness to use with the brillig tests. + min_inliner: i64, +} + +// Enum to be able to preserve readable test labels and also compare to numbers. +enum Inliner { + Min, + Default, + Max, + Custom(i64), +} + +impl Inliner { + fn value(&self) -> i64 { + match self { + Inliner::Min => i64::MIN, + Inliner::Default => 0, + Inliner::Max => i64::MAX, + Inliner::Custom(i) => *i, + } + } + fn label(&self) -> String { + match self { + Inliner::Min => "i64::MIN".to_string(), + Inliner::Default => "0".to_string(), + Inliner::Max => "i64::MAX".to_string(), + Inliner::Custom(i) => i.to_string(), + } + } +} + +/// Generate all test cases for a given test name (expected to be unique for the test directory), +/// based on the matrix configuration. These will be executed serially, but concurrently with +/// other test directories. Running multiple tests on the same directory would risk overriding +/// each others compilation artifacts, which is why this method injects a mutex shared by +/// all cases in the test matrix, as long as the test name and directory has a 1-to-1 relationship. +fn generate_test_cases( test_file: &mut File, test_name: &str, test_dir: &std::path::Display, + test_command: &str, test_content: &str, + matrix_config: &MatrixConfig, ) { + let brillig_cases = if matrix_config.vary_brillig { vec![false, true] } else { vec![false] }; + let inliner_cases = if matrix_config.vary_inliner { + let mut cases = vec![Inliner::Min, Inliner::Default, Inliner::Max]; + if !cases.iter().any(|c| c.value() == matrix_config.min_inliner) { + cases.push(Inliner::Custom(matrix_config.min_inliner)); + } + cases + } else { + vec![Inliner::Max] + }; + + // We can't use a `#[test_matrix(brillig_cases, inliner_cases)` if we only want to limit the + // aggressiveness range for the brillig tests, and let them go full range on the ACIR case. + let mut test_cases = Vec::new(); + for brillig in &brillig_cases { + for inliner in &inliner_cases { + if *brillig && inliner.value() < matrix_config.min_inliner { + continue; + } + test_cases.push(format!("#[test_case::test_case({brillig}, {})]", inliner.label())); + } + } + let test_cases = test_cases.join("\n"); + + // We need to isolate test cases in the same group, otherwise they overwrite each other's artifacts. + // On CI we use `cargo nextest`, which runs tests in different processes; for this we use a file lock. + // Locally we might be using `cargo test`, which run tests in the same process; in this case the file lock + // wouldn't work, becuase the process itself has the lock, and it looks like it can have N instances without + // any problems; for this reason we also use a `Mutex`. + let mutex_name = format! {"TEST_MUTEX_{}", test_name.to_uppercase()}; write!( test_file, r#" -#[test] -fn test_{test_name}() {{ +lazy_static::lazy_static! {{ + /// Prevent concurrent tests in the matrix from overwriting the compilation artifacts in {test_dir} + static ref {mutex_name}: std::sync::Mutex<()> = std::sync::Mutex::new(()); +}} + +{test_cases} +fn test_{test_name}(force_brillig: bool, inliner_aggressiveness: i64) {{ let test_program_dir = PathBuf::from("{test_dir}"); + // Ignore poisoning errors if some of the matrix cases failed. + let mutex_guard = {mutex_name}.lock().unwrap_or_else(|e| e.into_inner()); + + let file_guard = file_lock::FileLock::lock( + test_program_dir.join("Nargo.toml"), + true, + file_lock::FileOptions::new().read(true).write(true).append(true) + ).expect("failed to lock Nargo.toml"); + let mut nargo = Command::cargo_bin("nargo").unwrap(); nargo.arg("--program-dir").arg(test_program_dir); + nargo.arg("{test_command}").arg("--force"); + nargo.arg("--inliner-aggressiveness").arg(inliner_aggressiveness.to_string()); + if force_brillig {{ + nargo.arg("--force-brillig"); + }} + {test_content} + + drop(file_guard); + drop(mutex_guard); }} "# ) @@ -124,27 +227,24 @@ fn generate_execution_success_tests(test_file: &mut File, test_data_dir: &Path) for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "execute", r#" - nargo.arg("execute").arg("--force"); - - nargo.assert().success();"#, + nargo.assert().success(); + "#, + &MatrixConfig { + vary_brillig: !IGNORED_BRILLIG_TESTS.contains(&test_name.as_str()), + vary_inliner: false, + min_inliner: INLINER_MIN_OVERRIDES + .iter() + .find(|(n, _)| *n == test_name.as_str()) + .map(|(_, i)| *i) + .unwrap_or(i64::MIN), + }, ); - - if !IGNORED_BRILLIG_TESTS.contains(&test_name.as_str()) { - generate_test_case( - test_file, - &format!("{test_name}_brillig"), - &test_dir, - r#" - nargo.arg("execute").arg("--force").arg("--force-brillig"); - - nargo.assert().success();"#, - ); - } } writeln!(test_file, "}}").unwrap(); } @@ -163,14 +263,15 @@ fn generate_execution_failure_tests(test_file: &mut File, test_data_dir: &Path) for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "execute", r#" - nargo.arg("execute").arg("--force"); - - nargo.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not());"#, + nargo.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not()); + "#, + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap(); @@ -190,14 +291,15 @@ fn generate_noir_test_success_tests(test_file: &mut File, test_data_dir: &Path) for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "test", r#" - nargo.arg("test"); - - nargo.assert().success();"#, + nargo.assert().success(); + "#, + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap(); @@ -216,14 +318,15 @@ fn generate_noir_test_failure_tests(test_file: &mut File, test_data_dir: &Path) .unwrap(); for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "test", r#" - nargo.arg("test"); - - nargo.assert().failure();"#, + nargo.assert().failure(); + "#, + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap(); @@ -266,16 +369,18 @@ fn generate_compile_success_empty_tests(test_file: &mut File, test_data_dir: &Pa assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0); "#; - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "info", &format!( r#" - nargo.arg("info").arg("--json").arg("--force"); - - {assert_zero_opcodes}"#, + nargo.arg("--json"); + {assert_zero_opcodes} + "#, ), + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap(); @@ -295,13 +400,15 @@ fn generate_compile_success_contract_tests(test_file: &mut File, test_data_dir: for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "compile", r#" - nargo.arg("compile").arg("--force"); - nargo.assert().success().stderr(predicate::str::contains("warning:").not());"#, + nargo.assert().success().stderr(predicate::str::contains("warning:").not()); + "#, + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap(); @@ -322,13 +429,15 @@ fn generate_compile_success_no_bug_tests(test_file: &mut File, test_data_dir: &P for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, + "compile", r#" - nargo.arg("compile").arg("--force"); - nargo.assert().success().stderr(predicate::str::contains("bug:").not());"#, + nargo.assert().success().stderr(predicate::str::contains("bug:").not()); + "#, + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap(); @@ -348,13 +457,15 @@ fn generate_compile_failure_tests(test_file: &mut File, test_data_dir: &Path) { for (test_name, test_dir) in test_cases { let test_dir = test_dir.display(); - generate_test_case( + generate_test_cases( test_file, &test_name, &test_dir, - r#"nargo.arg("compile").arg("--force"); - - nargo.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not());"#, + "compile", + r#" + nargo.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not()); + "#, + &MatrixConfig::default(), ); } writeln!(test_file, "}}").unwrap();