Skip to content

Commit b2b0595

Browse files
frmdstryrfrmdstryr
authored
Update atomlist (#270)
1 parent e78a716 commit b2b0595

2 files changed

Lines changed: 66 additions & 35 deletions

File tree

‎atom/src/atomlist.cpp‎

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,7 @@ ListSubtype_New( PyTypeObject* subtype, Py_ssize_t size )
9797
return PyErr_NoMemory(); // LCOV_EXCL_LINE (memory error)
9898
memset( op->ob_item, 0, nbytes );
9999
}
100-
#if PY_VERSION_HEX >= 0x03090000
101100
Py_SET_SIZE( op, size );
102-
#else
103-
Py_SIZE( op ) = size;
104-
#endif
105101
op->allocated = size;
106102
return ptr.release();
107103
}
@@ -133,12 +129,14 @@ class AtomListHandler
133129
return cppy::incref( Py_None );
134130
}
135131

136-
PyObject* insert( PyObject* args )
132+
PyObject* insert( PyObject*const *args, Py_ssize_t nargsf )
137133
{
138-
Py_ssize_t index;
139-
PyObject* value;
140-
if( !PyArg_ParseTuple( args, "nO:insert", &index, &value ) )
141-
return 0;
134+
if ( PyVectorcall_NARGS(nargsf) != 2 || !PyLong_Check( args[0] ) )
135+
return cppy::type_error("signature is insert(index: int, value: Any)");
136+
Py_ssize_t index = PyLong_AsSsize_t( args[0] );
137+
if ( PyErr_Occurred() )
138+
return 0; // out of range
139+
PyObject* value = args[1];
142140
cppy::ptr valptr( validate_single( value ) );
143141
if( !valptr )
144142
return 0;
@@ -248,7 +246,10 @@ class AtomListHandler
248246
PyObject* val = vd->full_validate( atm, Py_None, b );
249247
if( !val )
250248
return 0;
251-
PyList_SET_ITEM( templist.get(), i, cppy::incref( val ) );
249+
PyList_SET_ITEM( templist.get(), i, val );
250+
// b is a borrowed reference but PyList_SET_ITEM does not decrement
251+
// the refcount of the replaced object so it must be released here
252+
Py_DECREF( b );
252253
}
253254
item = templist;
254255
}
@@ -288,11 +289,7 @@ int AtomList_clear( AtomList* self )
288289
int AtomList_traverse( AtomList* self, visitproc visit, void* arg )
289290
{
290291
Py_VISIT( self->validator );
291-
#if PY_VERSION_HEX >= 0x03090000
292-
// This was not needed before Python 3.9 (Python issue 35810 and 40217)
293292
Py_VISIT(Py_TYPE(self));
294-
#endif
295-
// PyList_type is not heap allocated so it does visit the type
296293
return PyList_Type.tp_traverse( pyobject_cast( self ), visit, arg );
297294
}
298295

@@ -315,9 +312,9 @@ AtomList_append( AtomList* self, PyObject* value )
315312

316313

317314
PyObject*
318-
AtomList_insert( AtomList* self, PyObject* args )
315+
AtomList_insert( AtomList* self, PyObject*const *args, Py_ssize_t nargsf )
319316
{
320-
return AtomListHandler( self ).insert( args );
317+
return AtomListHandler( self ).insert( args, nargsf );
321318
}
322319

323320

@@ -384,7 +381,7 @@ PyDoc_STRVAR( a_extend_doc,
384381

385382
static PyMethodDef AtomList_methods[] = {
386383
{ "append", ( PyCFunction )AtomList_append, METH_O, a_append_doc },
387-
{ "insert", ( PyCFunction )AtomList_insert, METH_VARARGS, a_insert_doc },
384+
{ "insert", ( PyCFunction )AtomList_insert, METH_FASTCALL, a_insert_doc },
388385
{ "extend", ( PyCFunction )AtomList_extend, METH_O, a_extend_doc },
389386
{ "__reduce_ex__", ( PyCFunction )AtomList_reduce_ex, METH_O, "" },
390387
{ 0 } /* sentinel */
@@ -550,7 +547,7 @@ init_containerlistchange()
550547
return false; // LCOV_EXCL_LINE (failed interned string creation)
551548
}
552549
PySStr::__delitem__str = PyUnicode_InternFromString( "__delitem__" );
553-
if( !PySStr::typestr )
550+
if( !PySStr::__delitem__str )
554551
{
555552
return false; // LCOV_EXCL_LINE (failed interned string creation)
556553
}
@@ -664,10 +661,10 @@ class AtomCListHandler : public AtomListHandler
664661
return res.release();
665662
}
666663

667-
PyObject* insert( PyObject* args )
664+
PyObject* insert( PyObject*const *args, Py_ssize_t nargsf )
668665
{
669666
Py_ssize_t size = PyList_GET_SIZE( m_list.get() );
670-
cppy::ptr res( AtomListHandler::insert( args ) );
667+
cppy::ptr res( AtomListHandler::insert( args, nargsf ) );
671668
if( !res )
672669
return 0;
673670
if( observer_check() )
@@ -678,10 +675,10 @@ class AtomCListHandler : public AtomListHandler
678675
if( PyDict_SetItem( c.get(), PySStr::operationstr, PySStr::insertstr ) != 0 )
679676
return 0;
680677
// if the superclass call succeeds, then this is safe.
681-
Py_ssize_t where = PyLong_AsSsize_t( PyTuple_GET_ITEM( args, 0 ) );
678+
PyObject* index = args[0];
679+
Py_ssize_t where = PyLong_AsSsize_t( index );
682680
clip_index( where, size );
683-
cppy::ptr index( PyLong_FromSsize_t( where ) );
684-
if( PyDict_SetItem( c.get(), PySStr::indexstr, index.get() ) != 0 )
681+
if( PyDict_SetItem( c.get(), PySStr::indexstr, index ) != 0 )
685682
return 0;
686683
if( PyDict_SetItem( c.get(), PySStr::itemstr, m_validated.get() ) != 0)
687684
return 0;
@@ -711,12 +708,11 @@ class AtomCListHandler : public AtomListHandler
711708
return res.release();
712709
}
713710

714-
PyObject* pop( PyObject* args )
711+
PyObject* pop( PyObject*const *args, Py_ssize_t nargsf )
715712
{
716713
Py_ssize_t size = PyList_GET_SIZE( m_list.get() );
717-
int nargs = (int)PyTuple_GET_SIZE( args);
718-
PyObject **stack = &PyTuple_GET_ITEM(args, 0);
719-
cppy::ptr res( ListMethods::pop( m_list.get(), stack, nargs ) );
714+
const int nargs = PyVectorcall_NARGS(nargsf);
715+
cppy::ptr res( ListMethods::pop( m_list.get(), args, nargs ) );
720716
if( !res )
721717
return 0;
722718
if( observer_check() )
@@ -728,11 +724,13 @@ class AtomCListHandler : public AtomListHandler
728724
return 0;
729725
// if the superclass call succeeds, then this is safe.
730726
Py_ssize_t i = -1;
731-
if( PyTuple_GET_SIZE( args ) == 1 )
732-
i = PyLong_AsSsize_t( PyTuple_GET_ITEM( args, 0 ) );
727+
if( nargs == 1 )
728+
i = PyLong_AsSsize_t( args[0] );
733729
if( i < 0 )
734730
i += size;
735731
cppy::ptr index( PyLong_FromSsize_t( i ) );
732+
if ( !index )
733+
return 0; // LCOV_EXCL_LINE
736734
if( PyDict_SetItem( c.get(), PySStr::indexstr, index.get() ) != 0 )
737735
return 0;
738736
if( PyDict_SetItem( c.get(), PySStr::itemstr, res.get() ) != 0 )
@@ -1053,9 +1051,9 @@ AtomCList_append( AtomCList* self, PyObject* value )
10531051

10541052

10551053
PyObject*
1056-
AtomCList_insert( AtomCList* self, PyObject* args )
1054+
AtomCList_insert( AtomCList* self, PyObject*const *args, Py_ssize_t nargsf )
10571055
{
1058-
return AtomCListHandler( self ).insert( args );
1056+
return AtomCListHandler( self ).insert( args, nargsf );
10591057
}
10601058

10611059

@@ -1067,9 +1065,9 @@ AtomCList_extend( AtomCList* self, PyObject* value )
10671065

10681066

10691067
PyObject*
1070-
AtomCList_pop( AtomCList* self, PyObject* args )
1068+
AtomCList_pop( AtomCList* self, PyObject*const *args, Py_ssize_t nargsf )
10711069
{
1072-
return AtomCListHandler( self ).pop( args );
1070+
return AtomCListHandler( self ).pop( args, nargsf );
10731071
}
10741072

10751073

@@ -1144,9 +1142,9 @@ cmp(x, y) -> -1, 0, 1");
11441142
static PyMethodDef
11451143
AtomCList_methods[] = {
11461144
{ "append", ( PyCFunction )AtomCList_append, METH_O, c_append_doc },
1147-
{ "insert", ( PyCFunction )AtomCList_insert, METH_VARARGS, c_insert_doc },
1145+
{ "insert", ( PyCFunction )AtomCList_insert, METH_FASTCALL, c_insert_doc },
11481146
{ "extend", ( PyCFunction )AtomCList_extend, METH_O, c_extend_doc },
1149-
{ "pop", ( PyCFunction )AtomCList_pop, METH_VARARGS, c_pop_doc },
1147+
{ "pop", ( PyCFunction )AtomCList_pop, METH_FASTCALL, c_pop_doc },
11501148
{ "remove", ( PyCFunction )AtomCList_remove, METH_O, c_remove_doc },
11511149
{ "reverse", ( PyCFunction )AtomCList_reverse, METH_NOARGS, c_reverse_doc },
11521150
{ "sort", ( PyCFunction )AtomCList_sort, METH_VARARGS | METH_KEYWORDS, c_sort_doc },

‎tests/test_atomlist.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# The full license is in the file LICENSE, distributed with this software.
77
# --------------------------------------------------------------------------------------
88
import gc
9+
import sys
910
from collections import Counter
1011
from pickle import dumps, loads
1112

@@ -649,3 +650,35 @@ def test_container_repeat(container_model, kind):
649650
for change in (container_model.change, container_model.get_static_change(kind)):
650651
assert change["operation"] == "__imul__"
651652
assert change["count"] == 2
653+
654+
655+
def test_insert_args():
656+
class Obj(Atom):
657+
items = List()
658+
659+
obj = Obj()
660+
with pytest.raises(TypeError):
661+
obj.items.insert(None, None)
662+
with pytest.raises(TypeError):
663+
obj.items.insert(0, None, None)
664+
with pytest.raises(OverflowError):
665+
obj.items.insert(2**64 + 1, 0)
666+
667+
668+
def test_validate_seq_refcnt():
669+
class Item(Atom):
670+
name = Value()
671+
672+
class Obj(Atom):
673+
items = List(Item)
674+
675+
a = Item(name="a")
676+
b = Item(name="b")
677+
rca = sys.getrefcount(a)
678+
rcb = sys.getrefcount(b)
679+
obj = Obj()
680+
obj.items.extend([a, b])
681+
del obj
682+
gc.collect()
683+
assert sys.getrefcount(a) == rca
684+
assert sys.getrefcount(b) == rcb

0 commit comments

Comments
 (0)