From db415ae809979a463d955dc4ac8ed496504545f9 Mon Sep 17 00:00:00 2001
From: Daniel McGregor <daniel.mcgregor@makerx.com.au>
Date: Tue, 12 Nov 2024 14:15:37 +0800
Subject: [PATCH] fix: ensure expressions are only evaluated once

---
 src/puya/ir/builder/main.py | 38 +++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/puya/ir/builder/main.py b/src/puya/ir/builder/main.py
index 31a1dd60eb..a0951f9343 100644
--- a/src/puya/ir/builder/main.py
+++ b/src/puya/ir/builder/main.py
@@ -92,6 +92,7 @@ def __init__(
         self.context = context.for_function(function, subroutine, self)
         self._itxn = InnerTransactionBuilder(self.context)
         self._single_eval_cache = dict[awst_nodes.SingleEvaluation, TExpression]()
+        self._visited_exprs = dict[int, TExpression]()
 
     @classmethod
     def build_body(
@@ -1022,7 +1023,7 @@ def visit_loop_continue(self, statement: awst_nodes.LoopContinue) -> TStatement:
 
     def visit_expression_statement(self, statement: awst_nodes.ExpressionStatement) -> TStatement:
         # NOTE: popping of ignored return values should happen at code gen time
-        result = statement.expr.accept(self)
+        result = self._visit_and_check_for_double_eval(statement.expr)
         if result is None:
             wtype = statement.expr.wtype
             match wtype:
@@ -1195,18 +1196,47 @@ def visit_and_materialise_single(
     def visit_and_materialise(
         self, expr: awst_nodes.Expression, temp_description: str | Sequence[str] = "tmp"
     ) -> Sequence[Value]:
-        value_provider = self.visit_expr(expr)
-        return self.materialise_value_provider(value_provider, description=temp_description)
+        value_seq_or_provider = self._visit_and_check_for_double_eval(expr, temp_description)
+        if value_seq_or_provider is None:
+            raise InternalError(
+                "No value produced by expression IR conversion", expr.source_location
+            )
+        return self.materialise_value_provider(value_seq_or_provider, description=temp_description)
 
     def visit_expr(self, expr: awst_nodes.Expression) -> ValueProvider:
         """Visit the expression and ensure result is not None"""
-        value_seq_or_provider = expr.accept(self)
+        value_seq_or_provider = self._visit_and_check_for_double_eval(expr)
         if value_seq_or_provider is None:
             raise InternalError(
                 "No value produced by expression IR conversion", expr.source_location
             )
         return value_seq_or_provider
 
+    def _visit_and_check_for_double_eval(
+        self, expr: awst_nodes.Expression, desc: str | Sequence[str] | None = None
+    ) -> ValueProvider | None:
+        try:
+            result = self._visited_exprs[id(expr)]
+        except KeyError:
+            pass
+        else:
+            if isinstance(result, ValueProvider) and not isinstance(result, ValueTuple | Value):
+                raise InternalError(
+                    "double evaluation of expression without materialization", expr.source_location
+                )
+            return result
+        source = expr.accept(self)
+        if desc is None or source is None or not source.types or isinstance(source, Value):
+            result = source
+        else:
+            values = self.materialise_value_provider(source, description=desc)
+            if len(values) == 1:
+                (result,) = values
+            else:
+                result = ValueTuple(values=values, source_location=expr.source_location)
+        self._visited_exprs[id(expr)] = result
+        return result
+
     def materialise_value_provider(
         self, provider: ValueProvider, description: str | Sequence[str]
     ) -> list[Value]: