Repository: pyccel/pyccel (python)

Issue:
Add support/error for Pythonic swap
### Describe the bug
Ideally add support for a Pythonic swap, if this proves too difficult to implement then an error should be raised to warn users that the translation does not match their expectations

### To Reproduce
Provide code to reproduce the behavior:
```python
if __name__ == '__main__':
    a = 1
    b = 2
    a, b = b, a
    print(a, b)
```

### Error details
Provide the generated code, or the error message:
```c
int main()
{
    int64_t a;
    int64_t b;
    a = INT64_C(1);
    b = INT64_C(2);
    a = b;
    b = a;
    printf("%"PRId64" %"PRId64"\n", a, b);
    return 0;
}
```

### Expected behavior
Ideally the code should be:
```c
int main()
{
    int64_t a;
    int64_t b;
    int64_t Dummy_0000;
    a = INT64_C(1);
    b = INT64_C(2);
    Dummy_0000 = b;
    b = a;
    a = b;
    printf("%"PRId64" %"PRId64"\n", a, b);
    return 0;
}
```

### Language
All languages (including Python)

### Additional context
When fixing this we should bear in mind that a swap can contain more than 2 variables. The `get_attribute_nodes` function will be useful for this issue


Pull request:
Fix swap
Add support for Pythonic swapping and raise an error for expressions which are too complex. Fixes #2104 

**Commit Summary**
- Add `is_equivalent` method to `pyccel.ast.base.PyccelAstNode.substitute` to allow more flexibility in how items are recognised
- Make `symbol` argument of `PyccelError` into a string to ensure it is printed before `substitute` is called if possible
- Add support for Pythonic swapping 
- Add tests

Patch:
diff --git a/.dict_custom.txt b/.dict_custom.txt
index 8246c1c5..93e0ea93 100644
--- a/.dict_custom.txt
+++ b/.dict_custom.txt
@@ -124,3 +124,4 @@ STC's
 gFTL's
 CMake
 suboptimal
+pythonic
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 110565dd..950cf7ec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -120,6 +120,7 @@ All notable changes to this project will be documented in this file.
 -   #2195 : Fix string comparisons.
 -   Fixed returning strings from functions.
 -   #2197 : Allow strings as dictionary keys in C.
+-   #2104 : Add support for Pythonic swapping and raise errors for expressions which are too complex.
 -   Lifted the restriction on ndarrays limiting them to rank<15.
 -   #2175 : Fix the shape of multi-level containers.
 -   Catch all internal errors arising in the syntactic, semantic, printing or code generation stages.
diff --git a/pyccel/ast/basic.py b/pyccel/ast/basic.py
index 0cc38c12..7da71db6 100644
--- a/pyccel/ast/basic.py
+++ b/pyccel/ast/basic.py
@@ -256,7 +256,8 @@ class PyccelAstNode:
         node.toggle_recursion()
         return False
 
-    def substitute(self, original, replacement, excluded_nodes = (), invalidate = True):
+    def substitute(self, original, replacement, excluded_nodes = (), invalidate = True,
+            is_equivalent = None):
         """
         Substitute object 'original' for object 'replacement' in the code.
 
@@ -274,6 +275,12 @@ class PyccelAstNode:
         invalidate : bool
                     Indicates whether the removed object should
                     be invalidated.
+        is_equivalent : function, optional
+                    A function that compares the original object to the object
+                    in the PyccelAstNode to determine if it is the object that
+                    we are searching for. Usually this is an equality in the
+                    syntactic stage and an identity comparison in the semantic
+                    stage, but occasionally a different choice may be useful.
         """
         if self._recursion_in_progress:
             return
@@ -291,6 +298,12 @@ class PyccelAstNode:
             original = (original,)
             replacement = (replacement,)
 
+        if is_equivalent is None:
+            if self.pyccel_staging == 'syntactic':
+                is_equivalent = lambda x, y: x == y #pylint:disable=unnecessary-lambda-assignment
+            else:
+                is_equivalent = lambda x, y: x is y #pylint:disable=unnecessary-lambda-assignment
+
         def prepare_sub(found_node):
             idx = original.index(found_node)
             rep = replacement[idx]
@@ -311,7 +324,7 @@ class PyccelAstNode:
             if isinstance(v, excluded_nodes):
                 continue
 
-            elif any(v is oi for oi in original):
+            elif any(is_equivalent(v, oi) for oi in original):
                 setattr(self, n, prepare_sub(v))
 
             elif isinstance(v, tuple):
@@ -319,17 +332,17 @@ class PyccelAstNode:
                 for vi in v:
                     new_vi = vi
                     if not isinstance(vi, excluded_nodes):
-                        if any(vi is oi for oi in original):
+                        if any(is_equivalent(vi, oi) for oi in original):
                             new_vi = prepare_sub(vi)
                         elif not self._ignore(vi):
-                            vi.substitute(original, replacement, excluded_nodes, invalidate)
+                            vi.substitute(original, replacement, excluded_nodes, invalidate, is_equivalent)
                     if iterable(new_vi):
                         new_v.extend(new_vi)
                     else:
                         new_v.append(new_vi)
                 setattr(self, n, tuple(new_v))
             elif not self._ignore(v):
-                v.substitute(original, replacement, excluded_nodes, invalidate)
+                v.substitute(original, replacement, excluded_nodes, invalidate, is_equivalent)
         self._recursion_in_progress = False
 
     @property
diff --git a/pyccel/ast/datatypes.py b/pyccel/ast/datatypes.py
index d531cda7..b77f37ca 100644
--- a/pyccel/ast/datatypes.py
+++ b/pyccel/ast/datatypes.py
@@ -809,14 +809,14 @@ class HomogeneousListType(HomogeneousContainerType, metaclass = ArgumentSingleto
 
     def __init__(self, element_type):
         assert isinstance(element_type, PyccelType)
+        self._element_type = element_type
+        self._order = 'C' if (element_type.order == 'C' or element_type.rank == 1) else None
         if element_type.rank > 0:
             # When this error is removed the pop() of the warning must also be removed
             # in parser/semantic.py before the creation of the AllDeclaration node
             errors.report("Lists of non-scalar objects are not yet fully supported. " +
                     "Using containers in lists may lead to bugs such as memory leaks",
                     symbol=self, severity="warning")
-        self._element_type = element_type
-        self._order = 'C' if (element_type.order == 'C' or element_type.rank == 1) else None
         super().__init__()
 
     def __eq__(self, other):
diff --git a/pyccel/errors/errors.py b/pyccel/errors/errors.py
index dbacb204..bcd12c23 100644
--- a/pyccel/errors/errors.py
+++ b/pyccel/errors/errors.py
@@ -105,8 +105,9 @@ class ErrorInfo:
     severity : str, optional
         The severity of the error. This is one of : [warning/error/fatal].
 
-    symbol : pyccel.ast.basic.PyccelAstNode, optional
-        The PyccelAstNode object which caused the error to need to be raised.
+    symbol : str, optional
+        A string representation of the PyccelAstNode object which caused
+        the error to need to be raised.
         This object is printed in the error message.
 
     traceback : str, optional
@@ -163,10 +164,7 @@ class ErrorInfo:
                 info['location'] = ' [{line}]'.format(line=self.line)
 
         if self.symbol:
-            if self.traceback:
-                info['symbol'] = ' ({})'.format(repr(self.symbol))
-            else:
-                info['symbol'] = ' ({})'.format(self.symbol)
+            info['symbol'] = f' ({self.symbol})'
 
         return pattern.format(**info)
 
@@ -329,6 +327,11 @@ class Errors(metaclass = Singleton):
             elif isinstance(symbol, PyccelAstNode):
                 ast_node = symbol.python_ast
 
+            if self.mode == 'developer':
+                symbol = repr(symbol)
+            else:
+                symbol = str(symbol)
+
         if ast_node:
             if line is None:
                 line   = getattr(ast_node, 'lineno', None)
diff --git a/pyccel/parser/semantic.py b/pyccel/parser/semantic.py
index d3f92a50..0e373a4f 100644
--- a/pyccel/parser/semantic.py
+++ b/pyccel/parser/semantic.py
@@ -3441,13 +3441,64 @@ class SemanticParser(BasicParser):
             except RuntimeError as e:
                 errors.report(e, symbol=expr, severity='error')
 
-        if isinstance(rhs, PythonTuple):
+        if isinstance(rhs, (PythonTuple, PythonList)):
             assign_elems = None
             if isinstance(lhs, PythonTuple):
+                # Create variables to handle swap expressions
+                unsaved_vars = set()
                 pyccel_stage.set_stage('syntactic')
-                syntactic_assign_elems = [Assign(l, r, python_ast=expr.python_ast) for l, r in zip(lhs, rhs)]
+                unsaved_vars = set(rhs.get_attribute_nodes((PyccelSymbol, DottedName, IndexedElement),
+                                                            excluded_nodes = (FunctionDef,)))
                 pyccel_stage.set_stage('semantic')
-                assign_elems = [self._visit(a) for a in syntactic_assign_elems]
+
+                # Test if the expression describes a basic swap or if the rhs contains expressions
+                # (e.g. arithmetic expressions or further tuples)
+                # using variables from the left-hand side.
+                modified_vars = set(lhs.get_attribute_nodes((PyccelSymbol, DottedName, IndexedElement)))
+                used_vars = set(rhs.get_attribute_nodes((PyccelSymbol, DottedName, IndexedElement),
+                                    excluded_nodes = (FunctionDef,)))
+                trivial_assign = len(modified_vars.intersection(unsaved_vars)) == 0
+                all_indexed_are_simple = all(all(isinstance(idx, (PyccelSymbol, DottedName, Literal)) for idx in elem.indices)
+                                             for elem in modified_vars if isinstance(elem, IndexedElement))
+                if not trivial_assign and (used_vars.intersection(modified_vars).difference(unsaved_vars) or not all_indexed_are_simple):
+                    errors.report("Assign statement is too complex. It seems that some of the variables used non-trivially on the right-hand side appear on the left-hand side.",
+                            severity='error', symbol=expr)
+
+                assign_elems = []
+                for i, l in enumerate(lhs):
+                    r = rhs[i]
+                    # Get unsaved variables that are still needed
+                    pyccel_stage.set_stage('syntactic')
+                    tmp_rhs_tuple = PythonTuple(*rhs.args[i+1:])
+                    unsaved_vars = set(tmp_rhs_tuple.get_attribute_nodes((PyccelSymbol, DottedName, IndexedElement),
+                                                                         excluded_nodes = (FunctionDef,)))
+                    pyccel_stage.set_stage('semantic')
+
+                    # If the lhs element has not yet been saved to a variable create a new
+                    # variable to hold this value
+                    if l in unsaved_vars:
+                        temp = self.scope.get_new_name()
+                        pyccel_stage.set_stage('syntactic')
+                        local_assign = Assign(temp, l, python_ast = expr.python_ast)
+                        pyccel_stage.set_stage('semantic')
+                        assign_elems.append(self._visit(local_assign))
+                        # Save the variable containing the value to rhs so it can be
+                        # used when it appears in the assignment
+                        if isinstance(l, IndexedElement):
+                            # A list is required for IndexedElements as they are not singletons
+                            l_list = [r for r in rhs.get_attribute_nodes(IndexedElement) if r == l]
+                        else:
+                            l_list = [l]
+                        for l_elem in l_list:
+                            rhs.substitute(l_elem, temp)
+                        if r == l:
+                            r = temp
+
+                    # Check for a replacement right-hand side if the rhs is found among the lhs variables
+                    pyccel_stage.set_stage('syntactic')
+                    local_assign = Assign(l, r, python_ast = expr.python_ast)
+                    pyccel_stage.set_stage('semantic')
+                    assign_elems.append(self._visit(local_assign))
             elif isinstance(lhs, (PyccelSymbol, DottedName)):
                 semantic_lhs = self.scope.find(lhs)
                 if semantic_lhs and isinstance(semantic_lhs.class_type, InhomogeneousTupleType):
@@ -3672,7 +3723,7 @@ class SemanticParser(BasicParser):
                 body = []
                 for i,(l,r) in enumerate(zip(lhs,r_iter)):
                     pyccel_stage.set_stage('syntactic')
-                    local_assign = Assign(l,r, python_ast = expr.python_ast)
+                    local_assign = Assign(l, r, python_ast = expr.python_ast)
                     pyccel_stage.set_stage('semantic')
                     body.append(self._visit(local_assign))
                 return CodeBlock(body)
@@ -3907,7 +3958,9 @@ class SemanticParser(BasicParser):
     def _visit_FunctionalFor(self, expr):
         old_index   = expr.index
         new_index   = self.scope.get_new_name()
-        expr.substitute(old_index, new_index)
+        # Identity comparison is required to only find duplicates of the index caused
+        # by the construction of FunctionalFor.
+        expr.substitute(old_index, new_index, is_equivalent = lambda x,y: x is y)
 
         target  = expr.expr
         index   = new_index


Test patch:
diff --git a/tests/epyccel/test_arrays_multiple_assignments.py b/tests/epyccel/test_arrays_multiple_assignments.py
index e8ecd2c4..53592916 100644
--- a/tests/epyccel/test_arrays_multiple_assignments.py
+++ b/tests/epyccel/test_arrays_multiple_assignments.py
@@ -59,7 +59,7 @@ def test_reallocation_heap(language):
 
     # Check that the warning is correct
     warning_info = [*errors.error_info_map.values()][0][0]
-    assert warning_info.symbol  == 'x'
+    assert warning_info.symbol in ('x', "'x'")
     expected_msg = ARRAY_REALLOCATION.split()[1:]
     assert warning_info.message.split()[-len(expected_msg):] == expected_msg
 
@@ -86,7 +86,7 @@ def test_reallocation_stack(language):
 
     # Check that the error is correct
     error_info = [*errors.error_info_map.values()][0][0]
-    assert error_info.symbol  == 'x'
+    assert error_info.symbol in ('x', "'x'")
     assert error_info.message == INCOMPATIBLE_REDEFINITION_STACK_ARRAY
 
 #==============================================================================
@@ -113,7 +113,7 @@ def test_creation_in_loop_heap(language):
 
     # Check that the warning is correct
     warning_info = [*errors.error_info_map.values()][0][0]
-    assert warning_info.symbol  == 'x'
+    assert warning_info.symbol in ('x', "'x'")
     assert warning_info.message == ARRAY_DEFINITION_IN_LOOP
 
 #==============================================================================
@@ -143,7 +143,7 @@ def test_creation_in_loop_stack(language):
     # Check that the errors are correct
     error_info_list = [*errors.error_info_map.values()][0]
     error_info = error_info_list[0]
-    assert error_info.symbol  == 'x'
+    assert error_info.symbol in ('x', "'x'")
     assert error_info.message == STACK_ARRAY_UNKNOWN_SHAPE
     if errors.mode != 'developer':
         error_info = error_info_list[1]
@@ -212,7 +212,7 @@ def test_Reassign_to_Target():
 
     # Check that the error is correct
     error_info = [*errors.error_info_map.values()][0][0]
-    assert error_info.symbol  == 'x'
+    assert error_info.symbol in ('x', "'x'")
     assert error_info.message == ARRAY_ALREADY_IN_USE
 
 #==============================================================================
@@ -240,7 +240,7 @@ def test_Assign_Between_Allocatables():
 
     # Check that the error is correct
     error_info = [*errors.error_info_map.values()][0][0]
-    assert str(error_info.symbol)  == 'x'
+    assert str(error_info.symbol) in ('x', "Variable(x, type=numpy.int64[:,:](order=C))")
     assert error_info.message == ASSIGN_ARRAYS_ONE_ANOTHER
 
 #==============================================================================
@@ -270,7 +270,7 @@ def test_Assign_after_If():
 
     # Check that the warning is correct
     warning_info = [*errors.error_info_map.values()][0][0]
-    assert warning_info.symbol  == 'x'
+    assert warning_info.symbol in ('x', "'x'")
     expected_msg = ARRAY_REALLOCATION.split()[1:]
     assert warning_info.message.split()[-len(expected_msg):] == expected_msg
 
diff --git a/tests/epyccel/test_epyccel_expressions.py b/tests/epyccel/test_epyccel_expressions.py
new file mode 100644
index 00000000..ef3f6f30
--- /dev/null
+++ b/tests/epyccel/test_epyccel_expressions.py
@@ -0,0 +1,156 @@
+# pylint: disable=missing-function-docstring, missing-module-docstring
+from numpy.random import randint, rand
+import numpy as np
+from numpy import iinfo
+from pyccel import epyccel
+
+# Use int32 for Windows compatibility
+min_int = iinfo(np.int32).min
+max_int = iinfo(np.int32).max
+
+def test_swap_basic(language):
+    def swp(a : int, b : int):
+        (a, b) = (b, a)
+        return a, b
+
+    f = epyccel(swp, language=language)
+    assert f(2,4) == swp(2,4)
+    assert f(-2,4) == swp(-2,4)
+    assert f(4,100) == swp(4,100)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    assert f(x,y) == swp(x,y)
+
+def test_swap_basic_2(language):
+    def swp(a : int, b : int):
+        a, b = b, a
+        return a, b
+
+    f = epyccel(swp, language=language)
+    assert f(2,4) == swp(2,4)
+    assert f(-2,4) == swp(-2,4)
+    assert f(4,100) == swp(4,100)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    assert f(x,y) == swp(x,y)
+
+def test_swap_basic_3(language):
+    def swp(a : int, b : int, c : int):
+        a, b, c = b, c, a
+        return a, b, c
+
+    f = epyccel(swp, language=language)
+    assert f(2,4,8) == swp(2,4,8)
+    assert f(-2,4,-6) == swp(-2,4,-6)
+    assert f(4,100,234) == swp(4,100,234)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    z = randint(min_int, max_int)
+    assert f(x,y,z) == swp(x,y,z)
+
+def test_swap_basic_4(language):
+    def swp(a : int, b : int, c : int):
+        a, b, c = c, b, a #pylint: disable=self-assigning-variable
+        return a, b, c
+
+    f = epyccel(swp, language=language)
+    assert f(2,4,8) == swp(2,4,8)
+    assert f(-2,4,-6) == swp(-2,4,-6)
+    assert f(4,100,234) == swp(4,100,234)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    z = randint(min_int, max_int)
+    assert f(x,y,z) == swp(x,y,z)
+
+def test_swap_index_1(language):
+    def swp(a : int, b : int, c : int):
+        l = [a,b,c]
+        l[0], l[1] = l[1], l[0]
+        return l[0], l[1], l[2]
+
+    f = epyccel(swp, language=language)
+    assert f(2,4,8) == swp(2,4,8)
+    assert f(-2,4,-6) == swp(-2,4,-6)
+    assert f(4,100,234) == swp(4,100,234)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    z = randint(min_int, max_int)
+    assert f(x,y,z) == swp(x,y,z)
+
+def test_swap_index_2(language):
+    def swp(i : int, j : int):
+        l = [1,2,3]
+        l[i], l[j] = l[j], l[i]
+        return l[0], l[1], l[2]
+
+    f = epyccel(swp, language=language)
+    assert f(0,1) == swp(0,1)
+    assert f(1,0) == swp(1,0)
+    assert f(2,1) == swp(2,1)
+
+def test_multi_level_swap(language):
+    def swp(a : int, b : int, c : int):
+        d, (b, c) = a, (c, b)
+        return a, b, c, d
+
+    f = epyccel(swp, language=language)
+    assert f(2,4,8) == swp(2,4,8)
+    assert f(-2,4,-6) == swp(-2,4,-6)
+    assert f(4,100,234) == swp(4,100,234)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    z = randint(min_int, max_int)
+    assert f(x,y,z) == swp(x,y,z)
+
+def test_multi_type_swap(language):
+    def swp(a : float, b : int, c : float, d : int):
+        a, b, c, d = c, d, a, b
+        return a, b, c, d
+
+    b = randint(min_int, max_int)
+    d = randint(min_int, max_int)
+    a = rand()*100
+    c = rand()*100
+
+    f = epyccel(swp, language=language)
+    assert f(a,b,c,d) == swp(a,b,c,d)
+    assert f(-2.,4,-6.,10) == swp(-2.,4,-6.,10)
+
+def test_tuple_assign(language):
+    def tup_assign(a : int, b : int):
+        c, d = a, a+b
+        return c, d
+
+    f = epyccel(tup_assign, language=language)
+    assert f(2,4) == tup_assign(2,4)
+    assert f(-2,4) == tup_assign(-2,4)
+    assert f(4,100) == tup_assign(4,100)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    assert f(x,y) == tup_assign(x,y)
+
+def test_tuple_assign2(language):
+    def tup_assign(a : int, b : int):
+        a, d = a, a+b #pylint: disable=self-assigning-variable
+        return a, b, d
+
+    f = epyccel(tup_assign, language=language)
+    assert f(2,4) == tup_assign(2,4)
+    assert f(-2,4) == tup_assign(-2,4)
+    assert f(4,100) == tup_assign(4,100)
+    x = randint(min_int, max_int)
+    y = randint(min_int, max_int)
+    assert f(x,y) == tup_assign(x,y)
+
+def test_tuple_assign3(language):
+    def tup_assign(a : int):
+        a, a = a+3, a+5 #pylint: disable=redeclared-assigned-name
+        return a
+
+    f = epyccel(tup_assign, language=language)
+    assert f(2) == tup_assign(2)
+    assert f(-2) == tup_assign(-2)
+    assert f(40) == tup_assign(40)
+    x = randint(min_int, max_int)
+    assert f(x) == tup_assign(x)
+
diff --git a/tests/errors/semantic/non_blocking/COMPLICATED_SWAP.py b/tests/errors/semantic/non_blocking/COMPLICATED_SWAP.py
new file mode 100644
index 00000000..8476030d
--- /dev/null
+++ b/tests/errors/semantic/non_blocking/COMPLICATED_SWAP.py
@@ -0,0 +1,9 @@
+# pylint: disable=missing-function-docstring, missing-module-docstring
+import numpy as np
+
+if __name__ == '__main__':
+    a = 1
+    b = 0
+    c = np.array([1,2,3,4])
+
+    a, b, c[a+b] = c[a+b], a, b