Right now NodeIterator (node_iterator.py file) and Node (node.py file) contain a reference to the Context, so that it does not get deallocated when we are working with an iterator which traverses the tree, for example.
For the NodeIterator to be able to call iterate method we have to create an iterator in the C++ side, which right now does not do a Py_INCREF of any context, because the method call does not include any context (and in fact, for internal nodes we do not want for it to receive any, since it creates a free context and attaches it to the iterator). We may want to separate both of the methods (create an iterator from the C++ side for internal and external nodes), and add the Py_INCREF for contexts and external references (and the corresponding Py_DECREF in the
PyUastIter_dealloc method).
The same changes (about the Py_INCREF and Py_DECREF for the context) should be introduced in PythonContextExt_filter and PythonContext_filter. Right now we cannot introduce the former ones because the _filter methods also create a PyUastIter object (as PyUastIter_new does). And we have explained that PyUastIter_new is not receiving a context, so we cannot Py_DECREF the context in the deallocation of PyUastIter, and therefore we cannot Py_INCREF the context in _filter methods, if we do not want to leak memory).
It may seem that this is a problem related to design of Node and NodeIterator. If we want to get rid of the references to ctx in those classes, then we should move the iterate functionality entirely to ResultContext, but we have to think this through because right now having an iterate method in Node enhances ease of use, being able to call node.iterate(TreeOrder.PRE_ORDER), for example. This can ease the introduction of the missing Py_INCREFs and Py_DRECREFs but it is not mandatory to do.
Right now
NodeIterator
(node_iterator.py
file) andNode
(node.py
file) contain a reference to theContext
, so that it does not get deallocated when we are working with an iterator which traverses the tree, for example.For the
NodeIterator
to be able to calliterate
method we have to create aniterator
in the C++ side, which right now does not do aPy_INCREF
of any context, because the method call does not include any context (and in fact, for internal nodes we do not want for it to receive any, since it creates a free context and attaches it to the iterator). We may want to separate both of the methods (create an iterator from the C++ side for internal and external nodes), and add thePy_INCREF
for contexts and external references (and the correspondingPy_DECREF
in thePyUastIter_dealloc
method).The same changes (about the
Py_INCREF
andPy_DECREF
for the context) should be introduced inPythonContextExt_filter
andPythonContext_filter
. Right now we cannot introduce the former ones because the_filter
methods also create aPyUastIter
object (asPyUastIter_new
does). And we have explained thatPyUastIter_new
is not receiving a context, so we cannotPy_DECREF
the context in the deallocation ofPyUastIter
, and therefore we cannotPy_INCREF
the context in_filter
methods, if we do not want to leak memory).It may seem that this is a problem related to design of
Node
andNodeIterator
. If we want to get rid of the references toctx
in those classes, then we should move theiterate
functionality entirely toResultContext
, but we have to think this through because right now having aniterate
method inNode
enhances ease of use, being able to callnode.iterate(TreeOrder.PRE_ORDER)
, for example. This can ease the introduction of the missingPy_INCREF
s andPy_DRECREF
s but it is not mandatory to do.