Should `fami_dealloc()` use `PyObject_Del()`?

[flexatone]

Presently, iter() calls the following to create a new FAMIObject:

`FAMIObject *self = PyObject_New(FAMIObject, &FAMIType);`

However, fami_dealloc() calls the following;

Py_TYPE(self)->tp_free((PyObject *)self);

Is it preferable for fami_dealloc() to use PyObject_Del() here instead? The tp_dealloc docs seem to suggest that if PyObject_New() is used for allocation, PyObject_Del() should be used in deallocation. I expect it is equivalent but I wonder if there are other reasons to favor PyObject_Del().

https://docs.python.org/3/c-api/typeobj.html?highlight=py_tpflags_sequence#c.PyTypeObject.tp_dealloc

[brandtbucher]

It's been a while since I've touched this code, but I think that this is okay, since tp_free defaults to PyObject_Del.

I probably copied this from the CPython codebase (this is exactly what dict_dealloc does). I believe the motivation behind this pattern is so static subclasses could define different tp_alloc or tp_free functions without needing to also redefine tp_dealloc as well.

So it probably matters much less if you're not planning on having subclasses that use custom allocators or something.

[flexatone]

Many thanks for your comments.

This caught my attention because (a) FAMIType does not define .tp_new or use .tp_alloc (explicitly), but instead just uses PyObject_New in a dedicated constructor function and (b) I had a strange case over in ArrayKit where I had an instance created with PyObject_New composed within another instance created with PyObject_New; the reference counting all seemed correct but I was seg-faulting on the .tp_dealloc call; as soon as I used PyObject_Del() instead of tp_free the seg fault went away.

If you think it is better, I can submit a PR making that change here.