Skip to content

Commit

Permalink
fix: raise an error when attempting to iterate a tuple of mutable values
Browse files Browse the repository at this point in the history
BREAKING CHANGE: iterating a tuple of mutable types will now raise an error as they cannot be copied
  • Loading branch information
daniel-makerx committed Nov 1, 2024
1 parent 4de5a3e commit 2dc28c9
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 6 deletions.
26 changes: 26 additions & 0 deletions src/puya/awst/validation/arc4_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions src/puyapy/awst_build/eb/arc4/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(<array>.length) instead",
logger.info(
"use `algopy.urange(<array>.length)` to iterate by index",
location=self.source_location,
)
return self.resolve()
Expand Down
3 changes: 2 additions & 1 deletion tests/test_expected_output/arc4.test
Original file line number Diff line number Diff line change
Expand Up @@ -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(<array>.length) instead
for an_array in valid_array_of_arrays: ## N: use `algopy.urange(<array>.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
Expand Down
70 changes: 68 additions & 2 deletions tests/test_expected_output/expected_errors.test
Original file line number Diff line number Diff line change
Expand Up @@ -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(<array>.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(<array>.length)` to iterate by index
arr.append(arc4.Byte(1))


Expand All @@ -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(<array>.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(<array>.length)` to iterate by index
tup[0].append(arc4.Byte(1))


Expand Down Expand Up @@ -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())

0 comments on commit 2dc28c9

Please sign in to comment.