Skip to content

Commit

Permalink
Merge branch 'master' into remove-keys-from-artifact
Browse files Browse the repository at this point in the history
* master:
  chore: Reuse workspace target directory in wasm build script (#2312)
  feat(nargo): Add `--workspace` flag to run commands in every package (#2313)
  chore(frontend): Replace `ModuleOrigin` with `Location` on `ModuleData` (#2308)
  fix: Fix 3 parser test cases in parsing (#2284)
  fix: Require package names to be non-empty (#2293)
  fix(nargo)!: Remove `-p` short flag from the `--program-dir` flag (#2300)
  feat: optionally output a debug artifact on compile (#2260)
  chore: `nargo info` now prints information as a prettified table  (#2282)
  fix(lsp): Pass `--program-dir` to test command from codelens (#2292)
  fix(nargo): Allow `--program-dir` flag anywhere in a command (#2290)
  feat: Execute brillig opcodes with constant inputs at compile-time (#2190)
  feat: Add basic benchmarking (#2213)
  feat: Include struct names in ABIs (#2266)
  feat(nargo): Add `--exact` flag to `nargo test` (#2272)
  • Loading branch information
TomAFrench committed Aug 15, 2023
2 parents 40ed808 + 7d01789 commit d9236f4
Show file tree
Hide file tree
Showing 97 changed files with 1,242 additions and 242 deletions.
401 changes: 401 additions & 0 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion crates/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ impl From<&PathBuf> for PathString {
pub struct FileMap(SimpleFiles<PathString, String>);

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize)]
#[derive(
Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize, PartialOrd, Ord,
)]
pub struct FileId(usize);

impl FileId {
Expand Down
2 changes: 1 addition & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FileManager {
assert!(old_value.is_none(), "ice: the same path was inserted into the file manager twice");
}

pub fn fetch_file(&mut self, file_id: FileId) -> File {
pub fn fetch_file(&self, file_id: FileId) -> File {
// Unwrap as we ensure that all file_id's map to a corresponding file in the file map
self.file_map.get_file(file_id).unwrap()
}
Expand Down
19 changes: 14 additions & 5 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ use lsp_types::{
PublishDiagnosticsParams, Range, ServerCapabilities, TextDocumentSyncOptions,
};
use nargo::prepare_package;
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::check_crate;
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::hir::FunctionNameMatch;
use serde_json::Value as JsonValue;
use tower::Service;

Expand Down Expand Up @@ -155,7 +156,7 @@ fn on_code_lens_request(
return future::ready(Ok(None));
}
};
let workspace = match resolve_workspace_from_toml(&toml_path, None) {
let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) {
Ok(workspace) => workspace,
Err(err) => {
// If we found a manifest, but the workspace is invalid, we raise an error about it
Expand All @@ -176,7 +177,8 @@ fn on_code_lens_request(

let fm = &context.file_manager;
let files = fm.as_simple_files();
let tests = context.get_all_test_functions_in_crate_matching(&crate_id, "");
let tests = context
.get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Anything);

for (func_name, func_id) in tests {
let location = context.function_meta(&func_id).name.location;
Expand All @@ -196,7 +198,14 @@ fn on_code_lens_request(
let command = Command {
title: TEST_CODELENS_TITLE.into(),
command: TEST_COMMAND.into(),
arguments: Some(vec![func_name.into()]),
arguments: Some(vec![
"--program-dir".into(),
format!("{}", workspace.root_dir.display()).into(),
"--package".into(),
format!("{}", package.name).into(),
"--exact".into(),
func_name.into(),
]),
};

let lens = CodeLens { range, command: command.into(), data: None };
Expand Down Expand Up @@ -272,7 +281,7 @@ fn on_did_save_text_document(
return ControlFlow::Continue(());
}
};
let workspace = match resolve_workspace_from_toml(&toml_path, None) {
let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) {
Ok(workspace) => workspace,
Err(err) => {
// If we found a manifest, but the workspace is invalid, we raise an error about it
Expand Down
1 change: 1 addition & 0 deletions crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ acvm.workspace = true
fm.workspace = true
noirc_abi.workspace = true
noirc_driver.workspace = true
noirc_errors.workspace = true
noirc_frontend.workspace = true
iter-extended.workspace = true
serde.workspace = true
Expand Down
50 changes: 50 additions & 0 deletions crates/nargo/src/artifacts/debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use noirc_errors::debug_info::DebugInfo;
use serde::{Deserialize, Serialize};
use std::{
collections::{BTreeMap, BTreeSet},
path::PathBuf,
};

use fm::FileId;
use noirc_frontend::hir::Context;

/// For a given file, we store the source code and the path to the file
/// so consumers of the debug artifact can reconstruct the original source code structure.
#[derive(Debug, Serialize, Deserialize)]
pub struct DebugFile {
pub source: String,
pub path: PathBuf,
}

/// A Debug Artifact stores, for a given program, the debug info for every function
/// along with a map of file Id to the source code so locations in debug info can be mapped to source code they point to.
#[derive(Debug, Serialize, Deserialize)]
pub struct DebugArtifact {
pub debug_symbols: Vec<DebugInfo>,
pub file_map: BTreeMap<FileId, DebugFile>,
}

impl DebugArtifact {
pub fn new(debug_symbols: Vec<DebugInfo>, compilation_context: &Context) -> Self {
let mut file_map = BTreeMap::new();

let files_with_debug_symbols: BTreeSet<FileId> = debug_symbols
.iter()
.flat_map(|function_symbols| function_symbols.locations.values().map(|loc| loc.file))
.collect();

for file_id in files_with_debug_symbols {
let file_source = compilation_context.file_manager.fetch_file(file_id).source();

file_map.insert(
file_id,
DebugFile {
source: file_source.to_string(),
path: compilation_context.file_manager.path(file_id).to_path_buf(),
},
);
}

Self { debug_symbols, file_map }
}
}
1 change: 1 addition & 0 deletions crates/nargo/src/artifacts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use base64::Engine;
use serde::{Deserializer, Serializer};

pub mod contract;
pub mod debug;
pub mod program;

// TODO: move these down into ACVM.
Expand Down
20 changes: 18 additions & 2 deletions crates/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ acvm.workspace = true
toml.workspace = true
serde.workspace = true
serde_json.workspace = true
prettytable-rs = "0.10"
thiserror.workspace = true
tower.workspace = true
async-lsp = { version = "0.0.5", default-features = false, features = [
"client-monitor",
"stdio",
"tracing",
"tokio"
"tokio",
] }
const_format = "0.2.30"
hex = "0.4.2"
Expand All @@ -56,10 +57,25 @@ assert_cmd = "2.0.8"
assert_fs = "1.0.10"
predicates = "2.1.5"
fm.workspace = true
criterion = "0.5.0"
paste = "1.0.14"
pprof = { version = "0.12", features = [
"flamegraph",
"frame-pointer",
"criterion",
] }
iai = "0.1.1"

[[bench]]
name = "criterion"
harness = false

[[bench]]
name = "iai"
harness = false

[features]
default = ["plonk_bn254"]
# The plonk backend can only use bn254, so we do not specify the field
plonk_bn254 = ["acvm-backend-barretenberg/native"]
plonk_bn254_wasm = ["acvm-backend-barretenberg/wasm"]

33 changes: 33 additions & 0 deletions crates/nargo_cli/benches/criterion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//! Select representative tests to bench with criterion
use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt};
use criterion::{criterion_group, criterion_main, Criterion};
use paste::paste;
use pprof::criterion::{Output, PProfProfiler};
use std::process::Command;
include!("./utils.rs");

macro_rules! criterion_command {
($command_name:tt, $command_string:expr) => {
paste! {
fn [<criterion_selected_tests_ $command_name>](c: &mut Criterion) {
let test_program_dirs = get_selected_tests();
for test_program_dir in test_program_dirs {
let mut cmd = Command::cargo_bin("nargo").unwrap();
cmd.arg("--program-dir").arg(&test_program_dir);
cmd.arg($command_string);

c.bench_function(&format!("{}_{}", test_program_dir.file_name().unwrap().to_str().unwrap(), $command_string), |b| {
b.iter(|| cmd.assert())
});
}
}
}
};
}
criterion_command!(execution, "execute");
criterion_group! {
name = benches;
config = Criterion::default().sample_size(20).with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
targets = criterion_selected_tests_execution
}
criterion_main!(benches);
25 changes: 25 additions & 0 deletions crates/nargo_cli/benches/iai.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! Select representative tests to bench with iai
use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt};
use iai::black_box;
use paste::paste;
use std::process::Command;
include!("./utils.rs");

macro_rules! iai_command {
($command_name:tt, $command_string:expr) => {
paste! {
fn [<iai_selected_tests_ $command_name>]() {
let test_program_dirs = get_selected_tests();
for test_program_dir in test_program_dirs {
let mut cmd = Command::cargo_bin("nargo").unwrap();
cmd.arg("--program-dir").arg(&test_program_dir);
cmd.arg($command_string);

black_box(cmd.assert());
}
}
}
};
}
iai_command!(execution, "execute");
iai::main!(iai_selected_tests_execution);
14 changes: 14 additions & 0 deletions crates/nargo_cli/benches/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use std::path::PathBuf;

#[allow(unused)]
fn get_selected_tests() -> Vec<PathBuf> {
let manifest_dir = match std::env::var("CARGO_MANIFEST_DIR") {
Ok(dir) => PathBuf::from(dir),
Err(_) => std::env::current_dir().unwrap().join("crates").join("nargo_cli"),
};
let test_dir = manifest_dir.join("tests").join("execution_success");

let selected_tests =
vec!["8_integration", "sha256_blocks", "struct", "eddsa", "regression", "regression_2099"];
selected_tests.into_iter().map(|t| test_dir.join(t)).collect()
}
6 changes: 5 additions & 1 deletion crates/nargo_cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,11 @@ fn compile_success_{test_name}() {{
cmd.arg("info");
// `compile_success` tests should be able to compile down to an empty circuit.
cmd.assert().stdout(predicate::str::contains("Total ACIR opcodes generated for language PLONKCSat {{ width: 3 }}: 0"));
cmd.assert().stdout(predicate::str::contains("| Package")
.and(predicate::str::contains("| Language"))
.and(predicate::str::contains("| ACIR Opcodes | Backend Circuit Size |"))
.and(predicate::str::contains("| PLONKCSat {{ width: 3 }} |"))
.and(predicate::str::contains("| 0 | 1 |")));
}}
"#,
test_dir = test_dir.display(),
Expand Down
16 changes: 12 additions & 4 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use acvm::Backend;
use clap::Args;
use iter_extended::btree_map;
use nargo::{package::Package, prepare_package};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME};
use noirc_driver::{check_crate, compute_function_signature, CompileOptions};
use noirc_frontend::{
Expand All @@ -18,9 +18,13 @@ use super::NargoConfig;
#[derive(Debug, Clone, Args)]
pub(crate) struct CheckCommand {
/// The name of the package to check
#[clap(long)]
#[clap(long, conflicts_with = "workspace")]
package: Option<CrateName>,

/// Check all packages in the workspace
#[clap(long, conflicts_with = "package")]
workspace: bool,

#[clap(flatten)]
compile_options: CompileOptions,
}
Expand All @@ -31,7 +35,10 @@ pub(crate) fn run<B: Backend>(
config: NargoConfig,
) -> Result<(), CliError<B>> {
let toml_path = find_package_manifest(&config.program_dir)?;
let workspace = resolve_workspace_from_toml(&toml_path, args.package)?;
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;

for package in &workspace {
check_package(package, &args.compile_options)?;
Expand Down Expand Up @@ -88,7 +95,7 @@ fn create_input_toml_template(
.collect();
toml::Value::Array(default_value_vec)
}
AbiType::Struct { fields } => {
AbiType::Struct { fields, .. } => {
let default_value_map = toml::map::Map::from_iter(
fields.into_iter().map(|(name, typ)| (name, default_value(typ))),
);
Expand Down Expand Up @@ -128,6 +135,7 @@ mod tests {
typed_param(
"d",
AbiType::Struct {
name: String::from("MyStruct"),
fields: vec![
(String::from("d1"), AbiType::Field),
(
Expand Down
13 changes: 10 additions & 3 deletions crates/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use acvm::Backend;
use clap::Args;
use nargo::artifacts::program::PreprocessedProgram;
use nargo::{ops::codegen_verifier, package::Package};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::CompileOptions;
use noirc_frontend::graph::CrateName;

Expand All @@ -29,9 +29,13 @@ const BACKEND_IDENTIFIER: &str = "acvm-backend-barretenberg";
#[derive(Debug, Clone, Args)]
pub(crate) struct CodegenVerifierCommand {
/// The name of the package to codegen
#[clap(long)]
#[clap(long, conflicts_with = "workspace")]
package: Option<CrateName>,

/// Codegen all packages in the workspace
#[clap(long, conflicts_with = "package")]
workspace: bool,

#[clap(flatten)]
compile_options: CompileOptions,
}
Expand All @@ -42,7 +46,10 @@ pub(crate) fn run<B: Backend>(
config: NargoConfig,
) -> Result<(), CliError<B>> {
let toml_path = find_package_manifest(&config.program_dir)?;
let workspace = resolve_workspace_from_toml(&toml_path, args.package)?;
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;

for package in &workspace {
let circuit_build_path = workspace.package_build_path(package);
Expand Down
Loading

0 comments on commit d9236f4

Please sign in to comment.