From 7d53505a3d02e0782d0e7f10b45753fe03299f6f Mon Sep 17 00:00:00 2001 From: thunkar Date: Tue, 27 Feb 2024 18:15:44 +0100 Subject: [PATCH 1/5] remove original return from aztec fns --- noir/aztec_macros/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index 156ba1d5b08..3081ef2e0bb 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -1253,7 +1253,7 @@ fn create_avm_context() -> Result { /// Similarly; Structs will be pushed to the context, after serialize() is called on them. /// Arrays will be iterated over and each element will be pushed to the context. /// Any primitive type that can be cast will be casted to a field and pushed to the context. -fn abstract_return_values(func: &NoirFunction) -> Option { +fn abstract_return_values(func: &mut NoirFunction) -> Option { let current_return_type = func.return_type().typ; let len = func.def.body.len(); let last_statement = &func.def.body.0[len - 1]; @@ -1265,7 +1265,7 @@ fn abstract_return_values(func: &NoirFunction) -> Option { // Check if the return type is an expression, if it is, we can handle it match last_statement { Statement { kind: StatementKind::Expression(expression), .. } => { - match current_return_type { + let new_return = match current_return_type { // Call serialize on structs, push the whole array, calling push_array UnresolvedTypeData::Named(..) => Some(make_struct_return_type(expression.clone())), UnresolvedTypeData::Array(..) => Some(make_array_return_type(expression.clone())), @@ -1275,7 +1275,11 @@ fn abstract_return_values(func: &NoirFunction) -> Option { } UnresolvedTypeData::FieldElement => Some(make_return_push(expression.clone())), _ => None, + }; + if new_return.is_some() { + func.def_mut().body.0.remove(len - 1); } + new_return } _ => None, } From 4267b0bb9cb89123641d42a95b7f6818e40d5b1c Mon Sep 17 00:00:00 2001 From: thunkar Date: Tue, 27 Feb 2024 19:05:03 +0100 Subject: [PATCH 2/5] added clarifying comment --- noir/aztec_macros/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index 3081ef2e0bb..c33204bd83c 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -1276,6 +1276,8 @@ fn abstract_return_values(func: &mut NoirFunction) -> Option { UnresolvedTypeData::FieldElement => Some(make_return_push(expression.clone())), _ => None, }; + // In case we are pushing return values to the context, we remove the statement that originated the push + // This avoids running duplicate code, since blocks like if/else can be value returning statements if new_return.is_some() { func.def_mut().body.0.remove(len - 1); } From 9353cf1a53952742e2fe15c3933695bd6afbf28b Mon Sep 17 00:00:00 2001 From: thunkar Date: Tue, 27 Feb 2024 19:24:21 +0100 Subject: [PATCH 3/5] cleaner syntax --- noir/aztec_macros/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index c33204bd83c..9ed53ee2670 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -1255,15 +1255,13 @@ fn create_avm_context() -> Result { /// Any primitive type that can be cast will be casted to a field and pushed to the context. fn abstract_return_values(func: &mut NoirFunction) -> Option { let current_return_type = func.return_type().typ; - let len = func.def.body.len(); - let last_statement = &func.def.body.0[len - 1]; + let last_statement = func.def.body.0.last(); // TODO: (length, type) => We can limit the size of the array returned to be limited by kernel size // Doesn't need done until we have settled on a kernel size // TODO: support tuples here and in inputs -> convert into an issue - // Check if the return type is an expression, if it is, we can handle it - match last_statement { + match last_statement? { Statement { kind: StatementKind::Expression(expression), .. } => { let new_return = match current_return_type { // Call serialize on structs, push the whole array, calling push_array @@ -1279,7 +1277,7 @@ fn abstract_return_values(func: &mut NoirFunction) -> Option { // In case we are pushing return values to the context, we remove the statement that originated the push // This avoids running duplicate code, since blocks like if/else can be value returning statements if new_return.is_some() { - func.def_mut().body.0.remove(len - 1); + func.def_mut().body.0.pop(); } new_return } From 6f8bfd2d8fb26c1dde76f8d1ab98150c6f3f45d1 Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 28 Feb 2024 09:49:37 +0100 Subject: [PATCH 4/5] simpler approach --- noir/aztec_macros/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index 9ed53ee2670..85ed40b8e59 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -648,6 +648,10 @@ fn transform_function( // Abstract return types such that they get added to the kernel's return_values if let Some(return_values) = abstract_return_values(func) { + // In case we are pushing return values to the context, we remove the statement that originated it + // This avoids running duplicate code, since blocks like if/else can be value returning statements + func.def.body.0.pop(); + // Add the new return statement func.def.body.0.push(return_values); } @@ -1263,7 +1267,7 @@ fn abstract_return_values(func: &mut NoirFunction) -> Option { // Check if the return type is an expression, if it is, we can handle it match last_statement? { Statement { kind: StatementKind::Expression(expression), .. } => { - let new_return = match current_return_type { + match current_return_type { // Call serialize on structs, push the whole array, calling push_array UnresolvedTypeData::Named(..) => Some(make_struct_return_type(expression.clone())), UnresolvedTypeData::Array(..) => Some(make_array_return_type(expression.clone())), @@ -1273,13 +1277,7 @@ fn abstract_return_values(func: &mut NoirFunction) -> Option { } UnresolvedTypeData::FieldElement => Some(make_return_push(expression.clone())), _ => None, - }; - // In case we are pushing return values to the context, we remove the statement that originated the push - // This avoids running duplicate code, since blocks like if/else can be value returning statements - if new_return.is_some() { - func.def_mut().body.0.pop(); } - new_return } _ => None, } From d36a76fed854b1823f8175b9f27a2c510d9e3995 Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 28 Feb 2024 09:50:44 +0100 Subject: [PATCH 5/5] remove unnecesary mut --- noir/aztec_macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index 85ed40b8e59..2e4637b7732 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -1257,7 +1257,7 @@ fn create_avm_context() -> Result { /// Similarly; Structs will be pushed to the context, after serialize() is called on them. /// Arrays will be iterated over and each element will be pushed to the context. /// Any primitive type that can be cast will be casted to a field and pushed to the context. -fn abstract_return_values(func: &mut NoirFunction) -> Option { +fn abstract_return_values(func: &NoirFunction) -> Option { let current_return_type = func.return_type().typ; let last_statement = func.def.body.0.last();