Improve homogeneous containers allocation scheme. (pyccel#2007)

The goal of this PR is to resolve pyccel#1979 and to systematically address
memory allocation for STC containers.
This PR aims to use one of the following to address allocation of STC
containers:
- use STC `init` functions to allocate containers initialized with
literal lists, sets, or dictionaries.
- use `reserve` on STC containers when the capacity is known at compile
time and the container will be populated with append/add methods.
- use `resize` on STC containers when elements of the container will be
accessed through their indices in the code.
Care is taken to drop containers before they are reassigned or to reuse
them whenever possible.

---------

Co-authored-by: Emily Bourne <[email protected]>

Author: jalalium

.github/actions/valgrind_run/action.yml

@@ -9,3 +9,10 @@ runs:
         valgrind --leak-check=full --error-exitcode=1 ./leaks_check
       shell: bash
       working-directory: ./tests/ndarrays
+    
+    - name: Test with valgrind for memory leaks in stc_containers
+      run: |
+        pyccel --language=c --flags="-g -O0" leaks_check.py
+        valgrind --leak-check=full --error-exitcode=1 ./leaks_check
+      shell: bash
+      working-directory: ./tests/stc_containers

CHANGELOG.md

@@ -68,6 +68,7 @@ All notable changes to this project will be documented in this file.
 -   #1913 : Fix function calls to renamed functions.
 -   #1930 : Preserve ordering of import targets.
 -   #1892 : Fix implementation of list function when an iterable is passed as parameter.
+-   #1979 : Fix memory leaks in C due to homogeneous container redefinition.
 -   #1972 : Simplified `printf` statement for Literal String.
 -   #2026 : Fix missing loop in slice assignment.
 

ci_tools/check_pylint_commands.py

@@ -21,7 +21,8 @@
                             re.compile('tests/semantic/scripts/expressions.py'):['unused-variable'],
                             re.compile('tests/semantic/scripts/calls.py'):['unused-variable'],
                             re.compile('tests/pyccel/project_class_imports/.*'):['relative-beyond-top-level'], # ignore Codacy bad pylint call
-                            re.compile('tests/errors/syntax_errors/import_star.py'):['wildcard-import']
+                            re.compile('tests/errors/syntax_errors/import_star.py'):['wildcard-import'],
+                            re.compile('tests/stc_containers/leaks_check.py'):['unused-variable'],
                            }
 
 def run_pylint(file, flag, messages):

pyccel/ast/core.py

@@ -34,6 +34,7 @@
 
 from .variable import DottedName, IndexedElement
 from .variable import Variable, AnnotatedPyccelSymbol
+from .datatypes import HomogeneousSetType, HomogeneousListType, DictType
 
 errors = Errors()
 pyccel_stage = PyccelStage()
@@ -398,16 +399,25 @@ class Allocate(PyccelAstNode):
         In C this provides the size which will be passed to malloc. In Fortran
         this provides the source argument of the allocate function.
 
+    alloc_type : str {'init'|'reserve'|'resize'}, optional
+        Specifies the memory allocation strategy for containers with dynamic memory management.
+        This parameter is relevant for any container type where memory allocation patterns 
+        need to be specified based on usage.
+
+        - 'init' refers to direct allocation with predefined data (e.g., `x = [1, 2, 4]`).
+        - 'reserve' refers to cases where the container will be appended to.
+        - 'resize' referes to cases where the container is populated via indexed elements.
+
     Notes
     -----
     An object of this class is immutable, although it contains a reference to a
     mutable Variable object.
     """
-    __slots__ = ('_variable', '_shape', '_order', '_status', '_like')
+    __slots__ = ('_variable', '_shape', '_order', '_status', '_like', '_alloc_type')
     _attribute_nodes = ('_variable', '_like')
 
     # ...
-    def __init__(self, variable, *, shape, status, like = None):
+    def __init__(self, variable, *, shape, status, like = None, alloc_type = None):
 
         if not isinstance(variable, (Variable, PointerCast)):
             raise TypeError(f"Can only allocate a 'Variable' object, got {type(variable)} instead")
@@ -429,11 +439,15 @@ def __init__(self, variable, *, shape, status, like = None):
         if status not in ('allocated', 'unallocated', 'unknown'):
             raise ValueError(f"Value of 'status' not allowed: '{status}'")
 
+        assert alloc_type is None or isinstance(variable.class_type, (HomogeneousListType, HomogeneousSetType, DictType))
+        assert alloc_type is None or alloc_type in ('init', 'reserve', 'resize')
+
         self._variable = variable
         self._shape    = shape
         self._order    = variable.order
         self._status   = status
         self._like = like
+        self._alloc_type = alloc_type
         super().__init__()
     # ...
 
@@ -485,6 +499,18 @@ def like(self):
         """
         return self._like
 
+    @property
+    def alloc_type(self):
+        """
+        Determines the allocation type for homogeneous containers.
+
+        Returns a string that indicates the allocation type used for memory allocation.
+        The value is either 'init' for containers initialized with predefined data, 
+        'reserve' for containers populated through appending, and 'resize' for containers
+        populated through indexed element assignment.
+        """
+        return self._alloc_type
+
     def __str__(self):
         return f'Allocate({self.variable}, shape={self.shape}, order={self.order}, status={self.status})'
 

pyccel/codegen/printing/ccode.py

@@ -1426,6 +1426,9 @@ def _print_Declare(self, expr):
             assert init == ''
             preface = ''
             init    = ' = {.shape = NULL}'
+        elif isinstance(var.class_type, (HomogeneousListType, HomogeneousSetType, DictType)):
+            preface = ''
+            init = ' = {0}'
         else:
             preface = ''
 
@@ -1688,7 +1691,18 @@ def _print_Allocate(self, expr):
         free_code = ''
         variable = expr.variable
         if isinstance(variable.class_type, (HomogeneousListType, HomogeneousSetType, DictType)):
-            return ''
+            if expr.status in ('allocated', 'unknown'):
+                free_code = f'{self._print(Deallocate(variable))}\n'
+            if expr.shape[0] is None:
+                return free_code
+            size = self._print(expr.shape[0])
+            variable_address = self._print(ObjectAddress(expr.variable))
+            container_type = self.get_c_type(expr.variable.class_type)
+            if expr.alloc_type == 'reserve':
+                return free_code + f'{container_type}_reserve({variable_address}, {size});\n'
+            elif expr.alloc_type == 'resize':
+                return f'{container_type}_resize({variable_address}, {size}, {0});\n'
+            return free_code
         if isinstance(variable.class_type, (NumpyNDArrayType, HomogeneousTupleType)):
             #free the array if its already allocated and checking if its not null if the status is unknown
             if  (expr.status == 'unknown'):

pyccel/parser/semantic.py

@@ -1570,6 +1570,14 @@ def _assign_lhs_variable(self, lhs, d_var, rhs, new_expressions, is_augassign =
                                         shape=a.alloc_shape, status=status))
                             args = new_args
                             new_args = []
+                    elif isinstance(lhs.class_type, (HomogeneousListType, HomogeneousSetType,DictType)):
+                        if isinstance(rhs, (PythonList, PythonDict, PythonSet, FunctionCall)):
+                            alloc_type = 'init'
+                        elif isinstance(rhs, IndexedElement) or rhs.get_attribute_nodes(IndexedElement):
+                            alloc_type = 'resize'
+                        else:
+                            alloc_type = 'reserve'
+                        new_expressions.append(Allocate(lhs, shape=lhs.alloc_shape, status=status, alloc_type=alloc_type))
                     else:
                         new_expressions.append(Allocate(lhs, shape=lhs.alloc_shape, status=status))
                 # ...
@@ -1738,6 +1746,14 @@ def _ensure_inferred_type_matches_existing(self, class_type, d_var, var, is_auga
                                 self.current_ast_node.col_offset))
 
                 else:
+                    alloc_type = None
+                    if isinstance(var.class_type, (HomogeneousListType, HomogeneousSetType,DictType)):
+                        if isinstance(rhs, (PythonList, PythonDict, PythonSet, FunctionCall)):
+                            alloc_type = 'init'
+                        elif isinstance(rhs, IndexedElement) or rhs.get_attribute_nodes(IndexedElement):
+                            alloc_type = 'resize'
+                        else:
+                            alloc_type = 'reserve'
                     if previous_allocations:
                         var.set_changeable_shape()
                         last_allocation = previous_allocations[-1]
@@ -1762,7 +1778,7 @@ def _ensure_inferred_type_matches_existing(self, class_type, d_var, var, is_auga
                     else:
                         status = 'unallocated'
 
-                    new_expressions.append(Allocate(var, shape=d_var['shape'], status=status))
+                    new_expressions.append(Allocate(var, shape=d_var['shape'], status=status, alloc_type=alloc_type))
 
                     if status == 'unallocated':
                         self._allocs[-1].add(var)

tests/stc_containers/leaks_check.py

@@ -0,0 +1,62 @@
+
+# pylint: disable=missing-function-docstring, missing-module-docstring, unused-variable
+def create_list():
+    a = [1,2,3]
+
+def create_set():
+    a = {1,2,3}
+
+def create_dict():
+    a = {1:1,2:2,3:3}
+
+def list_reassign():
+    a = [1,2,3]
+    a = [1,2,3,4]
+    a = [2 * i for i in range(15)]
+    a = [1,2]
+
+def set_reassign():
+    a = {1,2,3}
+    a = {1,2}
+    a = {1,2,3,4}
+    a = {1,2,3,4,5}
+
+def dict_reassign():
+    a = {1:1,2:2,3:3}
+    a = {1:1,2:2,3:3,4:4}
+    a = {1:1,2:2}
+    a = {1:1,2:2,3:3,4:4,5:5}
+
+def conditional_list(b1: bool):
+    if (b1):
+        a = [1,2,3]
+    a = [1,2,3,4]
+
+def conditional_set(b1: bool):
+    if (b1):
+        a = {1,2,3}
+    a = {1,2,3,4}
+
+def conditional_dict(b1: bool):
+    if (b1):
+        a = {1:1,2:2,3:3}
+    a = {1:1,2:2,3:3,4:4,5:5}
+
+def slice_assign():
+    a = [1,2,3,4]
+    b = a[1:-1]
+
+if __name__ == '__main__':
+    create_list()
+    create_set()
+    create_dict()
+    list_reassign()
+    set_reassign()
+    dict_reassign()
+    conditional_list(True)
+    conditional_set(True)
+    conditional_dict(True)
+    conditional_list(False)
+    conditional_set(False)
+    conditional_dict(False)
+    slice_assign()