Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Evaluation of dynamic assert messages #4101

Merged
merged 72 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
a523278
initial updates to make assert_message not a string literal
vezenovm Jan 17, 2024
31b6e55
Merge branch 'master' into mv/assert-msg-fmtstr
vezenovm Jan 17, 2024
84ec1bc
working resolution of assert messages and runtime
vezenovm Jan 19, 2024
0a084c4
remove old impl
vezenovm Jan 19, 2024
c581cee
revert debugging changes for brillig inputs
vezenovm Jan 19, 2024
73e48fb
cargo fmt
vezenovm Jan 19, 2024
0c46345
remove old ocmment
vezenovm Jan 19, 2024
1cf135d
cargo clippy and cleanup
vezenovm Jan 19, 2024
3123241
cleanup old comments and initial debugging for brillig assert message
vezenovm Jan 19, 2024
08d8922
improved usage of From trait for ForeignCallResults
vezenovm Jan 19, 2024
3284b99
resolve merge conflicts
vezenovm Jan 19, 2024
83605d8
start switching order of evaluation of resolve_assert_message as to n…
vezenovm Jan 22, 2024
7fdf5be
Merge branch 'master' into mv/assert-msg-fmtstr
vezenovm Jan 22, 2024
f131b96
switch order of execution
vezenovm Jan 23, 2024
f253cd2
update parser tests and some cleanup
vezenovm Jan 23, 2024
e149ede
cargo clippy
vezenovm Jan 23, 2024
8eb77ad
add back debug logs lines that were removed
vezenovm Jan 23, 2024
d0234d2
added compile failure tests
vezenovm Jan 23, 2024
0f0cbe0
move transformation of call expression to resolution rather than have…
vezenovm Jan 23, 2024
845e269
Merge branch 'master' into mv/assert-msg-fmtstr
vezenovm Jan 23, 2024
85a40c4
handle parser rrs
vezenovm Jan 23, 2024
e42cb9c
fix create_mock
vezenovm Jan 23, 2024
ec87b69
move to non-optional expr
vezenovm Jan 23, 2024
bc317a0
clippy
vezenovm Jan 23, 2024
9284490
clear out circuits w/ only brillig
vezenovm Jan 23, 2024
4333637
update compile_empty_success test
vezenovm Jan 24, 2024
9bc00eb
fix format
vezenovm Jan 24, 2024
f3083c8
cleanup resolve assert message
vezenovm Jan 24, 2024
185aba1
update debugger handle_foreign_call
vezenovm Jan 24, 2024
27ea4d9
cargo fmt
vezenovm Jan 24, 2024
0c7ff78
fix frontend tests, but acvm_js tests still failing
vezenovm Jan 24, 2024
720e470
Update tooling/nargo/src/ops/execute.rs
vezenovm Jan 24, 2024
8698083
some initial switches to putting resolve_assert_message under a predi…
vezenovm Jan 30, 2024
8458f83
Merge remote-tracking branch 'origin/mv/assert-msg-fmtstr' into mv/as…
vezenovm Jan 30, 2024
1a3ea85
accept changes
vezenovm Jan 30, 2024
480297a
missing merge edit
vezenovm Jan 30, 2024
8de9539
remove unused imports
vezenovm Jan 30, 2024
86cab04
remove old dbg
vezenovm Jan 30, 2024
7afc317
switch assert_message call instruction to be atomic with SSA and add …
vezenovm Jan 31, 2024
55de71f
cleanup decompose_constrain
vezenovm Jan 31, 2024
3ebc34a
some comment cleanup
vezenovm Jan 31, 2024
c0d3403
remove old comments
vezenovm Jan 31, 2024
49e7118
resolve merge conflicts
vezenovm Jan 31, 2024
9944a83
cargo fmt
vezenovm Jan 31, 2024
282a7d7
reorganize codegen of constrain error
vezenovm Jan 31, 2024
4cd9255
defunc cleanup
vezenovm Jan 31, 2024
9796a1b
cleanup
vezenovm Jan 31, 2024
8579816
cleanup brillig assert message gen:
vezenovm Jan 31, 2024
5170bc1
Merge branch 'master' into mv/assert-msg-fmtstr
vezenovm Jan 31, 2024
ae06f9e
macro errs update
vezenovm Jan 31, 2024
4e4d273
add an expect func to option
vezenovm Jan 31, 2024
e516c27
leave option expect for separate branch
vezenovm Jan 31, 2024
0c77f8f
Update compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs
vezenovm Jan 31, 2024
5cd9335
fixup git suggestion
vezenovm Jan 31, 2024
c8170f3
use AssertMessageMacro when disable_macros is enabled
vezenovm Jan 31, 2024
ae23e5c
chore: reduce diff and fix clippy warnings
TomAFrench Jan 31, 2024
5e0e198
chore: reduce diff
TomAFrench Jan 31, 2024
fa6b50c
format constrain err better
vezenovm Jan 31, 2024
f5598fe
Update compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
vezenovm Jan 31, 2024
d3c2f0e
cargo fmt
vezenovm Jan 31, 2024
bb34b45
fix old test updates
vezenovm Jan 31, 2024
819c95a
remove old comment
vezenovm Jan 31, 2024
c62bec8
reduce diff
vezenovm Jan 31, 2024
7f174dc
update assert docs
vezenovm Jan 31, 2024
dd056a1
chore: remove some indentation
TomAFrench Jan 31, 2024
757c740
noir js assert_lt test to show usage of resolve_assert_message
vezenovm Jan 31, 2024
545923e
Merge remote-tracking branch 'origin/mv/assert-msg-fmtstr' into mv/as…
vezenovm Jan 31, 2024
e002ba2
improve comment
vezenovm Jan 31, 2024
c6fe900
add assert_msg_runtime execution failure test to noirJS
vezenovm Jan 31, 2024
1d06de2
prettier
vezenovm Jan 31, 2024
a48f065
Apply suggestions from code review
TomAFrench Feb 1, 2024
6f27d40
chore: delete unnecessary files
TomAFrench Feb 1, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
[workspace]

members = [
# Macros crates for metaprogramming
"aztec_macros",
"noirc_macros",
# Compiler crates
"compiler/noirc_evaluator",
"compiler/noirc_frontend",
"compiler/noirc_errors",
Expand Down
4 changes: 4 additions & 0 deletions acvm-repo/acir/src/circuit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ pub struct Circuit {
// Note: This should be a BTreeMap, but serde-reflect is creating invalid
// c++ code at the moment when it is, due to OpcodeLocation needing a comparison
// implementation which is never generated.
//
// TODO: These are only used for constraints that are explicitly created during code generation (such as index out of bounds on slices)
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
// TODO: We should move towards having all the checks being evaluated in the same manner
// TODO: as runtime assert messages specified by the user. This will also be a breaking change as the `Circuit` structure will change.
pub assert_messages: Vec<(OpcodeLocation, String)>,
}

Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/brillig/src/foreign_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl ForeignCallParam {
}

/// Represents the full output of a [foreign call][crate::Opcode::ForeignCall].
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)]
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Default)]
pub struct ForeignCallResult {
/// Resolved output values of the foreign call.
pub values: Vec<ForeignCallParam>,
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/brillig_vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@
self.registers.set(*value_index, *value);
}
_ => unreachable!(
"Function result size does not match brillig bytecode (expected 1 result)"
"Function result size does not match brillig bytecode. Expected 1 result but got {output:?}"
),
},
RegisterOrMemory::HeapArray(HeapArray { pointer: pointer_index, size }) => {
Expand Down Expand Up @@ -545,7 +545,7 @@
}

#[test]
fn jmpifnot_opcode() {

Check warning on line 548 in acvm-repo/brillig_vm/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (jmpifnot)
let input_registers =
Registers::load(vec![Value::from(1u128), Value::from(2u128), Value::from(0u128)]);

Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ rust-embed.workspace = true
tracing.workspace = true

aztec_macros = { path = "../../aztec_macros" }
noirc_macros = { path = "../../noirc_macros" }
10 changes: 7 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub struct CompileOptions {
#[arg(long, hide = true)]
pub only_acir: bool,

/// Disables the builtin macros being used in the compiler
/// Disables the builtin Aztec macros being used in the compiler
#[arg(long, hide = true)]
pub disable_macros: bool,

Expand Down Expand Up @@ -191,9 +191,12 @@ pub fn check_crate(
disable_macros: bool,
) -> CompilationResult<()> {
let macros: Vec<&dyn MacroProcessor> = if disable_macros {
vec![]
vec![&noirc_macros::AssertMessageMacro as &dyn MacroProcessor]
} else {
vec![&aztec_macros::AztecMacro as &dyn MacroProcessor]
vec![
&aztec_macros::AztecMacro as &dyn MacroProcessor,
&noirc_macros::AssertMessageMacro as &dyn MacroProcessor,
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
]
};

let mut errors = vec![];
Expand Down Expand Up @@ -244,6 +247,7 @@ pub fn compile_main(
let compiled_program =
compile_no_check(context, options, main, cached_program, options.force_compile)
.map_err(FileDiagnostic::from)?;

let compilation_warnings = vecmap(compiled_program.warnings.clone(), FileDiagnostic::from);
if options.deny_warnings && !compilation_warnings.is_empty() {
return Err(compilation_warnings);
Expand Down
33 changes: 28 additions & 5 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::brillig::brillig_ir::{
BrilligBinaryOp, BrilligContext, BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE,
};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::instruction::ConstrainError;
use crate::ssa::ir::{
basic_block::{BasicBlock, BasicBlockId},
dfg::DataFlowGraph,
Expand Down Expand Up @@ -265,7 +266,30 @@ impl<'block> BrilligBlock<'block> {
condition,
);

self.brillig_context.constrain_instruction(condition, assert_message.clone());
let assert_message = if let Some(error) = assert_message {
match error.as_ref() {
ConstrainError::Static(string) => Some(string.clone()),
ConstrainError::Dynamic(call_instruction) => {
let Instruction::Call { func, arguments } = call_instruction else {
unreachable!("expected a call instruction")
};

let Value::Function(func_id) = &dfg[*func] else {
unreachable!("expected a function value")
};

self.convert_ssa_function_call(*func_id, arguments, dfg, &[]);

// Dynamic assert messages are handled in the generated function call.
// We then don't need to attach one to the constrain instruction.
None
}
}
} else {
None
};

self.brillig_context.constrain_instruction(condition, assert_message);
self.brillig_context.deallocate_register(condition);
}
Instruction::Allocate => {
Expand Down Expand Up @@ -369,7 +393,8 @@ impl<'block> BrilligBlock<'block> {
}
}
Value::Function(func_id) => {
self.convert_ssa_function_call(*func_id, arguments, dfg, instruction_id);
let result_ids = dfg.instruction_results(instruction_id);
self.convert_ssa_function_call(*func_id, arguments, dfg, result_ids);
}
Value::Intrinsic(Intrinsic::BlackBox(bb_func)) => {
// Slices are represented as a tuple of (length, slice contents).
Expand Down Expand Up @@ -638,16 +663,14 @@ impl<'block> BrilligBlock<'block> {
func_id: FunctionId,
arguments: &[ValueId],
dfg: &DataFlowGraph,
instruction_id: InstructionId,
result_ids: &[ValueId],
) {
// Convert the arguments to registers casting those to the types of the receiving function
let argument_registers: Vec<RegisterIndex> = arguments
.iter()
.flat_map(|argument_id| self.convert_ssa_value(*argument_id, dfg).extract_registers())
.collect();

let result_ids = dfg.instruction_results(instruction_id);

// Create label for the function that will be called
let label_of_function_to_call = FunctionContext::function_id_to_function_label(func_id);

Expand Down
130 changes: 85 additions & 45 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::fmt::Debug;
use self::acir_ir::acir_variable::{AcirContext, AcirType, AcirVar};
use super::function_builder::data_bus::DataBus;
use super::ir::dfg::CallStack;
use super::ir::instruction::ConstrainError;
use super::{
ir::{
dfg::DataFlowGraph,
Expand Down Expand Up @@ -413,15 +414,94 @@ impl Context {
let lhs = self.convert_numeric_value(*lhs, dfg)?;
let rhs = self.convert_numeric_value(*rhs, dfg)?;

self.acir_context.assert_eq_var(lhs, rhs, assert_message.clone())?;
let assert_message = if let Some(error) = assert_message {
match error.as_ref() {
ConstrainError::Static(string) => Some(string.clone()),
ConstrainError::Dynamic(call_instruction) => {
self.convert_ssa_call(call_instruction, dfg, ssa, brillig, &[])?;
None
}
}
} else {
None
};

self.acir_context.assert_eq_var(lhs, rhs, assert_message)?;
}
Instruction::Cast(value_id, _) => {
let acir_var = self.convert_numeric_value(*value_id, dfg)?;
self.define_result_var(dfg, instruction_id, acir_var);
}
Instruction::Call { func, arguments } => {
Instruction::Call { .. } => {
let result_ids = dfg.instruction_results(instruction_id);
match &dfg[*func] {
warnings.extend(self.convert_ssa_call(
instruction,
dfg,
ssa,
brillig,
result_ids,
)?);
}
Instruction::Not(value_id) => {
let (acir_var, typ) = match self.convert_value(*value_id, dfg) {
AcirValue::Var(acir_var, typ) => (acir_var, typ),
_ => unreachable!("NOT is only applied to numerics"),
};
let result_acir_var = self.acir_context.not_var(acir_var, typ)?;
self.define_result_var(dfg, instruction_id, result_acir_var);
}
Instruction::Truncate { value, bit_size, max_bit_size } => {
let result_acir_var =
self.convert_ssa_truncate(*value, *bit_size, *max_bit_size, dfg)?;
self.define_result_var(dfg, instruction_id, result_acir_var);
}
Instruction::EnableSideEffects { condition } => {
let acir_var = self.convert_numeric_value(*condition, dfg)?;
self.current_side_effects_enabled_var = acir_var;
}
Instruction::ArrayGet { .. } | Instruction::ArraySet { .. } => {
self.handle_array_operation(instruction_id, dfg, last_array_uses)?;
}
Instruction::Allocate => {
unreachable!("Expected all allocate instructions to be removed before acir_gen")
}
Instruction::Store { .. } => {
unreachable!("Expected all store instructions to be removed before acir_gen")
}
Instruction::Load { .. } => {
unreachable!("Expected all load instructions to be removed before acir_gen")
}
Instruction::IncrementRc { .. } => {
// Do nothing. Only Brillig needs to worry about reference counted arrays
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
let acir_var = self.convert_numeric_value(*value, dfg)?;
self.acir_context.range_constrain_var(
acir_var,
&NumericType::Unsigned { bit_size: *max_bit_size },
assert_message.clone(),
)?;
}
}

self.acir_context.set_call_stack(CallStack::new());
Ok(warnings)
}

fn convert_ssa_call(
&mut self,
instruction: &Instruction,
dfg: &DataFlowGraph,
ssa: &Ssa,
brillig: &Brillig,
result_ids: &[ValueId],
) -> Result<Vec<SsaReport>, RuntimeError> {
let mut warnings = Vec::new();

match instruction {
Instruction::Call { func, arguments } => {
let function_value = &dfg[*func];
match function_value {
Value::Function(id) => {
let func = &ssa.functions[id];
match func.runtime() {
Expand Down Expand Up @@ -495,51 +575,11 @@ impl Context {
Value::ForeignFunction(_) => unreachable!(
"All `oracle` methods should be wrapped in an unconstrained fn"
),
_ => unreachable!("expected calling a function"),
_ => unreachable!("expected calling a function but got {function_value:?}"),
}
}
Instruction::Not(value_id) => {
let (acir_var, typ) = match self.convert_value(*value_id, dfg) {
AcirValue::Var(acir_var, typ) => (acir_var, typ),
_ => unreachable!("NOT is only applied to numerics"),
};
let result_acir_var = self.acir_context.not_var(acir_var, typ)?;
self.define_result_var(dfg, instruction_id, result_acir_var);
}
Instruction::Truncate { value, bit_size, max_bit_size } => {
let result_acir_var =
self.convert_ssa_truncate(*value, *bit_size, *max_bit_size, dfg)?;
self.define_result_var(dfg, instruction_id, result_acir_var);
}
Instruction::EnableSideEffects { condition } => {
let acir_var = self.convert_numeric_value(*condition, dfg)?;
self.current_side_effects_enabled_var = acir_var;
}
Instruction::ArrayGet { .. } | Instruction::ArraySet { .. } => {
self.handle_array_operation(instruction_id, dfg, last_array_uses)?;
}
Instruction::Allocate => {
unreachable!("Expected all allocate instructions to be removed before acir_gen")
}
Instruction::Store { .. } => {
unreachable!("Expected all store instructions to be removed before acir_gen")
}
Instruction::Load { .. } => {
unreachable!("Expected all load instructions to be removed before acir_gen")
}
Instruction::IncrementRc { .. } => {
// Do nothing. Only Brillig needs to worry about reference counted arrays
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
let acir_var = self.convert_numeric_value(*value, dfg)?;
self.acir_context.range_constrain_var(
acir_var,
&NumericType::Unsigned { bit_size: *max_bit_size },
assert_message.clone(),
)?;
}
_ => unreachable!("expected calling a call instruction"),
}
self.acir_context.set_call_stack(CallStack::new());
Ok(warnings)
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::{
basic_block::BasicBlock,
dfg::{CallStack, InsertInstructionResult},
function::RuntimeType,
instruction::{Endian, InstructionId, Intrinsic},
instruction::{ConstrainError, Endian, InstructionId, Intrinsic},
types::NumericType,
},
ssa_gen::Ssa,
Expand Down Expand Up @@ -250,7 +250,7 @@ impl FunctionBuilder {
&mut self,
lhs: ValueId,
rhs: ValueId,
assert_message: Option<String>,
assert_message: Option<Box<ConstrainError>>,
) {
self.insert_instruction(Instruction::Constrain(lhs, rhs, assert_message), None);
}
Expand Down
Loading
Loading