Skip to content

Commit

Permalink
Use the DefaultForeignCallBuilder where its mostly None we're passing
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Dec 19, 2024
1 parent 20fa935 commit c8e31c7
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 62 deletions.
5 changes: 3 additions & 2 deletions tooling/acvm_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use nargo::PrintOutput;

use crate::cli::fs::inputs::{read_bytecode_from_file, read_inputs_from_file};
use crate::errors::CliError;
use nargo::{foreign_calls::DefaultForeignCallExecutor, ops::execute_program};
use nargo::{foreign_calls::DefaultForeignCallBuilder, ops::execute_program};

use super::fs::witness::{create_output_witness_string, save_witness_to_dir};

Expand Down Expand Up @@ -74,7 +74,8 @@ pub(crate) fn execute_program_from_witness(
&program,
inputs_map,
&Bn254BlackBoxSolver,
&mut DefaultForeignCallExecutor::new(PrintOutput::Stdout, None, None, None),
&mut DefaultForeignCallBuilder { output: PrintOutput::Stdout, ..Default::default() }
.build(),
)
.map_err(CliError::CircuitExecutionError)
}
7 changes: 2 additions & 5 deletions tooling/debugger/src/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ use acvm::{
AcirField, FieldElement,
};
use nargo::{
foreign_calls::{
layers::{Layer, Layering},
DefaultForeignCallExecutor, ForeignCallExecutor,
},
foreign_calls::{layers::Layer, DefaultForeignCallBuilder, ForeignCallExecutor},
PrintOutput,
};
use noirc_artifacts::debug::{DebugArtifact, DebugVars, StackFrame};
Expand Down Expand Up @@ -57,7 +54,7 @@ impl DefaultDebugForeignCallExecutor {
output: PrintOutput<'_>,
ex: DefaultDebugForeignCallExecutor,
) -> impl DebugForeignCallExecutor + '_ {
DefaultForeignCallExecutor::new(output, None, None, None).add_layer(ex)
DefaultForeignCallBuilder::default().with_output(output).build().add_layer(ex)
}

#[allow(clippy::new_ret_no_self, dead_code)]
Expand Down
14 changes: 7 additions & 7 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::future::{self, Future};
use crate::insert_all_files_for_workspace_into_file_manager;
use async_lsp::{ErrorCode, ResponseError};
use nargo::{
foreign_calls::DefaultForeignCallExecutor,
foreign_calls::DefaultForeignCallBuilder,
ops::{run_test, TestStatus},
PrintOutput,
};
Expand Down Expand Up @@ -91,13 +91,13 @@ fn on_test_run_request_inner(
PrintOutput::Stdout,
&CompileOptions::default(),
|output, base| {
DefaultForeignCallExecutor::with_base(
base,
DefaultForeignCallBuilder {
output,
None,
Some(workspace.root_dir.clone()),
Some(package.name.to_string()),
)
resolver_url: None, // NB without this the root and package don't do anything.
root_path: Some(workspace.root_dir.clone()),
package_name: Some(package.name.to_string()),
}
.build_with_base(base)
},
);
let result = match test_result {
Expand Down
36 changes: 15 additions & 21 deletions tooling/nargo/src/foreign_calls/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ use super::{
#[cfg(feature = "rpc")]
use super::rpc::RPCForeignCallExecutor;

/// A configuration where we can enable fields based on feature flags,
/// which is easier than providing different overrides for `new`.
/// A builder for [DefaultForeignCallLayers] where we can enable fields based on feature flags,
/// which is easier than providing different overrides for a `new` method.
#[derive(Default)]
pub struct DefaultForeignCallConfig<'a> {
pub struct DefaultForeignCallBuilder<'a> {
pub output: PrintOutput<'a>,
#[cfg(feature = "rpc")]
pub resolver_url: Option<String>,
Expand All @@ -26,14 +26,22 @@ pub struct DefaultForeignCallConfig<'a> {
pub package_name: Option<String>,
}

impl<'a> DefaultForeignCallConfig<'a> {
impl<'a> DefaultForeignCallBuilder<'a> {
/// Override the output.
pub fn with_output(mut self, output: PrintOutput<'a>) -> Self {
self.output = output;
self
}

/// Compose the executor layers with [layers::Empty] as the default handler.
pub fn build<F>(self) -> DefaultForeignCallLayers<'a, layers::Empty, F>
where
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
{
self.build_with_base(layers::Empty)
}

/// Compose the executor layers with `base` as the default handler.
pub fn build_with_base<B, F>(self, base: B) -> DefaultForeignCallLayers<'a, B, F>
where
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
Expand Down Expand Up @@ -83,7 +91,7 @@ pub struct DefaultForeignCallExecutor;
/// Convenience constructors for the RPC case. Non-RPC versions are not provided
/// because once a crate opts into this within the workspace, everyone gets it
/// even if they don't want to. For the non-RPC case we can nudge people to
/// use `DefaultForeignCallConfig` which is easier to keep flexible.
/// use `DefaultForeignCallBuilder` which is easier to keep flexible.
#[cfg(feature = "rpc")]
impl DefaultForeignCallExecutor {
#[allow(clippy::new_ret_no_self)]
Expand All @@ -96,26 +104,12 @@ impl DefaultForeignCallExecutor {
where
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
{
Self::with_base(layers::Empty, output, resolver_url, root_path, package_name)
}

pub fn with_base<'a, F, B>(
base: B,
output: PrintOutput<'a>,
resolver_url: Option<&str>,
root_path: Option<std::path::PathBuf>,
package_name: Option<String>,
) -> DefaultForeignCallLayers<'a, B, F>
where
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
B: ForeignCallExecutor<F> + 'a,
{
DefaultForeignCallConfig {
DefaultForeignCallBuilder {
output,
resolver_url: resolver_url.map(|s| s.to_string()),
root_path,
package_name,
}
.build_with_base(base)
.build()
}
}
1 change: 1 addition & 0 deletions tooling/nargo/src/foreign_calls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub mod print;
pub mod default;
#[cfg(feature = "rpc")]
pub mod rpc;
pub use default::DefaultForeignCallBuilder;
#[cfg(feature = "rpc")]
pub use default::DefaultForeignCallExecutor;

Expand Down
3 changes: 1 addition & 2 deletions tooling/nargo_cli/benches/criterion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use acvm::{acir::native_types::WitnessMap, FieldElement};
use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt};
use criterion::{criterion_group, criterion_main, Criterion};

use nargo::PrintOutput;
use noirc_abi::{
input_parser::{Format, InputValue},
Abi, InputMap,
Expand Down Expand Up @@ -116,7 +115,7 @@ fn criterion_test_execution(c: &mut Criterion, test_program_dir: &Path, force_br
let artifacts = RefCell::new(None);

let mut foreign_call_executor =
nargo::foreign_calls::DefaultForeignCallExecutor::new(PrintOutput::None, None, None, None);
nargo::foreign_calls::DefaultForeignCallBuilder::default().build();

c.bench_function(&benchmark_name, |b| {
b.iter_batched(
Expand Down
5 changes: 2 additions & 3 deletions tooling/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use bn254_blackbox_solver::Bn254BlackBoxSolver;
use clap::Args;
use iter_extended::vecmap;
use nargo::{
constants::PROVER_INPUT_FILE, foreign_calls::DefaultForeignCallExecutor, package::Package,
PrintOutput,
constants::PROVER_INPUT_FILE, foreign_calls::DefaultForeignCallBuilder, package::Package,
};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml};
use noirc_abi::input_parser::Format;
Expand Down Expand Up @@ -255,7 +254,7 @@ fn profile_brillig_execution(
&program_artifact.bytecode,
initial_witness,
&Bn254BlackBoxSolver,
&mut DefaultForeignCallExecutor::new(PrintOutput::None, None, None, None),
&mut DefaultForeignCallBuilder::default().build(),
)?;

let expression_width = get_target_width(package.expression_width, expression_width);
Expand Down
14 changes: 7 additions & 7 deletions tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use clap::Args;
use fm::FileManager;
use formatters::{Formatter, JsonFormatter, PrettyFormatter, TerseFormatter};
use nargo::{
foreign_calls::DefaultForeignCallExecutor, insert_all_files_for_workspace_into_file_manager,
foreign_calls::DefaultForeignCallBuilder, insert_all_files_for_workspace_into_file_manager,
ops::TestStatus, package::Package, parse_all, prepare_package, workspace::Workspace,
PrintOutput,
};
Expand Down Expand Up @@ -497,13 +497,13 @@ impl<'a> TestRunner<'a> {
PrintOutput::String(&mut output_string),
&self.args.compile_options,
|output, base| {
DefaultForeignCallExecutor::with_base(
base,
DefaultForeignCallBuilder {
output,
foreign_call_resolver_url,
root_path.clone(),
Some(package_name.clone()),
)
resolver_url: foreign_call_resolver_url.map(|s| s.to_string()),
root_path: root_path.clone(),
package_name: Some(package_name.clone()),
}
.build_with_base(base)
},
);
(test_status, output_string)
Expand Down
7 changes: 2 additions & 5 deletions tooling/nargo_cli/tests/stdlib-props.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ use std::{cell::RefCell, collections::BTreeMap, path::Path};

use acvm::{acir::native_types::WitnessStack, AcirField, FieldElement};
use iter_extended::vecmap;
use nargo::{
foreign_calls::DefaultForeignCallExecutor, ops::execute_program, parse_all, PrintOutput,
};
use nargo::{foreign_calls::DefaultForeignCallBuilder, ops::execute_program, parse_all};
use noirc_abi::input_parser::InputValue;
use noirc_driver::{
compile_main, file_manager_with_stdlib, prepare_crate, CompilationResult, CompileOptions,
Expand Down Expand Up @@ -81,8 +79,7 @@ fn run_snippet_proptest(
};

let blackbox_solver = bn254_blackbox_solver::Bn254BlackBoxSolver;
let foreign_call_executor =
RefCell::new(DefaultForeignCallExecutor::new(PrintOutput::None, None, None, None));
let foreign_call_executor = RefCell::new(DefaultForeignCallBuilder::default().build());

// Generate multiple input/output
proptest!(ProptestConfig::with_cases(100), |(io in strategy)| {
Expand Down
10 changes: 2 additions & 8 deletions tooling/nargo_cli/tests/stdlib-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![allow(clippy::items_after_test_module)]
use clap::Parser;
use fm::FileManager;
use nargo::foreign_calls::DefaultForeignCallExecutor;
use nargo::foreign_calls::DefaultForeignCallBuilder;
use nargo::PrintOutput;
use noirc_driver::{check_crate, file_manager_with_stdlib, CompileOptions};
use noirc_frontend::hir::FunctionNameMatch;
Expand Down Expand Up @@ -91,13 +91,7 @@ fn run_stdlib_tests(force_brillig: bool, inliner_aggressiveness: i64) {
PrintOutput::Stdout,
&CompileOptions { force_brillig, inliner_aggressiveness, ..Default::default() },
|output, base| {
DefaultForeignCallExecutor::with_base(
base,
output,
None,
Some(dummy_package.root_dir.clone()),
Some(dummy_package.name.to_string()),
)
DefaultForeignCallBuilder { output, ..Default::default() }.build_with_base(base)
},
);
(test_name, status)
Expand Down
4 changes: 2 additions & 2 deletions tooling/profiler/src/cli/execution_flamegraph_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ use std::path::{Path, PathBuf};
use acir::circuit::OpcodeLocation;
use clap::Args;
use color_eyre::eyre::{self, Context};
use nargo::foreign_calls::DefaultForeignCallBuilder;
use nargo::PrintOutput;

use crate::flamegraph::{BrilligExecutionSample, FlamegraphGenerator, InfernoFlamegraphGenerator};
use crate::fs::{read_inputs_from_file, read_program_from_file};
use crate::opcode_formatter::format_brillig_opcode;
use bn254_blackbox_solver::Bn254BlackBoxSolver;
use nargo::foreign_calls::DefaultForeignCallExecutor;
use noirc_abi::input_parser::Format;
use noirc_artifacts::debug::DebugArtifact;

Expand Down Expand Up @@ -55,7 +55,7 @@ fn run_with_generator(
&program.bytecode,
initial_witness,
&Bn254BlackBoxSolver,
&mut DefaultForeignCallExecutor::new(PrintOutput::Stdout, None, None, None),
&mut DefaultForeignCallBuilder::default().with_output(PrintOutput::Stdout).build(),
)?;
println!("Executed");

Expand Down

0 comments on commit c8e31c7

Please sign in to comment.