chore: better logic for checking whether an array index could have si…

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,

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")
             }