Skip to content

Commit

Permalink
feat: several nargo test improvements (noir-lang/noir#6728)
Browse files Browse the repository at this point in the history
chore: Try replace callstack with a linked list (noir-lang/noir#6747)
chore: Use `NumericType` not `Type` for casts and numeric constants (noir-lang/noir#6769)
chore(ci): Extend compiler memory report to external repos (noir-lang/noir#6768)
chore(ci): Handle external libraries in compilation timing report (noir-lang/noir#6750)
feat(ssa): Implement missing brillig constraints SSA check (noir-lang/noir#6658)
fix: Do not merge expressions that contain output witnesses (noir-lang/noir#6757)
fix: parser would hand on function type with colon in it (noir-lang/noir#6764)
chore(docs): Update branding (noir-lang/noir#6759)
feat(cli): Run command on the package closest to the current directory (noir-lang/noir#6752)
chore: lock CI to use ubuntu 22.04 (noir-lang/noir#6755)
chore: free memory for silenced warnings early (noir-lang/noir#6748)
chore(stdlib)!: Remove Schnorr (noir-lang/noir#6749)
fix: Improve type error when indexing a variable of unknown type (noir-lang/noir#6744)
fix: println("{{}}") was printing "{{}}" instead of "{}" (noir-lang/noir#6745)
feat: `std::hint::black_box` function. (noir-lang/noir#6529)
feat(ci): Initial compilation report on test_programs (noir-lang/noir#6731)
chore: Cleanup unrolling pass (noir-lang/noir#6743)
fix: allow empty loop headers (noir-lang/noir#6736)
fix: map entry point indexes after all ssa passes (noir-lang/noir#6740)
chore: Update url to 2.5.4 (noir-lang/noir#6741)
feat: Order attribute execution by their source ordering (noir-lang/noir#6326)
feat(test): Check that `nargo::ops::transform_program` is idempotent (noir-lang/noir#6694)
feat: Sync from aztec-packages (noir-lang/noir#6730)
  • Loading branch information
AztecBot committed Dec 12, 2024
2 parents 2abfafa + 22818a6 commit 5c54d74
Show file tree
Hide file tree
Showing 20 changed files with 527 additions and 264 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
f6957faf50025afc053a32d86398d0823253066b
1b0dd4149d9249f0ea4fb5e2228c688e0135618f
12 changes: 4 additions & 8 deletions noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,19 @@ pub fn compile<F: AcirField>(
acir: Circuit<F>,
expression_width: ExpressionWidth,
) -> (Circuit<F>, AcirTransformationMap) {
let acir_opcode_positions = (0..acir.opcodes.len()).collect::<Vec<_>>();

if MAX_OPTIMIZER_PASSES == 0 {
return (acir, AcirTransformationMap::new(&acir_opcode_positions));
let acir_opcode_positions = (0..acir.opcodes.len()).collect::<Vec<_>>();
let transformation_map = AcirTransformationMap::new(&acir_opcode_positions);
return (acir, transformation_map);
}

let mut pass = 0;
let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes);
let mut prev_acir = acir;
let mut prev_acir_opcode_positions = acir_opcode_positions;

// For most test programs it would be enough to only loop `transform_internal`,
// but some of them don't stabilize unless we also repeat the backend agnostic optimizations.
let (mut acir, acir_opcode_positions) = loop {
let (acir, acir_opcode_positions) =
optimize_internal(prev_acir, prev_acir_opcode_positions);
let (acir, acir_opcode_positions) = optimize_internal(prev_acir);

// Stop if we have already done at least one transform and an extra optimization changed nothing.
if pass > 0 && prev_opcodes_hash == fxhash::hash64(&acir.opcodes) {
Expand All @@ -117,7 +114,6 @@ pub fn compile<F: AcirField>(
pass += 1;
prev_acir = acir;
prev_opcodes_hash = opcodes_hash;
prev_acir_opcode_positions = acir_opcode_positions;
};

let transformation_map = AcirTransformationMap::new(&acir_opcode_positions);
Expand Down
2 changes: 0 additions & 2 deletions noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ pub fn optimize<F: AcirField>(acir: Circuit<F>) -> (Circuit<F>, AcirTransformati
// by applying the modifications done to the circuit opcodes and also to the opcode_positions (delete and insert)
let acir_opcode_positions = (0..acir.opcodes.len()).collect();

let (mut acir, new_opcode_positions) = optimize_internal(acir, acir_opcode_positions);

let transformation_map = AcirTransformationMap::new(&new_opcode_positions);

acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);
Expand Down
3 changes: 2 additions & 1 deletion noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use acir::native_types::{WitnessMap, WitnessStack};
use acir::FieldElement;
use bn254_blackbox_solver::Bn254BlackBoxSolver;
use clap::Args;
use nargo::PrintOutput;

use crate::cli::fs::inputs::{read_bytecode_from_file, read_inputs_from_file};
use crate::errors::CliError;
Expand Down Expand Up @@ -73,7 +74,7 @@ pub(crate) fn execute_program_from_witness(
&program,
inputs_map,
&Bn254BlackBoxSolver,
&mut DefaultForeignCallExecutor::new(true, None, None, None),
&mut DefaultForeignCallExecutor::new(PrintOutput::Stdout, None, None, None),
)
.map_err(CliError::CircuitExecutionError)
}
15 changes: 10 additions & 5 deletions noir/noir-repo/tooling/debugger/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ mod tests {
BinaryFieldOp, HeapValueType, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray,
},
};
use nargo::PrintOutput;

#[test]
fn test_resolve_foreign_calls_stepping_into_brillig() {
Expand Down Expand Up @@ -1025,8 +1026,10 @@ mod tests {

let initial_witness = BTreeMap::from([(Witness(1), fe_1)]).into();

let foreign_call_executor =
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact));
let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(
PrintOutput::Stdout,
debug_artifact,
));
let mut context = DebugContext::new(
&StubbedBlackBoxSolver,
circuits,
Expand Down Expand Up @@ -1190,8 +1193,10 @@ mod tests {

let initial_witness = BTreeMap::from([(Witness(1), fe_1), (Witness(2), fe_1)]).into();

let foreign_call_executor =
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact));
let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(
PrintOutput::Stdout,
debug_artifact,
));
let brillig_funcs = &vec![brillig_bytecode];
let mut context = DebugContext::new(
&StubbedBlackBoxSolver,
Expand Down Expand Up @@ -1285,7 +1290,7 @@ mod tests {
&circuits,
&debug_artifact,
WitnessMap::new(),
Box::new(DefaultDebugForeignCallExecutor::new(true)),
Box::new(DefaultDebugForeignCallExecutor::new(PrintOutput::Stdout)),
brillig_funcs,
);

Expand Down
6 changes: 5 additions & 1 deletion noir/noir-repo/tooling/debugger/src/dap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use acvm::acir::circuit::brillig::BrilligBytecode;
use acvm::acir::circuit::Circuit;
use acvm::acir::native_types::WitnessMap;
use acvm::{BlackBoxFunctionSolver, FieldElement};
use nargo::PrintOutput;

use crate::context::DebugContext;
use crate::context::{DebugCommandResult, DebugLocation};
Expand Down Expand Up @@ -71,7 +72,10 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver<FieldElement>> DapSession<
circuits,
debug_artifact,
initial_witness,
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)),
Box::new(DefaultDebugForeignCallExecutor::from_artifact(
PrintOutput::Stdout,
debug_artifact,
)),
unconstrained_functions,
);
Self {
Expand Down
23 changes: 13 additions & 10 deletions noir/noir-repo/tooling/debugger/src/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use acvm::{
pwg::ForeignCallWaitInfo,
AcirField, FieldElement,
};
use nargo::foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor};
use nargo::{
foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor},
PrintOutput,
};
use noirc_artifacts::debug::{DebugArtifact, DebugVars, StackFrame};
use noirc_errors::debug_info::{DebugFnId, DebugVarId};
use noirc_printable_type::ForeignCallError;
Expand Down Expand Up @@ -41,21 +44,21 @@ pub trait DebugForeignCallExecutor: ForeignCallExecutor<FieldElement> {
fn current_stack_frame(&self) -> Option<StackFrame<FieldElement>>;
}

pub struct DefaultDebugForeignCallExecutor {
executor: DefaultForeignCallExecutor<FieldElement>,
pub struct DefaultDebugForeignCallExecutor<'a> {
executor: DefaultForeignCallExecutor<'a, FieldElement>,
pub debug_vars: DebugVars<FieldElement>,
}

impl DefaultDebugForeignCallExecutor {
pub fn new(show_output: bool) -> Self {
impl<'a> DefaultDebugForeignCallExecutor<'a> {
pub fn new(output: PrintOutput<'a>) -> Self {
Self {
executor: DefaultForeignCallExecutor::new(show_output, None, None, None),
executor: DefaultForeignCallExecutor::new(output, None, None, None),
debug_vars: DebugVars::default(),
}
}

pub fn from_artifact(show_output: bool, artifact: &DebugArtifact) -> Self {
let mut ex = Self::new(show_output);
pub fn from_artifact(output: PrintOutput<'a>, artifact: &DebugArtifact) -> Self {
let mut ex = Self::new(output);
ex.load_artifact(artifact);
ex
}
Expand All @@ -70,7 +73,7 @@ impl DefaultDebugForeignCallExecutor {
}
}

impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor {
impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor<'_> {
fn get_variables(&self) -> Vec<StackFrame<FieldElement>> {
self.debug_vars.get_variables()
}
Expand All @@ -88,7 +91,7 @@ fn debug_fn_id(value: &FieldElement) -> DebugFnId {
DebugFnId(value.to_u128() as u32)
}

impl ForeignCallExecutor<FieldElement> for DefaultDebugForeignCallExecutor {
impl ForeignCallExecutor<FieldElement> for DefaultDebugForeignCallExecutor<'_> {
fn execute(
&mut self,
foreign_call: &ForeignCallWaitInfo<FieldElement>,
Expand Down
14 changes: 9 additions & 5 deletions noir/noir-repo/tooling/debugger/src/repl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use acvm::brillig_vm::brillig::Opcode as BrilligOpcode;
use acvm::brillig_vm::MemoryValue;
use acvm::AcirField;
use acvm::{BlackBoxFunctionSolver, FieldElement};
use nargo::NargoError;
use nargo::{NargoError, PrintOutput};
use noirc_driver::CompiledProgram;

use crate::foreign_calls::DefaultDebugForeignCallExecutor;
Expand Down Expand Up @@ -42,8 +42,10 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {
initial_witness: WitnessMap<FieldElement>,
unconstrained_functions: &'a [BrilligBytecode<FieldElement>],
) -> Self {
let foreign_call_executor =
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact));
let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(
PrintOutput::Stdout,
debug_artifact,
));
let context = DebugContext::new(
blackbox_solver,
circuits,
Expand Down Expand Up @@ -313,8 +315,10 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {

fn restart_session(&mut self) {
let breakpoints: Vec<DebugLocation> = self.context.iterate_breakpoints().copied().collect();
let foreign_call_executor =
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, self.debug_artifact));
let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(
PrintOutput::Stdout,
self.debug_artifact,
));
self.context = DebugContext::new(
self.blackbox_solver,
self.circuits,
Expand Down
7 changes: 5 additions & 2 deletions noir/noir-repo/tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ use std::future::{self, Future};

use crate::insert_all_files_for_workspace_into_file_manager;
use async_lsp::{ErrorCode, ResponseError};
use nargo::ops::{run_test, TestStatus};
use nargo::{
ops::{run_test, TestStatus},
PrintOutput,
};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING};
use noirc_frontend::hir::FunctionNameMatch;
Expand Down Expand Up @@ -84,7 +87,7 @@ fn on_test_run_request_inner(
&state.solver,
&mut context,
&test_function,
true,
PrintOutput::Stdout,
None,
Some(workspace.root_dir.clone()),
Some(package.name.to_string()),
Expand Down
24 changes: 9 additions & 15 deletions noir/noir-repo/tooling/nargo/src/foreign_calls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::PathBuf;
use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField};
use mocker::MockForeignCallExecutor;
use noirc_printable_type::ForeignCallError;
use print::PrintForeignCallExecutor;
use print::{PrintForeignCallExecutor, PrintOutput};
use rand::Rng;
use rpc::RPCForeignCallExecutor;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -65,22 +65,22 @@ impl ForeignCall {
}

#[derive(Debug, Default)]
pub struct DefaultForeignCallExecutor<F> {
pub struct DefaultForeignCallExecutor<'a, F> {
/// The executor for any [`ForeignCall::Print`] calls.
printer: Option<PrintForeignCallExecutor>,
printer: PrintForeignCallExecutor<'a>,
mocker: MockForeignCallExecutor<F>,
external: Option<RPCForeignCallExecutor>,
}

impl<F: Default> DefaultForeignCallExecutor<F> {
impl<'a, F: Default> DefaultForeignCallExecutor<'a, F> {
pub fn new(
show_output: bool,
output: PrintOutput<'a>,
resolver_url: Option<&str>,
root_path: Option<PathBuf>,
package_name: Option<String>,
) -> Self {
let id = rand::thread_rng().gen();
let printer = if show_output { Some(PrintForeignCallExecutor) } else { None };
let printer = PrintForeignCallExecutor { output };
let external_resolver = resolver_url.map(|resolver_url| {
RPCForeignCallExecutor::new(resolver_url, id, root_path, package_name)
});
Expand All @@ -92,22 +92,16 @@ impl<F: Default> DefaultForeignCallExecutor<F> {
}
}

impl<F: AcirField + Serialize + for<'a> Deserialize<'a>> ForeignCallExecutor<F>
for DefaultForeignCallExecutor<F>
impl<'a, F: AcirField + Serialize + for<'b> Deserialize<'b>> ForeignCallExecutor<F>
for DefaultForeignCallExecutor<'a, F>
{
fn execute(
&mut self,
foreign_call: &ForeignCallWaitInfo<F>,
) -> Result<ForeignCallResult<F>, ForeignCallError> {
let foreign_call_name = foreign_call.function.as_str();
match ForeignCall::lookup(foreign_call_name) {
Some(ForeignCall::Print) => {
if let Some(printer) = &mut self.printer {
printer.execute(foreign_call)
} else {
Ok(ForeignCallResult::default())
}
}
Some(ForeignCall::Print) => self.printer.execute(foreign_call),
Some(
ForeignCall::CreateMock
| ForeignCall::SetMockParams
Expand Down
22 changes: 19 additions & 3 deletions noir/noir-repo/tooling/nargo/src/foreign_calls/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,19 @@ use noirc_printable_type::{ForeignCallError, PrintableValueDisplay};
use super::{ForeignCall, ForeignCallExecutor};

#[derive(Debug, Default)]
pub(crate) struct PrintForeignCallExecutor;
pub enum PrintOutput<'a> {
#[default]
None,
Stdout,
String(&'a mut String),
}

#[derive(Debug, Default)]
pub(crate) struct PrintForeignCallExecutor<'a> {
pub(crate) output: PrintOutput<'a>,
}

impl<F: AcirField> ForeignCallExecutor<F> for PrintForeignCallExecutor {
impl<F: AcirField> ForeignCallExecutor<F> for PrintForeignCallExecutor<'_> {
fn execute(
&mut self,
foreign_call: &ForeignCallWaitInfo<F>,
Expand All @@ -26,7 +36,13 @@ impl<F: AcirField> ForeignCallExecutor<F> for PrintForeignCallExecutor {
let display_string =
format!("{display_values}{}", if skip_newline { "" } else { "\n" });

print!("{display_string}");
match &mut self.output {
PrintOutput::None => (),
PrintOutput::Stdout => print!("{display_string}"),
PrintOutput::String(string) => {
string.push_str(&display_string);
}
}

Ok(ForeignCallResult::default())
}
Expand Down
1 change: 1 addition & 0 deletions noir/noir-repo/tooling/nargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use rayon::prelude::*;
use walkdir::WalkDir;

pub use self::errors::NargoError;
pub use self::foreign_calls::print::PrintOutput;

pub fn prepare_dependencies(
context: &mut Context,
Expand Down
Loading

0 comments on commit 5c54d74

Please sign in to comment.