From 2dc28c90825808f8931cbe2f8baf819586f0be77 Mon Sep 17 00:00:00 2001 From: Daniel McGregor Date: Fri, 1 Nov 2024 16:23:38 +0800 Subject: [PATCH] fix: raise an error when attempting to iterate a tuple of mutable values BREAKING CHANGE: iterating a tuple of mutable types will now raise an error as they cannot be copied --- src/puya/awst/validation/arc4_copy.py | 26 +++++++ src/puyapy/awst_build/eb/arc4/_base.py | 5 +- tests/test_expected_output/arc4.test | 3 +- .../test_expected_output/expected_errors.test | 70 ++++++++++++++++++- 4 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/puya/awst/validation/arc4_copy.py b/src/puya/awst/validation/arc4_copy.py index cc25a49771..a6ab9a9a63 100644 --- a/src/puya/awst/validation/arc4_copy.py +++ b/src/puya/awst/validation/arc4_copy.py @@ -44,10 +44,36 @@ def visit_tuple_expression(self, expr: awst_nodes.TupleExpression) -> None: _check_for_arc4_copy(item, "being passed to a tuple expression") def visit_for_in_loop(self, statement: awst_nodes.ForInLoop) -> None: + # statement.items is immediately checked before entering the for body + # so don't need to worry about preserving _for_items through multiple loops self._for_items = statement.items super().visit_for_in_loop(statement) self._for_items = None + # looping is essentially assigning so check sequence + sequence = statement.sequence + while isinstance(sequence, awst_nodes.Enumeration | awst_nodes.Reversed): + sequence = sequence.expr + if ( # mutable tuples cannot be iterated in a semantically correct way + isinstance(sequence.wtype, wtypes.WTuple) + and _is_referable_expression(sequence) + and _is_arc4_mutable(sequence.wtype) + ): + logger.error( + "tuple of mutable ARC4 values cannot be iterated", + location=sequence.source_location, + ) + elif ( # arrays of mutable types, must be modified and iterated by index + isinstance(sequence.wtype, wtypes.ARC4Array) + and _is_referable_expression(sequence) + and _is_arc4_mutable(sequence.wtype.element_type) + ): + logger.error( + "cannot directly iterate an ARC4 array of mutable objects," + " construct a for-loop over the indexes instead", + location=sequence.source_location, + ) + def visit_assignment_expression(self, expr: awst_nodes.AssignmentExpression) -> None: _check_assignment(expr.target, expr.value) expr.value.accept(self) diff --git a/src/puyapy/awst_build/eb/arc4/_base.py b/src/puyapy/awst_build/eb/arc4/_base.py index a779665c09..4a02657ddf 100644 --- a/src/puyapy/awst_build/eb/arc4/_base.py +++ b/src/puyapy/awst_build/eb/arc4/_base.py @@ -138,9 +138,8 @@ class _ARC4ArrayExpressionBuilder(BytesBackedInstanceExpressionBuilder[pytypes.A @typing.override def iterate(self) -> Expression: if not self.pytype.items.wtype.immutable: - logger.error( - "cannot directly iterate an ARC4 array of mutable objects," - " construct a for-loop over the indexes via urange(.length) instead", + logger.info( + "use `algopy.urange(.length)` to iterate by index", location=self.source_location, ) return self.resolve() diff --git a/tests/test_expected_output/arc4.test b/tests/test_expected_output/arc4.test index 0303692039..961a62b342 100644 --- a/tests/test_expected_output/arc4.test +++ b/tests/test_expected_output/arc4.test @@ -206,7 +206,8 @@ class Arc4CopyContract(arc4.ARC4Contract): # Can't create copies by iterating valid_array_of_arrays = arc4.DynamicArray(local_array.copy()) - for an_array in valid_array_of_arrays: ## E: cannot directly iterate an ARC4 array of mutable objects, construct a for-loop over the indexes via urange(.length) instead + for an_array in valid_array_of_arrays: ## N: use `algopy.urange(.length)` to iterate by index \ + ## E: cannot directly iterate an ARC4 array of mutable objects, construct a for-loop over the indexes instead assert an_array.length ## case: copy_arc4_struct diff --git a/tests/test_expected_output/expected_errors.test b/tests/test_expected_output/expected_errors.test index 2ba18e372b..d6781c09ee 100644 --- a/tests/test_expected_output/expected_errors.test +++ b/tests/test_expected_output/expected_errors.test @@ -83,7 +83,8 @@ def okay1() -> None: @subroutine def baddie1() -> None: arr_of_arr = arc4.DynamicArray[arc4.DynamicBytes]() - for arr in arr_of_arr: ## E: cannot directly iterate an ARC4 array of mutable objects, construct a for-loop over the indexes via urange(.length) instead + for arr in arr_of_arr: ## E: cannot directly iterate an ARC4 array of mutable objects, construct a for-loop over the indexes instead \ + ## N: use `algopy.urange(.length)` to iterate by index arr.append(arc4.Byte(1)) @@ -97,7 +98,8 @@ def okay2() -> None: @subroutine def baddie2() -> None: arr_of_tup_with_arr = arc4.DynamicArray[arc4.Tuple[arc4.DynamicBytes, arc4.UInt64]]() - for tup in arr_of_tup_with_arr: ## E: cannot directly iterate an ARC4 array of mutable objects, construct a for-loop over the indexes via urange(.length) instead + for tup in arr_of_tup_with_arr: ## E: cannot directly iterate an ARC4 array of mutable objects, construct a for-loop over the indexes instead \ + ## N: use `algopy.urange(.length)` to iterate by index tup[0].append(arc4.Byte(1)) @@ -366,3 +368,67 @@ def unresolveable() -> None: one = UInt64(1) assert bool(op.bitlen(empty or one)) ## E: type unions are unsupported at this location bad = empty or one ## E: expression would produce a union type, which isn't supported unless evaluating a boolean condition + + +## case: iterate_tuple_mutable_types + +from algopy import * + + +class TupleIterationMutation(ARC4Contract): + + @arc4.abimethod() + def test_iteration_mutation(self, param: tuple[arc4.DynamicBytes, arc4.DynamicBytes, arc4.DynamicBytes]) -> None: + items = get_dynamic_bytes() + a, b, c = get_dynamic_bytes() + + # not ok as both item and items[n] refer to the same values + for item in items: ## E: tuple of mutable ARC4 values cannot be iterated + item.append(arc4.Byte()) + + for item in param: ## E: tuple of mutable ARC4 values cannot be iterated + item.append(arc4.Byte()) + + for item in (a, b, c): ## E: mutable reference to ARC4-encoded value must be copied using .copy() when being passed to a tuple expression + item.append(arc4.Byte()) + + for outer in Bytes(b" "): + for item in items: ## E: tuple of mutable ARC4 values cannot be iterated + item.append(arc4.Byte()) + + for item in items: ## E: tuple of mutable ARC4 values cannot be iterated + for inner in Bytes(b" "): + item.append(arc4.Byte()) + + for item in reversed(items): ## E: tuple of mutable ARC4 values cannot be iterated + item.append(arc4.Byte()) + + for idx, item in uenumerate(items): ## E: tuple of mutable ARC4 values cannot be iterated + item.append(arc4.Byte(idx + 1)) + + # scenarios that are ok as there is no other reference + for item in (a.copy(), b.copy(), c.copy()): + item.append(arc4.Byte()) + + for item in get_dynamic_bytes(): + item.append(arc4.Byte()) + + for outer in Bytes(b" "): + for item in get_dynamic_bytes(): + item.append(arc4.Byte()) + + for item in get_dynamic_bytes(): + for inner in Bytes(b" "): + item.append(arc4.Byte()) + + for item in reversed(get_dynamic_bytes()): + item.append(arc4.Byte()) + + for idx, item in uenumerate(get_dynamic_bytes()): + item.append(arc4.Byte(idx + 1)) + + + +@subroutine +def get_dynamic_bytes() -> tuple[arc4.DynamicBytes, arc4.DynamicBytes, arc4.DynamicBytes]: + return (arc4.DynamicBytes(), arc4.DynamicBytes(), arc4.DynamicBytes())