diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index bbb2f32a33a..c5990ebc255 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -18,7 +18,7 @@ use crate::{ type_check::{Source, TypeCheckError}, }, hir_def::{ - expr::{HirBlockExpression, HirExpression, HirIdent}, + expr::{HirBlockExpression, HirExpression, HirIdent, HirLiteral}, stmt::{ HirAssignStatement, HirForStatement, HirLValue, HirLetStatement, HirPattern, HirStatement, @@ -712,10 +712,7 @@ impl Elaborator<'_> { // If the original expression trivially cannot have side-effects, don't bother cluttering // the output with a let binding. Note that array literals can have side-effects but these // would produce type errors anyway. - if matches!( - self.interner.expression(&expr), - HirExpression::Ident(..) | HirExpression::Literal(..) - ) { + if !self.index_could_have_side_effects(expr) { return None; } @@ -738,6 +735,57 @@ impl Elaborator<'_> { Some((let_, ident_id)) } + /// Returns `true` if the given index expression could potentially have side-effects. + /// Returns `false` if the expression is guaranteed to not have side-effects. + fn index_could_have_side_effects(&self, expr: ExprId) -> bool { + match self.interner.expression_ref(&expr) { + HirExpression::Ident(..) => false, + HirExpression::Literal(hir_literal) => match hir_literal { + HirLiteral::Bool(_) + | HirLiteral::Integer(..) + | HirLiteral::Str(_) + | HirLiteral::Unit => false, + HirLiteral::Array(..) | HirLiteral::Vector(..) | HirLiteral::FmtStr(..) => true, + }, + HirExpression::Prefix(hir_prefix_expression) => { + hir_prefix_expression.trait_method_id.is_some() + || self.index_could_have_side_effects(hir_prefix_expression.rhs) + } + HirExpression::Infix(hir_infix_expression) => { + hir_infix_expression.trait_method_id.is_some() + || self.index_could_have_side_effects(hir_infix_expression.lhs) + || self.index_could_have_side_effects(hir_infix_expression.rhs) + } + HirExpression::Index(hir_index_expression) => { + self.index_could_have_side_effects(hir_index_expression.collection) + || self.index_could_have_side_effects(hir_index_expression.index) + } + HirExpression::Constructor(hir_constructor_expression) => { + !hir_constructor_expression.fields.is_empty() + } + HirExpression::EnumConstructor(hir_enum_constructor_expression) => { + !hir_enum_constructor_expression.arguments.is_empty() + } + HirExpression::MemberAccess(hir_member_access) => { + self.index_could_have_side_effects(hir_member_access.lhs) + } + HirExpression::Call(..) => true, + HirExpression::Constrain(..) => true, + HirExpression::Cast(hir_cast_expression) => { + self.index_could_have_side_effects(hir_cast_expression.lhs) + } + HirExpression::If(..) + | HirExpression::Match(..) + | HirExpression::Tuple(..) + | HirExpression::Lambda(..) + | HirExpression::Quote(..) + | HirExpression::Unquote(..) + | HirExpression::Unsafe(..) + | HirExpression::Block(..) + | HirExpression::Error => true, + } + } + fn elaborate_comptime_statement( &mut self, statement: Statement, diff --git a/compiler/noirc_frontend/src/node_interner/mod.rs b/compiler/noirc_frontend/src/node_interner/mod.rs index d8b5847af33..75a37d121b1 100644 --- a/compiler/noirc_frontend/src/node_interner/mod.rs +++ b/compiler/noirc_frontend/src/node_interner/mod.rs @@ -788,11 +788,16 @@ impl NodeInterner { /// Returns the interned expression corresponding to `expr_id` pub fn expression(&self, expr_id: &ExprId) -> HirExpression { + self.expression_ref(expr_id).clone() + } + + /// Returns the interned expression corresponding to `expr_id` + pub fn expression_ref(&self, expr_id: &ExprId) -> &HirExpression { let def = self.nodes.get(expr_id.0).expect("ice: all expression ids should have definitions"); match def { - Node::Expression(expr) => expr.clone(), + Node::Expression(expr) => expr, _ => { panic!("ice: all expression ids should correspond to a expression in the interner") } diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics/execute__tests__expanded.snap index 4c46a8fae81..c57a35cdaff 100644 --- a/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics/execute__tests__expanded.snap +++ b/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics/execute__tests__expanded.snap @@ -26,10 +26,7 @@ struct MyStruct { impl MyStruct { fn insert(mut self, index: Field, elem: Field) -> Self { assert((index as u64) < (S as u64)); - { - let i_0: u32 = index as u32; - self.data[i_0] = elem; - }; + self.data[index as u32] = elem; self } } diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics_explicit/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics_explicit/execute__tests__expanded.snap index 28e94c291b0..a19e5d78158 100644 --- a/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics_explicit/execute__tests__expanded.snap +++ b/tooling/nargo_cli/tests/snapshots/compile_success_empty/numeric_generics_explicit/execute__tests__expanded.snap @@ -35,10 +35,7 @@ struct MyStruct { impl MyStruct { fn insert(mut self, index: Field, elem: Field) -> Self { assert((index as u32) < S); - { - let i_0: u32 = index as u32; - self.data[i_0] = elem; - }; + self.data[index as u32] = elem; self } } diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/databus_two_calldata/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/databus_two_calldata/execute__tests__expanded.snap index c6c36598e06..11623081201 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_success/databus_two_calldata/execute__tests__expanded.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_success/databus_two_calldata/execute__tests__expanded.snap @@ -8,9 +8,6 @@ fn main(mut x: [u32; 4], y: [u32; 3], z: [u32; 4]) -> return_data [u32; 4] { let idx: u32 = x[i]; result[idx] = y[idx] + z[idx]; } - { - let i_0: u32 = x[3_u32]; - result[i_0] = z[x[3_u32]]; - }; + result[x[3_u32]] = z[x[3_u32]]; result } diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_1/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_1/execute__tests__expanded.snap index 87fe57239ff..d75d69062d2 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_1/execute__tests__expanded.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_1/execute__tests__expanded.snap @@ -15,10 +15,7 @@ impl BoundedVec4 { } pub fn push(&mut self, elem: Field) { - { - let i_0: u32 = self.len; - self.storage[i_0] = elem; - }; + self.storage[self.len] = elem; self.len = self.len + 1_u32; } } diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_2/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_2/execute__tests__expanded.snap index d1bf2a84095..1c13665056a 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_2/execute__tests__expanded.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_6674_2/execute__tests__expanded.snap @@ -15,10 +15,7 @@ impl BoundedVec4 { } pub fn push(&mut self, elem: Field) { - { - let i_0: u32 = self.len; - self.storage[i_0] = elem; - }; + self.storage[self.len] = elem; self.len = self.len + 1_u32; } } diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_capacity_tracker/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_capacity_tracker/execute__tests__expanded.snap index 3187ef25290..2e165b50570 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_success/regression_capacity_tracker/execute__tests__expanded.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_capacity_tracker/execute__tests__expanded.snap @@ -12,10 +12,7 @@ fn main(expected: pub Field, first: Field, input: [Field; 20]) { hasher_vector[i_0] = 100_Field; } } else { - { - let i_1: u32 = expected as u32; - hasher_vector[i_1] = 100_Field; - } + hasher_vector[expected as u32] = 100_Field; }; assert(hasher_vector[0_u32] == expected); }