Skip to content

Commit

Permalink
fix: ACIR optimizer should update assertion messages (#3010)
Browse files Browse the repository at this point in the history
  • Loading branch information
sirasistant authored Oct 6, 2023
1 parent db1e736 commit 758b6b6
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 19 deletions.
6 changes: 4 additions & 2 deletions acvm-repo/acvm/src/compiler/optimizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub(crate) use redundant_range::RangeOptimizer;

use self::unused_memory::UnusedMemoryOptimizer;

use super::AcirTransformationMap;
use super::{transform_assert_messages, AcirTransformationMap};

/// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`].
pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) {
Expand All @@ -36,10 +36,12 @@ pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) {

// Range optimization pass
let range_optimizer = RangeOptimizer::new(acir);
let (acir, acir_opcode_positions) =
let (mut acir, acir_opcode_positions) =
range_optimizer.replace_redundant_ranges(acir_opcode_positions);

let transformation_map = AcirTransformationMap { acir_opcode_positions };

acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);

(acir, transformation_map)
}
27 changes: 15 additions & 12 deletions tooling/nargo/src/ops/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ fn test_status_program_compile_fail(err: RuntimeError, test_function: TestFuncti
}

// The test has failed compilation, extract the assertion message if present and check if it's expected.
if let RuntimeError::FailedConstraint { assert_message: Some(assert_message), .. } = &err {
let assert_message = assert_message.clone();
check_expected_failure_message(test_function, &assert_message, Some(err.into()))
let assert_message = if let RuntimeError::FailedConstraint { assert_message, .. } = &err {
assert_message.clone()
} else {
TestStatus::CompileError(err.into())
}
None
};

check_expected_failure_message(test_function, assert_message, Some(err.into()))
}

/// The test function compiled successfully.
Expand Down Expand Up @@ -91,15 +92,16 @@ fn test_status_program_compile_pass(
};
}

let assertion_message =
circuit_execution_err.user_defined_failure_message().unwrap_or_default().to_string();

check_expected_failure_message(test_function, &assertion_message, diagnostic)
check_expected_failure_message(
test_function,
circuit_execution_err.user_defined_failure_message().map(|s| s.to_string()),
diagnostic,
)
}

fn check_expected_failure_message(
test_function: TestFunction,
failed_assertion: &str,
failed_assertion: Option<String>,
error_diagnostic: Option<FileDiagnostic>,
) -> TestStatus {
// Extract the expected failure message, if there was one
Expand All @@ -112,7 +114,8 @@ fn check_expected_failure_message(
None => return TestStatus::Pass,
};

let expected_failure_message_matches = failed_assertion == expected_failure_message;
let expected_failure_message_matches =
matches!(&failed_assertion, Some(message) if message == expected_failure_message);
if expected_failure_message_matches {
return TestStatus::Pass;
}
Expand All @@ -122,7 +125,7 @@ fn check_expected_failure_message(
message: format!(
"\nerror: Test failed with the wrong message. \nExpected: {} \nGot: {}",
test_function.failure_reason().unwrap_or_default(),
failed_assertion.trim_matches('\'')
failed_assertion.unwrap_or_default().trim_matches('\'')
),
error_diagnostic,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@

#[test(should_fail_with = "Not equal")]
fn test_main_1() {
fn test_different_string() {
assert_eq(0, 1, "Different string");
}

// The assert message has a space
#[test(should_fail_with = "Not equal")]
fn test_main_1() {
fn test_with_extra_space() {
assert_eq(0, 1, "Not equal ");
}

// The assert message has a space
#[test(should_fail_with = "Not equal")]
fn test_runtime_mismatch() {
assert_eq(dep::std::hash::pedersen([27])[0], 0, "Not equal ");
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@

#[test(should_fail_with = "Not equal")]
fn test_main_1() {
fn test_should_fail_with_match() {
assert_eq(0, 1, "Not equal");
}
}

#[test(should_fail)]
fn test_should_fail_without_match() {
assert_eq(0, 1);
}

#[test(should_fail_with = "Not equal")]
fn test_should_fail_with_runtime_match() {
assert_eq(dep::std::hash::pedersen([27])[0], 0, "Not equal");
}

#[test(should_fail)]
fn test_should_fail_without_runtime_match() {
assert_eq(dep::std::hash::pedersen([27])[0], 0);
}

0 comments on commit 758b6b6

Please sign in to comment.