Repository: pyccel/pyccel
Pull request #1974 (2024-08-29 15:15:43)

Add C support for list.pop method
Add C support for list.pop method in an assign or without an index element. In order to handle negative indices `_print_PyccelArrayShapeElement` is updated to handle STC objects and `Variable.shape_can_change` is corrected.

pyccel-bot[bot]: Hello again! Thank you for this new pull request :star_struck:.

Please begin by requesting your checklist using the command `/bot checklist`

EmilyBourne: /bot checklist
pyccel-bot[bot]: Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with `/bot mark as ready`.

pyccel-bot[bot]: Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with `/bot mark as ready`.

pyccel-bot[bot]: Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with `/bot mark as ready`.

EmilyBourne: /bot run macosx
EmilyBourne: /bot run macosx
pyccel-bot[bot]: Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with `/bot mark as ready`.

pyccel-bot[bot]: Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with `/bot mark as ready`.

pyccel-bot[bot]: Hey @pyccel/pyccel-dev ! @EmilyBourne has just created this great new pull request! Check it out and let me know what you think!

EmilyBourne: > for lists of lists, issues relative to `double free` can arise when the popped item is freed along with the original list.

This point is related to #1959. But I'm not sure I understand your point. If we pop then the popped item is no longer in the original list so it will not be freed by it.

However probably a free is missing if the result is not saved (and maybe also if it is as this object is not allocated).

Do you think we should fix this here or should we leave it until we have support for lists of lists?
EmilyBourne: /bot run pr_tests
pyccel-bot[bot]: Hey @yguclu, @EmilyBourne, this PR is looking pretty good. @EmilyBourne and @jalalium think it is ready to merge. Could you add your expertise to confirm that this follows all the coding conventions and fits in Pyccel's future plans? Thanks :smile:


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1ddd193a..00a80981 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file.
 -   #1700 : Add Python support for list method `sort()`.
 -   #1696 : Add Python support for list method `copy()`.
 -   #1693 : Add Python support for list method `remove()`.
+-   #1690 : Add C support for list method `pop()`.
 -   #1739 : Add abstract class `SetMethod` to handle calls to various set methods.
 -   #1739 : Add Python support for set method `clear()`.
 -   #1740 : Add Python support for set method `copy()`.
diff --git a/docs/builtin-functions.md b/docs/builtin-functions.md
index 6d3d82d5..ff4650bb 100644
--- a/docs/builtin-functions.md
+++ b/docs/builtin-functions.md
@@ -87,7 +87,7 @@ Python contains a limited number of builtin functions defined [here](https://doc
 | `insert` | Python-only |
 | `max` | No |
 | `min` | No |
-| `pop` | Python-only |
+| `pop` | Python and C only |
 | `remove` | Python-only |
 | `reverse` | No |
 | `sort` | Python-only |
diff --git a/pyccel/ast/builtin_methods/list_methods.py b/pyccel/ast/builtin_methods/list_methods.py
index 5094649f..5dbf9a72 100644
--- a/pyccel/ast/builtin_methods/list_methods.py
+++ b/pyccel/ast/builtin_methods/list_methods.py
@@ -117,6 +117,15 @@ class ListPop(ListMethod) :
         self._class_type = list_obj.class_type.element_type
         super().__init__(list_obj, index_element)
 
+    @property
+    def index_element(self):
+        """
+        The current index value for the element to be popped.
+
+        The current index value for the element to be popped.
+        """
+        return self._args[0]
+
 #==============================================================================
 class ListClear(ListMethod) :
     """
diff --git a/pyccel/ast/variable.py b/pyccel/ast/variable.py
index e2f9d649..2f67fefe 100644
--- a/pyccel/ast/variable.py
+++ b/pyccel/ast/variable.py
@@ -13,7 +13,7 @@ from pyccel.errors.errors   import Errors
 from pyccel.utilities.stage import PyccelStage
 
 from .basic     import PyccelAstNode, TypedAstNode
-from .datatypes import PyccelType, InhomogeneousTupleType
+from .datatypes import PyccelType, InhomogeneousTupleType, HomogeneousListType, HomogeneousSetType, DictType
 from .internals import PyccelArrayShapeElement, Slice, PyccelSymbol
 from .internals import apply_pickle
 from .literals  import LiteralInteger, Nil, LiteralEllipsis
@@ -236,7 +236,7 @@ class Variable(TypedAstNode):
         bool
             Whether or not the variable shape can change in the i-th dimension.
         """
-        return self.is_alias
+        return self.is_alias or isinstance(self.class_type, (HomogeneousListType, HomogeneousSetType, DictType))
 
     def set_changeable_shape(self):
         """
diff --git a/pyccel/codegen/printing/ccode.py b/pyccel/codegen/printing/ccode.py
index 8212e367..5f228a3f 100644
--- a/pyccel/codegen/printing/ccode.py
+++ b/pyccel/codegen/printing/ccode.py
@@ -13,7 +13,7 @@ from pyccel.ast.bind_c    import BindCPointer
 
 from pyccel.ast.builtins  import PythonRange, PythonComplex
 from pyccel.ast.builtins  import PythonPrint, PythonType
-from pyccel.ast.builtins  import PythonList, PythonTuple, PythonSet, PythonDict
+from pyccel.ast.builtins  import PythonList, PythonTuple, PythonSet, PythonDict, PythonLen
 
 from pyccel.ast.core      import Declare, For, CodeBlock
 from pyccel.ast.core      import FuncAddressDeclare, FunctionCall, FunctionCallArgument
@@ -54,7 +54,7 @@ from pyccel.ast.operators import PyccelUnarySub, IfTernaryOperator
 
 from pyccel.ast.type_annotations import VariableTypeAnnotation
 
-from pyccel.ast.utilities import expand_to_loops
+from pyccel.ast.utilities import expand_to_loops, is_literal_integer
 
 from pyccel.ast.variable import IndexedElement
 from pyccel.ast.variable import Variable
@@ -244,7 +244,8 @@ c_imports = {n : Import(n, Module(n, (), ())) for n in
                  'assert',
                  'numpy_c']}
 
-import_header_guard_prefix = {'Set_extensions' : '_TOOLS_SET'}
+import_header_guard_prefix = {'Set_extensions'  : '_TOOLS_SET',
+                              'List_extensions' : '_TOOLS_LIST'}
 
 class CCodePrinter(CodePrinter):
     """
@@ -1630,9 +1631,19 @@ class CCodePrinter(CodePrinter):
 
     def _print_PyccelArrayShapeElement(self, expr):
         arg = expr.arg
-        if self.is_c_pointer(arg):
-            return '{}->shape[{}]'.format(self._print(ObjectAddress(arg)), self._print(expr.index))
-        return '{}.shape[{}]'.format(self._print(arg), self._print(expr.index))
+        if isinstance(arg.class_type, (NumpyNDArrayType, HomogeneousTupleType)):
+            idx = self._print(expr.index)
+            if self.is_c_pointer(arg):
+                arg_code = self._print(ObjectAddress(arg))
+                return f'{arg_code}->shape[{idx}]'
+            arg_code = self._print(arg)
+            return f'{arg_code}.shape[{idx}]'
+        elif isinstance(arg.class_type, (HomogeneousListType, HomogeneousSetType, DictType)):
+            c_type = self.get_c_type(arg.class_type)
+            arg_code = self._print(ObjectAddress(arg))
+            return f'{c_type}_size({arg_code})'
+        else:
+            raise NotImplementedError(f"Don't know how to represent shape of object of type {arg.class_type}")
 
     def _print_Allocate(self, expr):
         free_code = ''
@@ -2227,14 +2238,7 @@ class CCodePrinter(CodePrinter):
         if isinstance(rhs, (PythonList, PythonSet, PythonDict)):
             return prefix_code+self.init_stc_container(rhs, expr)
         rhs = self._print(expr.rhs)
-        return prefix_code+'{} = {};\n'.format(lhs, rhs)
-
-    def _print_SetPop(self, expr):
-        dtype = expr.set_variable.class_type
-        var_type = self.get_c_type(dtype)
-        self.add_import(Import('Set_extensions', AsName(VariableTypeAnnotation(dtype), var_type)))
-        set_var = self._print(ObjectAddress(expr.set_variable))
-        return f'{var_type}_pop({set_var})'
+        return prefix_code+f'{lhs} = {rhs};\n'
 
     def _print_AliasAssign(self, expr):
         lhs_var = expr.lhs
@@ -2574,6 +2578,31 @@ class CCodePrinter(CodePrinter):
         interfaces = ''.join(self._print(function) for interface in expr.interfaces for function in interface.functions)
 
         return methods + interfaces
+
+    #================== List methods ==================
+    def _print_ListPop(self, expr):
+        class_type = expr.list_obj.class_type
+        c_type = self.get_c_type(class_type)
+        list_obj = self._print(ObjectAddress(expr.list_obj))
+        if expr.index_element:
+            self.add_import(Import('List_extensions', AsName(VariableTypeAnnotation(class_type), c_type)))
+            if is_literal_integer(expr.index_element) and int(expr.index_element) < 0:
+                idx_code = self._print(PyccelAdd(PythonLen(expr.list_obj), expr.index_element, simplify=True))
+            else:
+                idx_code = self._print(expr.index_element)
+            return f'{c_type}_pull_elem({list_obj}, {idx_code})'
+        else:
+            return f'{c_type}_pull({list_obj})'
+
+    #================== Set methods ==================
+
+    def _print_SetPop(self, expr):
+        dtype = expr.set_variable.class_type
+        var_type = self.get_c_type(dtype)
+        self.add_import(Import('Set_extensions', AsName(VariableTypeAnnotation(dtype), var_type)))
+        set_var = self._print(ObjectAddress(expr.set_variable))
+        return f'{var_type}_pop({set_var})'
+
     #=================== MACROS ==================
 
     def _print_MacroShape(self, expr):
diff --git a/pyccel/codegen/utilities.py b/pyccel/codegen/utilities.py
index c6140a1a..e7863293 100644
--- a/pyccel/codegen/utilities.py
+++ b/pyccel/codegen/utilities.py
@@ -36,13 +36,14 @@ external_libs = {"stc"  : "STC/include",}
 # map internal libraries to their folders inside pyccel/stdlib and their compile objects
 # The compile object folder will be in the pyccel dirpath
 internal_libs = {
-    "ndarrays"       : ("ndarrays", CompileObj("ndarrays.c",folder="ndarrays")),
-    "pyc_math_f90"   : ("math", CompileObj("pyc_math_f90.f90",folder="math")),
-    "pyc_math_c"     : ("math", CompileObj("pyc_math_c.c",folder="math")),
-    "cwrapper"       : ("cwrapper", CompileObj("cwrapper.c",folder="cwrapper", accelerators=('python',))),
-    "numpy_f90"      : ("numpy", CompileObj("numpy_f90.f90",folder="numpy")),
-    "numpy_c"        : ("numpy", CompileObj("numpy_c.c",folder="numpy")),
-    "Set_extensions" : ("STC_Extensions", CompileObj("Set_Extensions.h", folder="STC_Extensions", has_target_file = False)),
+    "ndarrays"        : ("ndarrays", CompileObj("ndarrays.c",folder="ndarrays")),
+    "pyc_math_f90"    : ("math", CompileObj("pyc_math_f90.f90",folder="math")),
+    "pyc_math_c"      : ("math", CompileObj("pyc_math_c.c",folder="math")),
+    "cwrapper"        : ("cwrapper", CompileObj("cwrapper.c",folder="cwrapper", accelerators=('python',))),
+    "numpy_f90"       : ("numpy", CompileObj("numpy_f90.f90",folder="numpy")),
+    "numpy_c"         : ("numpy", CompileObj("numpy_c.c",folder="numpy")),
+    "Set_extensions"  : ("STC_Extensions", CompileObj("Set_Extensions.h", folder="STC_Extensions", has_target_file = False)),
+    "List_extensions" : ("STC_Extensions", CompileObj("List_Extensions.h", folder="STC_Extensions", has_target_file = False)),
 }
 internal_libs["cwrapper_ndarrays"] = ("cwrapper_ndarrays", CompileObj("cwrapper_ndarrays.c",folder="cwrapper_ndarrays",
                                                              accelerators = ('python',),
diff --git a/pyccel/parser/semantic.py b/pyccel/parser/semantic.py
index 14b884f3..1d886cfc 100644
--- a/pyccel/parser/semantic.py
+++ b/pyccel/parser/semantic.py
@@ -940,7 +940,7 @@ class SemanticParser(BasicParser):
                 bounding_box=(self.current_ast_node.lineno, self.current_ast_node.col_offset),
                 severity='fatal')
 
-        if isinstance(val.class_type, HomogeneousTupleType):
+        if isinstance(val.class_type, (HomogeneousTupleType, HomogeneousListType)):
             return Duplicate(val, length)
         else:
             if isinstance(length, LiteralInteger):
diff --git a/pyccel/stdlib/STC_Extensions/List_extensions.h b/pyccel/stdlib/STC_Extensions/List_extensions.h
new file mode 100644
index 00000000..96997d1f
--- /dev/null
+++ b/pyccel/stdlib/STC_Extensions/List_extensions.h
@@ -0,0 +1,27 @@
+#ifndef LIST_EXTENSIONS_H
+#define LIST_EXTENSIONS_H
+
+
+#define _c_MEMB(name) c_JOIN(i_type, name)
+
+// This function represents a call to the .pop() method.
+// i_type: Class type (e.g., hset_int64_t).
+// i_key: Data type of the elements in the set (e.g., int64_t).
+
+static inline i_key _c_MEMB(_pull_elem)(i_type* self, intptr_t pop_idx) {
+    // Get the iterator for the specified element using (_advance) and (_begin)
+    _c_MEMB(_iter) itr = _c_MEMB(_advance)(_c_MEMB(_begin)(self), pop_idx);
+
+    // If the element is found then remove it from the list
+    if (itr.ref) 
+    {
+        i_key value = *(itr.ref);
+        _c_MEMB(_erase_at)(self, itr); // Remove the element by value using "_erase_at".
+        return value;
+    }
+    return *(itr.ref); // Return the element that is being popped.
+}
+
+#undef i_type
+#undef i_key
+#endif
diff --git a/pyccel/stdlib/STC_Extensions/Set_extensions.h b/pyccel/stdlib/STC_Extensions/Set_extensions.h
index 91ad65e6..6f5eb20f 100644
--- a/pyccel/stdlib/STC_Extensions/Set_extensions.h
+++ b/pyccel/stdlib/STC_Extensions/Set_extensions.h
@@ -19,4 +19,6 @@ static inline i_key _c_MEMB(_pop)(i_type* self) {
     return *(itr.ref); // Return the element that is being popped.
 }
 
+#undef i_type
+#undef i_key
 #endif


Test patch:
diff --git a/tests/epyccel/test_epyccel_lists.py b/tests/epyccel/test_epyccel_lists.py
index 19d71084..75ff809f 100644
--- a/tests/epyccel/test_epyccel_lists.py
+++ b/tests/epyccel/test_epyccel_lists.py
@@ -29,31 +29,31 @@ def language(request):
 def stc_language(request):
     return request.param
 
-def test_pop_last_element(language) :
+def test_pop_last_element(stc_language) :
     def pop_last_element():
         a = [1,3,45]
         return a.pop()
-    epyc_last_element = epyccel(pop_last_element, language = language)
+    epyc_last_element = epyccel(pop_last_element, language = stc_language)
     pyccel_result = epyc_last_element()
     python_result = pop_last_element()
     assert isinstance(python_result, type(pyccel_result))
     assert python_result == pyccel_result
 
-def test_pop_list_bool(language) :
+def test_pop_list_bool(stc_language) :
     def pop_last_element():
         a = [True, False, True]
         return a.pop()
-    epyc_last_element = epyccel(pop_last_element, language = language)
+    epyc_last_element = epyccel(pop_last_element, language = stc_language)
     pyccel_result = epyc_last_element()
     python_result = pop_last_element()
     assert isinstance(python_result, type(pyccel_result))
     assert python_result == pyccel_result
 
-def test_pop_list_float(language) :
+def test_pop_list_float(stc_language) :
     def pop_last_element():
         a = [1.5 , 3.1, 4.5]
         return a.pop()
-    epyc_last_element = epyccel(pop_last_element, language = language)
+    epyc_last_element = epyccel(pop_last_element, language = stc_language)
     pyccel_result = epyc_last_element()
     python_result = pop_last_element()
     assert isinstance(python_result, type(pyccel_result))
@@ -94,32 +94,32 @@ def test_pop_list_of_ndarrays(language) :
     assert isinstance(python_result, type(pyccel_result))
     assert np.array_equal(python_result, pyccel_result)
 
-def test_pop_specific_index(language) :
+def test_pop_specific_index(stc_language) :
     def pop_specific_index():
         a = [1j,3j,45j]
         return a.pop(1)
-    epyc_specific_index = epyccel(pop_specific_index, language = language)
+    epyc_specific_index = epyccel(pop_specific_index, language = stc_language)
     python_result = pop_specific_index()
     pyccel_result = epyc_specific_index()
     assert isinstance(python_result, type(pyccel_result))
     assert python_result == pyccel_result
 
-def test_pop_negative_index(language) :
+def test_pop_negative_index(stc_language) :
     def pop_negative_index():
         a = [1j,3j,45j]
         return a.pop(-1)
-    epyc_negative_index = epyccel(pop_negative_index, language = language)
+    epyc_negative_index = epyccel(pop_negative_index, language = stc_language)
     python_result = pop_negative_index()
     pyccel_result = epyc_negative_index()
     assert isinstance(python_result, type(pyccel_result))
     assert python_result == pyccel_result
 
-def test_pop_2(language) :
+def test_pop_2(stc_language) :
     def pop_2():
         a = [1.7,2.7,45.0]
         a.pop()
         return a.pop(-1)
-    pop_2_epyc = epyccel(pop_2, language = language)
+    pop_2_epyc = epyccel(pop_2, language = stc_language)
     python_result = pop_2()
     pyccel_result = pop_2_epyc()
     assert isinstance(python_result, type(pyccel_result))
diff --git a/tests/epyccel/test_loops.py b/tests/epyccel/test_loops.py
index a1176010..61a0f431 100644
--- a/tests/epyccel/test_loops.py
+++ b/tests/epyccel/test_loops.py
@@ -80,7 +80,7 @@ def test_double_loop_on_2d_array_F(language):
 @pytest.mark.parametrize( 'language', (
         pytest.param("fortran", marks = pytest.mark.fortran),
         pytest.param("c", marks = [
-            pytest.mark.xfail(reason="C does not support list indexing yet, related issue #1876"),
+            pytest.mark.skip(reason="C does not support list indexing yet, related issue #1948"),
             pytest.mark.c]
         ),
         pytest.param("python", marks = pytest.mark.python)
@@ -101,7 +101,7 @@ def test_product_loop_on_2d_array_C(language):
 @pytest.mark.parametrize( 'language', (
         pytest.param("fortran", marks = pytest.mark.fortran),
         pytest.param("c", marks = [
-            pytest.mark.xfail(reason="C does not support list indexing yet, related issue #1876"),
+            pytest.mark.skip(reason="C does not support list indexing yet, related issue #1948"),
             pytest.mark.c]
         ),
         pytest.param("python", marks = pytest.mark.python)
@@ -122,7 +122,7 @@ def test_product_loop_on_2d_array_F(language):
 @pytest.mark.parametrize( 'language', (
         pytest.param("fortran", marks = pytest.mark.fortran),
         pytest.param("c", marks = [
-            pytest.mark.xfail(reason="C does not support list indexing yet, related issue #1876"),
+            pytest.mark.skip(reason="C does not support list indexing yet, related issue #1948"),
             pytest.mark.c]
         ),
         pytest.param("python", marks = pytest.mark.python)
@@ -180,7 +180,7 @@ def test_enumerate_on_1d_array_with_start(language):
 @pytest.mark.parametrize( 'language', (
         pytest.param("fortran", marks = pytest.mark.fortran),
         pytest.param("c", marks = [
-            pytest.mark.xfail(reason="C does not support list indexing yet, related issue #1876"),
+            pytest.mark.skip(reason="C does not support list indexing yet, related issue #1948"),
             pytest.mark.c]
         ),
         pytest.param("python", marks = pytest.mark.python)