Repository: pyccel/pyccel (python)

Issue:
Bug in semantic parser when sliced array is modified in function call
**Describe the bug**

Pull request #716 introduced a regression bug at the semantic stage, which appears when these conditions are met:
- One function modifies an input array (which has `intent(inout)`);
- Another function calls the first function passing a sliced array to it;
- Both functions are pyccelized.

**To Reproduce**

Let's consider file `test_view.py`:

```python
import numpy as np

def f(x : 'float[:]'):
    x[0] = 1

def g():
    x = np.zeros(3)
    f(x[:])
    print(x)
```

Pyccel fails during the semantic stage:
```bash/fortran/c
$ pyccel test_view.py 
Traceback (most recent call last):
  File "/home/yamanguc/.local/bin/pyccel", line 11, in <module>
    load_entry_point('pyccel', 'console_scripts', 'pyccel')()
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/commands/console.py", line 258, in pyccel
    folder        = args.output)
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/codegen/pipeline.py", line 247, in execute_pyccel
    parser.annotate(**settings)
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/parser.py", line 191, in annotate
    **settings)
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 173, in __init__
    self.annotate()
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 209, in annotate
    ast = self._visit(ast, **settings)
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 802, in _visit
    obj = getattr(self, annotation_method)(expr, **settings)
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 838, in _visit_CodeBlock
    ls = [self._visit(i, **settings) for i in expr.body]
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 838, in <listcomp>
    ls = [self._visit(i, **settings) for i in expr.body]
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 802, in _visit
    obj = getattr(self, annotation_method)(expr, **settings)
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 2593, in _visit_FunctionDef
    assigned     = [a.name for a in all_assigned if a.rank > 0]
  File "/home/yamanguc/Library/PYCCEL/pyccel/pyccel/parser/semantic.py", line 2593, in <listcomp>
    assigned     = [a.name for a in all_assigned if a.rank > 0]
AttributeError: 'IndexedElement' object has no attribute 'name'

```

**Expected behavior**

Pyccel should be able to process file `test_view.py` without any problems, and a call to `g()` should print `[1, 0, 0]`.

**Language**

This bug is independent of the target language, as it happens during the semantic stage.

Pull request:
Fix bug in semantic parser when sliced array is modified in function call
Collect all `Variable` objects from assigned objects to fix #803 

**Commit Summary**

- Add test in a module;
- Remove obsolete `get_assigned_symbols` function;
- Add `__str__` method to `IndexedElement`;
- Replace call to `get_assigned_symbols` with 2 calls to `get_attribute_nodes` and 2 list comprehensions;
- Collect `Variable` objects from all assigned objects (fixes #803);
- [BUGFIX] Check function name not class name when assembling `apps` variable;
- Update patch number: new library version is 1.2.1.

Patch:
diff --git a/pyccel/ast/core.py b/pyccel/ast/core.py
index 22b1837d..9b7c7f63 100644
--- a/pyccel/ast/core.py
+++ b/pyccel/ast/core.py
@@ -90,7 +90,6 @@ __all__ = (
     'With',
     'create_variable',
     'create_incremented_string',
-    'get_assigned_symbols',
     'get_initial_value',
     'get_iterable_ranges',
     'inline',
@@ -3517,65 +3516,6 @@ def get_initial_value(expr, var):
     return Nil()
 
 
-# ...
-
-# ... TODO treat other statements
-
-def get_assigned_symbols(expr):
-    """Returns all assigned symbols in the AST.
-
-    Parameters
-    ----------
-    expr: Expression
-        any AST valid expression
-    """
-
-    if isinstance(expr, (CodeBlock, FunctionDef, For, While)):
-        return get_assigned_symbols(expr.body)
-    elif isinstance(expr, FunctionalFor):
-        return get_assigned_symbols(expr.loops)
-    elif isinstance(expr, If):
-
-        return get_assigned_symbols(expr.bodies)
-
-    elif iterable(expr):
-        symbols = []
-
-        for a in expr:
-            symbols += get_assigned_symbols(a)
-        symbols = set(symbols)
-        symbols = list(symbols)
-        return symbols
-    elif isinstance(expr, (Assign, AugAssign)):
-
-
-        if expr.lhs is None:
-            raise TypeError('Found None lhs')
-
-        var = expr.lhs
-        symbols = []
-        if isinstance(var, DottedVariable):
-            var = expr.lhs
-            while isinstance(var, DottedVariable):
-                var = var.lhs
-            symbols.append(var)
-        elif isinstance(var, IndexedElement):
-            var = var.base
-            symbols.append(var)
-        elif isinstance(var, Variable):
-            symbols.append(var)
-        return symbols
-    elif isinstance(expr, FunctionCall):
-        f = expr.funcdef
-        symbols = []
-        for func_arg, inout in zip(expr.args,f.arguments_inout):
-            if inout:
-                symbols.append(func_arg)
-        return symbols
-
-    return []
-
-
 # ...
 
 # ... TODO: improve and make it recursive
diff --git a/pyccel/ast/literals.py b/pyccel/ast/literals.py
index 528112d3..7482d11e 100644
--- a/pyccel/ast/literals.py
+++ b/pyccel/ast/literals.py
@@ -231,6 +231,9 @@ class Nil(Basic, metaclass=Singleton):
     def __eq__(self, other):
         return isinstance(other, Nil)
 
+    def __hash__(self):
+        return hash('Nil')+hash(None)
+
 #------------------------------------------------------------------------------
 
 def get_default_literal_value(dtype):
diff --git a/pyccel/ast/variable.py b/pyccel/ast/variable.py
index f2e81530..f244f5a8 100644
--- a/pyccel/ast/variable.py
+++ b/pyccel/ast/variable.py
@@ -796,6 +796,9 @@ class IndexedElement(PyccelAstNode):
         """
         return self._indices
 
+    def __str__(self):
+        return '{}[{}]'.format(self.base, ','.join(str(i) for i in self.indices))
+
 class VariableAddress(PyccelAstNode):
 
     """Represents the address of a variable.
diff --git a/pyccel/parser/semantic.py b/pyccel/parser/semantic.py
index 1ca44666..7dce6e5d 100644
--- a/pyccel/parser/semantic.py
+++ b/pyccel/parser/semantic.py
@@ -52,7 +52,6 @@ from pyccel.ast.core import With
 from pyccel.ast.builtins import PythonList
 from pyccel.ast.core import Dlist
 from pyccel.ast.core import StarredArguments
-from pyccel.ast.core import get_assigned_symbols
 from pyccel.ast.operators import PyccelIs, PyccelIsNot, IfTernaryOperator
 from pyccel.ast.itertoolsext import Product
 
@@ -2633,12 +2632,22 @@ class SemanticParser(BasicParser):
 
             results_names = [i.name for i in results]
 
-            all_assigned = get_assigned_symbols(body)
-            assigned     = [a.name for a in all_assigned if a.rank > 0]
-            all_assigned = [i.name for i in all_assigned]
+            # Find all nodes which can modify variables
+            assigns = body.get_attribute_nodes(Assign, excluded_nodes = (FunctionCall,))
+            calls   = body.get_attribute_nodes(FunctionCall)
 
-            apps = body.get_attribute_nodes(FunctionCall)
-            apps = [i for i in apps if (i.__class__.__name__
+            # Collect the modified objects
+            lhs_assigns   = [a.lhs for a in assigns]
+            modified_args = [func_arg for f in calls
+                                for func_arg, inout in zip(f.args,f.funcdef.arguments_inout) if inout]
+            # Collect modified variables
+            all_assigned = [v for a in (lhs_assigns + modified_args) for v in
+                            (a.get_attribute_nodes(Variable) if not isinstance(a, Variable) else [a])]
+
+            permanent_assign = [a.name for a in all_assigned if a.rank > 0]
+            local_assign     = [i.name for i in all_assigned]
+
+            apps = [i for i in calls if (i.funcdef.name
                     in self.get_parent_functions())]
 
             d_apps = OrderedDict((a, []) for a in args)
@@ -2648,7 +2657,7 @@ class SemanticParser(BasicParser):
                     d_apps[a].append(f)
 
             for i, a in enumerate(args):
-                if a.name in chain(results_names, assigned, ['self']):
+                if a.name in chain(results_names, permanent_assign, ['self']):
                     args_inout[i] = True
 
                 if d_apps[a] and not( args_inout[i] ):
@@ -2657,7 +2666,7 @@ class SemanticParser(BasicParser):
                     i_fa = 0
                     while not(intent) and i_fa < n_fa:
                         fa = d_apps[a][i_fa]
-                        f_name = fa.__class__.__name__
+                        f_name = fa.funcdef.name
                         func = self.get_function(f_name)
 
                         j = list(fa.args).index(a)
@@ -2667,7 +2676,7 @@ class SemanticParser(BasicParser):
 
                         i_fa += 1
                 if isinstance(a, Variable):
-                    if a.is_const and (args_inout[i] or (a.name in all_assigned)):
+                    if a.is_const and (args_inout[i] or (a.name in local_assign)):
                         msg = "Cannot modify 'const' argument ({})".format(a)
                         errors.report(msg, bounding_box=(self._current_fst_node.lineno,
                             self._current_fst_node.col_offset),
diff --git a/pyccel/version.py b/pyccel/version.py
index c68196d1..a955fdae 100644
--- a/pyccel/version.py
+++ b/pyccel/version.py
@@ -1,1 +1,1 @@
-__version__ = "1.2.0"
+__version__ = "1.2.1"


Test patch:
diff --git a/tests/epyccel/modules/Module_6.py b/tests/epyccel/modules/Module_6.py
new file mode 100644
index 00000000..fccade2b
--- /dev/null
+++ b/tests/epyccel/modules/Module_6.py
@@ -0,0 +1,30 @@
+# pylint: disable=missing-function-docstring, missing-module-docstring/
+
+def set_i(x : 'float[:]', i : 'int', val : 'float'):
+    x[i] = val
+
+def swap(x : 'float[:]', i : 'int', j : 'int'):
+    temp = x[i]
+    x[i] = x[j]
+    x[j] = temp
+
+def inplace_max(x : 'float[:]'):
+    n = x.shape[0]
+    # bubble sort
+    for j in range(n):
+        i = n-1-j
+        while i < n-1:
+            if x[i] > x[i+1]:
+                swap(x, i, i+1)
+            else:
+                i+=1
+    return x[-1]
+
+def f(x : 'float[:]', y : 'float[:]'):
+    n = x.shape[0]
+    for i in range(n):
+        set_i(x, i, float(i))
+    for i in range(n//3):
+        set_i(x[::3], i, -1.0)
+    b = inplace_max(y[:])
+    return b
diff --git a/tests/epyccel/recognised_functions/test_numpy_funcs.py b/tests/epyccel/recognised_functions/test_numpy_funcs.py
index 8c853635..cdab6c85 100644
--- a/tests/epyccel/recognised_functions/test_numpy_funcs.py
+++ b/tests/epyccel/recognised_functions/test_numpy_funcs.py
@@ -5172,6 +5172,16 @@ def test_numpy_double_scalar(language):
     assert f_fl64_output == test_float64_output
     assert matching_types(f_fl64_output, test_float64_output)
 
+@pytest.mark.parametrize( 'language', (
+        pytest.param("fortran", marks = pytest.mark.fortran),
+        pytest.param("c", marks = pytest.mark.c),
+        pytest.param("python", marks = [
+            pytest.mark.skip(reason=("complex handles bool types in __new__ so it "
+                "cannot be used in a translated interface in python. See #802")),
+            pytest.mark.python]
+        )
+    )
+)
 def test_numpy_complex64_scalar(language):
 
     @types('bool')
diff --git a/tests/epyccel/test_epyccel_modules.py b/tests/epyccel/test_epyccel_modules.py
index 950e848e..52f76078 100644
--- a/tests/epyccel/test_epyccel_modules.py
+++ b/tests/epyccel/test_epyccel_modules.py
@@ -68,3 +68,23 @@ def test_module_3(language):
     x_expected = mod.circle_volume(r)
     x = modnew.circle_volume(r)
     assert np.isclose( x, x_expected, rtol=1e-14, atol=1e-14 )
+
+def test_module_4(language):
+    import modules.Module_6 as mod
+
+    modnew = epyccel(mod, language=language)
+
+    n_x = np.random.randint(4,20)
+    n_y = np.random.randint(4,20)
+
+    x = np.empty(n_x, dtype=float)
+    y = np.random.random_sample(n_y)
+
+    x_pyc = x.copy()
+    y_pyc = y.copy()
+
+    max_pyt = mod.f(x,y)
+    max_pyc = modnew.f(x_pyc, y_pyc)
+    assert np.isclose( max_pyt, max_pyc, rtol=1e-14, atol=1e-14 )
+    assert np.allclose( x, x_pyc, rtol=1e-14, atol=1e-14 )
+    assert np.allclose( y, y_pyc, rtol=1e-14, atol=1e-14 )