Skip to content

Commit

Permalink
Merge branch 'master' into assert-eq-builtin
Browse files Browse the repository at this point in the history
* master:
  fix: Implement new mem2reg pass (#2420)
  feat(nargo): Support optional directory in git dependencies (#2436)
  fix(acir_gen): Pass accurate contents to slice inputs for bb func calls (#2435)
  fix(ssa): codegen missing check for unary minus (#2413)
  fix(lsp): Remove duplicated creation of lenses (#2433)
  feat: Add `test(should_fail)` attribute for tests that are meant to fail (#2418)
  chore: improve error message for InternalError (#2429)
  chore: Add stdlib to every crate as it is added to graph (#2392)
  • Loading branch information
TomAFrench committed Aug 25, 2023
2 parents f373495 + 7714cd0 commit 6a5a3d3
Show file tree
Hide file tree
Showing 34 changed files with 1,315 additions and 841 deletions.
71 changes: 2 additions & 69 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ fn on_code_lens_request(
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;
for (func_name, test_function) in tests {
let location = context.function_meta(&test_function.get_id()).name.location;
let file_id = location.file;

// Ignore diagnostics for any file that wasn't the file we saved
Expand Down Expand Up @@ -313,73 +313,6 @@ fn on_code_lens_request(
lenses.push(compile_lens);
}
}

if package.is_binary() {
if let Some(main_func_id) = context.get_main_function(&crate_id) {
let location = context.function_meta(&main_func_id).name.location;
let file_id = location.file;

// Ignore diagnostics for any file that wasn't the file we saved
// TODO: In the future, we could create "related" diagnostics for these files
// TODO: This currently just appends the `.nr` file extension that we store as a constant,
// but that won't work if we accept other extensions
if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path {
continue;
}

let range = byte_span_to_range(files, file_id.as_usize(), location.span.into())
.unwrap_or_default();

let command = Command {
title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"),
command: COMPILE_COMMAND.into(),
arguments: Some(vec![
"--program-dir".into(),
format!("{}", workspace.root_dir.display()).into(),
"--package".into(),
format!("{}", package.name).into(),
]),
};

let lens = CodeLens { range, command: command.into(), data: None };

lenses.push(lens);
}
}

if package.is_contract() {
// Currently not looking to deduplicate this since we don't have a clear decision on if the Contract stuff is staying
for contract in context.get_all_contracts(&crate_id) {
let location = contract.location;
let file_id = location.file;

// Ignore diagnostics for any file that wasn't the file we saved
// TODO: In the future, we could create "related" diagnostics for these files
// TODO: This currently just appends the `.nr` file extension that we store as a constant,
// but that won't work if we accept other extensions
if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path {
continue;
}

let range = byte_span_to_range(files, file_id.as_usize(), location.span.into())
.unwrap_or_default();

let command = Command {
title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"),
command: COMPILE_COMMAND.into(),
arguments: Some(vec![
"--program-dir".into(),
format!("{}", workspace.root_dir.display()).into(),
"--package".into(),
format!("{}", package.name).into(),
]),
};

let lens = CodeLens { range, command: command.into(), data: None };

lenses.push(lens);
}
}
}

let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) };
Expand Down
81 changes: 60 additions & 21 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelec
use noirc_driver::{compile_no_check, CompileOptions};
use noirc_frontend::{
graph::CrateName,
hir::{Context, FunctionNameMatch},
node_interner::FuncId,
hir::{def_map::TestFunction, Context, FunctionNameMatch},
};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

Expand Down Expand Up @@ -123,30 +122,70 @@ fn run_tests<B: Backend>(
fn run_test<B: Backend>(
backend: &B,
test_name: &str,
main: FuncId,
test_function: TestFunction,
context: &Context,
show_output: bool,
config: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut program = compile_no_check(context, config, main).map_err(|err| {
let report_error = |err| {
noirc_errors::reporter::report_all(&context.file_manager, &[err], config.deny_warnings);
CliError::Generic(format!("Test '{test_name}' failed to compile"))
})?;

// Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`.
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;

// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
match execute_circuit(backend, program.circuit, WitnessMap::new(), show_output) {
Ok(_) => Ok(()),
Err(error) => {
let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();
writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).ok();
writeln!(writer, "failed").ok();
writer.reset().ok();
Err(error.into())
Err(CliError::Generic(format!("Test '{test_name}' failed to compile")))
};
let write_error = |err| {
let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();
writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).ok();
writeln!(writer, "failed").ok();
writer.reset().ok();
Err(err)
};

let program = compile_no_check(context, config, test_function.get_id());
match program {
Ok(mut program) => {
// Note: We don't need to use the optimized ACIR here
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;

// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
let circuit_execution =
execute_circuit(backend, program.circuit, WitnessMap::new(), show_output);

if test_function.should_fail() {
match circuit_execution {
Ok(_) => {
write_error(CliError::Generic(format!("Test '{test_name}' should fail")))
}
Err(_) => Ok(()),
}
} else {
match circuit_execution {
Ok(_) => Ok(()),
Err(error) => write_error(error.into()),
}
}
}
// Test function failed to compile
//
// Note: This could be because the compiler was able to deduce
// that a constraint was never satisfiable.
// An example of this is the program `assert(false)`
// In that case, we check if the test function should fail, and if so, we return Ok.
Err(err) => {
// The test has failed compilation, but it should never fail. Report error.
if !test_function.should_fail() {
return report_error(err);
}

// The test has failed compilation, check if it is because the program is never satisfiable.
// If it is never satisfiable, then this is the expected behavior.
let program_is_never_satisfiable = err.diagnostic.message.contains("Failed constraint");
if program_is_never_satisfiable {
return Ok(());
}

// The test has failed compilation, but its a compilation error. Report error
report_error(err)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
message = [0,1,2,3,4,5,6,7,8,9]
message_field = "0x010203040506070809"
pub_key_x = "0x17cbd3ed3151ccfd170efe1d54280a6a4822640bf5c369908ad74ea21518a9c5"
pub_key_y = "0x0e0456e3795c1a31f20035b741cd6158929eeccd320d299cfcac962865a6bc74"
signature = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,20 @@ use dep::std;

// Note: If main has any unsized types, then the verifier will never be able
// to figure out the circuit instance
unconstrained fn main(message: [u8; 10], pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
unconstrained fn main(message: [u8; 10], message_field: Field, pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
// Regression for issue #2421
// We want to make sure that we can accurately verify a signature whose message is a slice vs. an array
let message_field_bytes = message_field.to_be_bytes(10);
for i in 0..10 {
assert(message[i] == message_field_bytes[i]);
}
// Is there ever a situation where someone would want
// to ensure that a signature was invalid?
// Check that passing a slice as the message is valid
let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message_field_bytes);
assert(valid_signature);

// Check that passing an array as the message is valid
let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message);
assert(valid_signature);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "references"
type = "bin"
authors = [""]
compiler_version = "0.5.1"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn increment(mut r: &mut Field) {
*r = *r + 1;
}

fn main() {
let mut x = 100;
let mut xref = &mut x;
increment(xref);
assert(*xref == 101);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
H4sIAAAAAAAA/+1c8ZONVRh+WCKykYqIiFRE37f37u5dkY2IEEJEtLv2bkJKQkpCSkJKIjLGNE3TmJqpmZqpmZrRH9L/Uu907szZzfjlPO+Z98yc88u+w8yz3/O853ue93zf3fsXgL/x/zXY/ex0P4uwVQ7ysCpFW7Vab2+pl5Wyu2jp6Km1FtXWnrZaWStba629LbVKpV6r1to7ejrai46yWqmXfa0dlT4HNpiAVe/7bzX9izHoJvwHkfkP5mEV/vU2efWQAb3z//82BU4Y8HsG6th8k3+j/nKNJjUp4A4Bb/Nr8R7C71HhQZrXtLEsG99QpGd8Q6FjfLd5dTa+QMyhSkINg23jE97D+D1SNT62po1l2fiGIz3jGw4d47vdq7PxBWIOd4KycUfAtvEJ7xH8HqkaH1vTxrJsfCORnvGNhI7x3eHV2fgCMUc6Qdm4o2Db+IT3KH6PVI2PrWljWTa+ZtCMrxbL+JqhY3x3enU2vkDMZicoG3c0bBuf8B7N71EyhjIGLEOp12MZyhjoGMpdXp0NJRBzjBOUjTsWtg1FeI/l9+iWhhKqA9Ok74bOTcHmzNxH9yTCmdnneyNxLsJWL7PP43jX1SIYbO+Rnoy7CW4o76vQ8bEmKv+yytzf44l9uQrWkBXvcRWRf78h6z6vzkNWIOZ4JygbdwJsD1nCewK/R6qPq9iaNhZ7SGCeLifS9CsrsYyPd839je9+r87GF4g50QnKxp0E28YnvCfxe6T5uKqVqelkIudYhsK8Zv96H/DqbCiBmJOdoGzcKbBtKMJ7Cr9HqpPUFMSZpIqwRX1OP5WAFfsIORU6xvegV2fjC8Sc6gRl406DbeMT3tP4PUrGUKaDZSjxXvxNh46hPOTV2VACMac7Qdm4M2DbUIT3DH6PVJ/1ME36YejcFGzOzH30SCKcmX1+NBLnImz1Mvs8k3ddKi/+pCczwX/xdw06PsZ+8cfc37OIfbkG2pDVG2vIIvLvN2Q95tV5yArEnOUEZePOhu0hS3jP5vdI9XEVW9PGsny6nAOW8cX7nPoc6Bjf416djS8Qc44TlI1bwLbxCe+C3yNFQylbmJqWRM6xDIV5zf71tnh1NpRAzNIJysatwLahCO8Kv0eqkxRb08ayPElVkd4kVYWO8bV6dTa+QMyqE5SN2wbbxie82/g9SsZQ2pGeobRDx1BqXp0NJRCz3QnKxu2AbUMR3h38HiVjKHORnqHMhY6hPOHV2VACMec6Qdm482DbUIT3PH6PkjGU+UjPUOZDx1Ce9OpsKIGY852gbNwFsG0ownsBv0fJGEon0jOUTugYylNenQ0lELPTCcrGXQjbhiI4C/k9SsZQFiE9Q1kEHUN52quzoQRiLnKCsnEXw7ahCO/F/B4lYyhLwDKUeH+NsQQ6hvKMV2dDCcRc4gRl4y6FbUMR3kv5PUrGUJYhPUNZBh1Dedars6EEYi5zgrJxl8O2oQjv5fweqf55F9OkV/A4t9yKcxG2qCa6EumZ6EromOhzXp1NNBBzpROUjbsKtk1UeK/i90jlWsXsV4D/N3XfQCc8mkjX2fiWEGYgryb2hahfMkG0BukF0RroBNHzXp2DKBBzjROUjbsWtoNIeK/l90jlWiUwV4MfRN8ijSBiDjXriH0h6pdMEK1HekG0HjpB9IJX5yAKxFzvBGXjboDtIBLeG/g9UrlWCcx14AfRd0gjiJhDzUZiX4j63TKIQjkzX0K/CNt+Jvf0RoV75Xukca8wfXcTsS9E/VS+yUn29SaFfXM90r4pglZZZXrEZmJfrhPvjViDL5F/v8H3Ja/Og28g5mYnKBt3C2wPvsJ7C79Hqt8/wNa0sSyf+LeCZnzRPuq6FTrG97JXZ+MLxNzqBGXjdsG28QnvLn6PkjGUbqRnKN3QMZQer86GEojZ7QRl426DbUMR3tv4PVL9ZFoXEasXOjcFmzNzH9UjcS7CFjU4+sAKjnjvnvqgExyveHUOjkDMPicoG3c7bAeH8N7O75FqcDBN9FWkERzMfbSDx7nfuyLLz4F3Evus9RzdcvDuAit4453YdkEneF/z6hy8gZi7nKBs3N2wHbzCeze/R8mc2F5HGsHL3EdvROJchC1qcOwBKzjindj2QCc43vTqHByBmHucoGzcvbAdHMJ7L79HqsHBNNG3kEZwMPfRPh5nlU8syYl8J/ifWPoB3P3N5i1PD3Yo8P4ROvd1E/k69xO1JPa61NLP8qB1AKxBK94J/QB0Bq23vToPWoGYB5ygbNyDsD1oCe+D/B4lc0J/Bzo3BZszcx+9G4lzEbaowXEIrOCId0I/BJ3geM+rc3AEYh5ygrJxD8N2cAjvw/weqQYH00TfRxrBwdxHR3icVU7o8gRmP/gn1Z/A3d9s3vK0aJ8C75+hc1+zT+hHiVoSe11q6Wd50DoG1qAV74R+DDqD1gdenQetQMxjTlA27nHYHrSE93F+j5I5oX8InZuCzZm5jz6KxLkIW9TgOAFWcMQ7oZ+ATnB87NU5OAIxTzhB2bgnYTs4hPdJfo9Ug4Npop8gjeBg7qNTPM4qJ3R5AnMU/JPqL+DubzZveVp0RIH3r9C5r9kn9NNELYm9LrX0szxonQFr0Ip3Qj8DnUHrU6/Og1Yg5hknKBv3LGwPWsL7LL9HyZzQP4POTcHmzNxHn0fiXIQtanCcAys44p3Qz0EnOL7w6hwcgZjnnKBs3POwHRzC+zy/R6rBwTTRL5FGcDD30QUeZ5UTujyBOQ3+SfU3cPc3m7c8LTqlwPt36NzX7BP6RaKWxF6XWvpZHqi/gu28lgy4qHCv/AHbHiF5dUGB959IwyMuEbUk9rpk68feN3I/X1LYNzcQZ98UQausdhE5Xyb25QbhumJ/1zWRf78D7ddenQ+0gZiXnaBs3CuwfaAV3lf4PVL9rmumpoMGXKO//gGokXOuaucAAA==
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
message = [0,1,2,3,4,5,6,7,8,9]
message_field = "0x010203040506070809"
pub_key_x = "0x17cbd3ed3151ccfd170efe1d54280a6a4822640bf5c369908ad74ea21518a9c5"
pub_key_y = "0x0e0456e3795c1a31f20035b741cd6158929eeccd320d299cfcac962865a6bc74"
signature = [
Expand Down
13 changes: 12 additions & 1 deletion crates/nargo_cli/tests/execution_success/schnorr/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,20 @@ use dep::std;

// Note: If main has any unsized types, then the verifier will never be able
// to figure out the circuit instance
fn main(message: [u8; 10], pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
fn main(message: [u8; 10], message_field: Field, pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
// Regression for issue #2421
// We want to make sure that we can accurately verify a signature whose message is a slice vs. an array
let message_field_bytes = message_field.to_be_bytes(10);
for i in 0..10 {
assert(message[i] == message_field_bytes[i]);
}
// Is there ever a situation where someone would want
// to ensure that a signature was invalid?
// Check that passing a slice as the message is valid
let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message_field_bytes);
assert(valid_signature);

// Check that passing an array as the message is valid
let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message);
assert(valid_signature);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "unary_operators"
type = "bin"
authors = [""]
compiler_version = "0.10.3"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
let x = -1;
assert(x == 1 - 2);

let y: i32 = -1;
assert(x == 1 - 2);
}
3 changes: 3 additions & 0 deletions crates/nargo_toml/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ pub enum ManifestError {
#[error("{} found in {toml}", if name.is_empty() { "Empty dependency name".into() } else { format!("Invalid dependency name `{name}`") })]
InvalidDependencyName { toml: PathBuf, name: String },

#[error("Invalid directory path {directory} in {toml}: It must point to a subdirectory")]
InvalidDirectory { toml: PathBuf, directory: PathBuf },

/// Encountered error while downloading git repository.
#[error("{0}")]
GitError(String),
Expand Down
18 changes: 15 additions & 3 deletions crates/nargo_toml/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,28 @@ struct PackageMetadata {
/// Enum representing the different types of ways to
/// supply a source for the dependency
enum DependencyConfig {
Github { git: String, tag: String },
Github { git: String, tag: String, directory: Option<String> },
Path { path: String },
}

impl DependencyConfig {
fn resolve_to_dependency(&self, pkg_root: &Path) -> Result<Dependency, ManifestError> {
let dep = match self {
Self::Github { git, tag } => {
Self::Github { git, tag, directory } => {
let dir_path = clone_git_repo(git, tag).map_err(ManifestError::GitError)?;
let toml_path = dir_path.join("Nargo.toml");
let project_path = if let Some(directory) = directory {
let internal_path = dir_path.join(directory).normalize();
if !internal_path.starts_with(&dir_path) {
return Err(ManifestError::InvalidDirectory {
toml: pkg_root.join("Nargo.toml"),
directory: directory.into(),
});
}
internal_path
} else {
dir_path
};
let toml_path = project_path.join("Nargo.toml");
let package = resolve_package_from_toml(&toml_path)?;
Dependency::Remote { package }
}
Expand Down
Loading

0 comments on commit 6a5a3d3

Please sign in to comment.