Skip to content

Commit

Permalink
Merge branch 'master' into jb/noir-testing-workflow
Browse files Browse the repository at this point in the history
* master:
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
  • Loading branch information
TomAFrench committed Aug 7, 2023
2 parents b6eac52 + 44df932 commit 1b7eca9
Show file tree
Hide file tree
Showing 791 changed files with 1,522 additions and 796 deletions.
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ result
*.vk
**/Verifier.toml
**/target
!crates/nargo_cli/tests/test_data/*/target
!crates/nargo_cli/tests/test_data/*/target/witness.tr
!crates/nargo_cli/tests/execution_success/*/target
!crates/nargo_cli/tests/execution_success/*/target/witness.tr
9 changes: 3 additions & 6 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ use lsp_types::{
};
use noirc_driver::{check_crate, prepare_crate};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{
graph::{CrateGraph, CrateType},
hir::Context,
};
use noirc_frontend::{graph::CrateGraph, hir::Context};
use serde_json::Value as JsonValue;
use tower::Service;

Expand Down Expand Up @@ -190,7 +187,7 @@ fn on_code_lens_request(
}
};

let crate_id = prepare_crate(&mut context, file_path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, file_path);

// We ignore the warnings and errors produced by compilation for producing codelenses
// because we can still get the test functions even if compilation fails
Expand Down Expand Up @@ -283,7 +280,7 @@ fn on_did_save_text_document(
}
};

let crate_id = prepare_crate(&mut context, file_path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, file_path);

let mut diagnostics = Vec::new();

Expand Down
6 changes: 3 additions & 3 deletions crates/lsp/src/lib_hacky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use lsp_types::{
use noirc_driver::{check_crate, prepare_crate, propagate_dep};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{
graph::{CrateGraph, CrateId, CrateType},
graph::{CrateGraph, CrateId},
hir::Context,
};

Expand Down Expand Up @@ -286,7 +286,7 @@ fn create_context_at_path(
}
let nargo_toml_path = find_nearest_parent_file(&file_path, &["Nargo.toml"]);

let current_crate_id = prepare_crate(&mut context, &file_path, CrateType::Binary);
let current_crate_id = prepare_crate(&mut context, &file_path);

// TODO(AD): undo hacky dependency resolution
if let Some(nargo_toml_path) = nargo_toml_path {
Expand All @@ -297,7 +297,7 @@ fn create_context_at_path(
.parent()
.unwrap() // TODO
.join(PathBuf::from(&dependency_path).join("src").join("lib.nr"));
let library_crate = prepare_crate(&mut context, &path_to_lib, CrateType::Library);
let library_crate = prepare_crate(&mut context, &path_to_lib);
propagate_dep(&mut context, library_crate, &crate_name.parse().unwrap());
}
}
Expand Down
43 changes: 40 additions & 3 deletions crates/nargo/src/package.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,48 @@
use std::{collections::BTreeMap, path::PathBuf};
use std::{collections::BTreeMap, fmt::Display, path::PathBuf};

use noirc_frontend::graph::{CrateName, CrateType};
use noirc_frontend::graph::CrateName;

use crate::constants::{PROVER_INPUT_FILE, VERIFIER_INPUT_FILE};

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum PackageType {
Library,
Binary,
}

impl Display for PackageType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Library => write!(f, "lib"),
Self::Binary => write!(f, "bin"),
}
}
}

#[derive(Clone)]
pub enum Dependency {
Local { package: Package },
Remote { package: Package },
}

impl Dependency {
pub fn is_binary(&self) -> bool {
match self {
Self::Local { package } | Self::Remote { package } => package.is_binary(),
}
}

pub fn package_name(&self) -> &CrateName {
match self {
Self::Local { package } | Self::Remote { package } => &package.name,
}
}
}

#[derive(Clone)]
pub struct Package {
pub root_dir: PathBuf,
pub crate_type: CrateType,
pub package_type: PackageType,
pub entry_path: PathBuf,
pub name: CrateName,
pub dependencies: BTreeMap<CrateName, Dependency>,
Expand All @@ -30,4 +59,12 @@ impl Package {
// For now it is hard-coded to be toml.
self.root_dir.join(format!("{VERIFIER_INPUT_FILE}.toml"))
}

pub fn is_binary(&self) -> bool {
self.package_type == PackageType::Binary
}

pub fn is_library(&self) -> bool {
self.package_type == PackageType::Library
}
}
124 changes: 85 additions & 39 deletions crates/nargo_cli/build.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use rustc_version::{version, Version};
use std::collections::BTreeMap;
use std::fs::File;
use std::io::Write;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -32,43 +31,25 @@ fn main() {
let destination = Path::new(&out_dir).join("execute.rs");
let mut test_file = File::create(destination).unwrap();

generate_tests(&mut test_file);
}

fn load_conf(conf_path: &Path) -> BTreeMap<String, Vec<String>> {
let config_str = std::fs::read_to_string(conf_path).unwrap();

let mut conf_data = match toml::from_str(&config_str) {
Ok(t) => t,
Err(_) => {
BTreeMap::from([("exclude".to_string(), Vec::new()), ("fail".to_string(), Vec::new())])
}
};
if conf_data.get("exclude").is_none() {
conf_data.insert("exclude".to_string(), Vec::new());
}
if conf_data.get("fail").is_none() {
conf_data.insert("fail".to_string(), Vec::new());
}
conf_data
}

fn generate_tests(test_file: &mut File) {
// Try to find the directory that Cargo sets when it is running; otherwise fallback to assuming the CWD
// is the root of the repository and append the crate path
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_sub_dir = "test_data";
let test_data_dir = manifest_dir.join("tests").join(test_sub_dir);
let config_path = test_data_dir.join("config.toml");
let test_dir = manifest_dir.join("tests");

generate_execution_success_tests(&mut test_file, &test_dir);
generate_compile_success_tests(&mut test_file, &test_dir);
generate_compile_failure_tests(&mut test_file, &test_dir);
}

// Load config.toml file from `test_data` directory
let config_data: BTreeMap<String, Vec<String>> = load_conf(&config_path);
fn generate_execution_success_tests(test_file: &mut File, test_data_dir: &Path) {
let test_sub_dir = "execution_success";
let test_data_dir = test_data_dir.join(test_sub_dir);

let test_case_dirs =
fs::read_dir(&test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir());
fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir());

for test_dir in test_case_dirs {
let test_name =
Expand All @@ -80,28 +61,93 @@ fn generate_tests(test_file: &mut File) {
};
let test_dir = &test_dir.path();

let exclude_macro =
if config_data["exclude"].contains(&test_name) { "#[ignore]" } else { "" };
write!(
test_file,
r#"
#[test]
fn execution_success_{test_name}() {{
let test_program_dir = PathBuf::from("{test_dir}");
let should_fail = config_data["fail"].contains(&test_name);
let mut cmd = Command::cargo_bin("nargo").unwrap();
cmd.arg("--program-dir").arg(test_program_dir);
cmd.arg("execute");
cmd.assert().success();
}}
"#,
test_dir = test_dir.display(),
)
.expect("Could not write templated test file.");
}
}

fn generate_compile_success_tests(test_file: &mut File, test_data_dir: &Path) {
let test_sub_dir = "compile_success";
let test_data_dir = test_data_dir.join(test_sub_dir);

let test_case_dirs =
fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir());

for test_dir in test_case_dirs {
let test_name =
test_dir.file_name().into_string().expect("Directory can't be converted to string");
if test_name.contains('-') {
panic!(
"Invalid test directory: {test_name}. Cannot include `-`, please convert to `_`"
);
};
let test_dir = &test_dir.path();

write!(
test_file,
r#"
#[test]
fn compile_success_{test_name}() {{
let test_program_dir = PathBuf::from("{test_dir}");
let mut cmd = Command::cargo_bin("nargo").unwrap();
cmd.arg("--program-dir").arg(test_program_dir);
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"));
}}
"#,
test_dir = test_dir.display(),
)
.expect("Could not write templated test file.");
}
}

fn generate_compile_failure_tests(test_file: &mut File, test_data_dir: &Path) {
let test_sub_dir = "compile_failure";
let test_data_dir = test_data_dir.join(test_sub_dir);

let test_case_dirs =
fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir());

for test_dir in test_case_dirs {
let test_name =
test_dir.file_name().into_string().expect("Directory can't be converted to string");
if test_name.contains('-') {
panic!(
"Invalid test directory: {test_name}. Cannot include `-`, please convert to `_`"
);
};
let test_dir = &test_dir.path();

write!(
test_file,
r#"
{exclude_macro}
#[test]
fn execute_{test_sub_dir}_{test_name}() {{
fn compile_failure_{test_name}() {{
let test_program_dir = PathBuf::from("{test_dir}");
let mut cmd = Command::cargo_bin("nargo").unwrap();
cmd.arg("--program-dir").arg(test_program_dir);
cmd.arg("execute");
if {should_fail} {{
cmd.assert().failure();
}} else {{
cmd.assert().success();
}}
cmd.assert().failure();
}}
"#,
test_dir = test_dir.display(),
Expand Down
52 changes: 29 additions & 23 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
use crate::{
errors::CliError, find_package_manifest, manifest::resolve_workspace_from_toml, prepare_package,
errors::{CliError, CompileError},
find_package_manifest,
manifest::resolve_workspace_from_toml,
prepare_package,
};
use acvm::Backend;
use clap::Args;
use iter_extended::btree_map;
use nargo::package::Package;
use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME};
use noirc_driver::{check_crate, compute_function_signature, CompileOptions};
use noirc_errors::reporter::ReportedErrors;
use noirc_frontend::{
graph::{CrateId, CrateName},
hir::Context,
Expand Down Expand Up @@ -42,33 +44,37 @@ pub(crate) fn run<B: Backend>(
Ok(())
}

fn check_package(
package: &Package,
compile_options: &CompileOptions,
) -> Result<(), ReportedErrors> {
fn check_package(package: &Package, compile_options: &CompileOptions) -> Result<(), CompileError> {
let (mut context, crate_id) = prepare_package(package);
check_crate_and_report_errors(&mut context, crate_id, compile_options.deny_warnings)?;

// XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
if let Some((parameters, return_type)) = compute_function_signature(&context, &crate_id) {
let path_to_prover_input = package.prover_input_path();
let path_to_verifier_input = package.verifier_input_path();
if package.is_library() {
// Libraries do not have ABIs.
Ok(())
} else {
// XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
if let Some((parameters, return_type)) = compute_function_signature(&context, &crate_id) {
let path_to_prover_input = package.prover_input_path();
let path_to_verifier_input = package.verifier_input_path();

// If they are not available, then create them and populate them based on the ABI
if !path_to_prover_input.exists() {
let prover_toml = create_input_toml_template(parameters.clone(), None);
write_to_file(prover_toml.as_bytes(), &path_to_prover_input);
}
if !path_to_verifier_input.exists() {
let public_inputs =
parameters.into_iter().filter(|param| param.is_public()).collect();

// If they are not available, then create them and populate them based on the ABI
if !path_to_prover_input.exists() {
let prover_toml = create_input_toml_template(parameters.clone(), None);
write_to_file(prover_toml.as_bytes(), &path_to_prover_input);
}
if !path_to_verifier_input.exists() {
let public_inputs = parameters.into_iter().filter(|param| param.is_public()).collect();
let verifier_toml = create_input_toml_template(public_inputs, return_type);
write_to_file(verifier_toml.as_bytes(), &path_to_verifier_input);
}

let verifier_toml = create_input_toml_template(public_inputs, return_type);
write_to_file(verifier_toml.as_bytes(), &path_to_verifier_input);
Ok(())
} else {
Err(CompileError::MissingMainFunction(package.name.clone()))
}
} else {
// This means that this is a library. Libraries do not have ABIs.
}
Ok(())
}

/// Generates the contents of a toml file with fields for each of the passed parameters.
Expand Down Expand Up @@ -223,7 +229,7 @@ pub(crate) fn check_crate_and_report_errors(
context: &mut Context,
crate_id: CrateId,
deny_warnings: bool,
) -> Result<(), ReportedErrors> {
) -> Result<(), CompileError> {
let result = check_crate(context, crate_id, deny_warnings).map(|warnings| ((), warnings));
super::compile_cmd::report_errors(result, context, deny_warnings)
}
Loading

0 comments on commit 1b7eca9

Please sign in to comment.