bpo-39947: revert incorrect change to a comment

[iritkatriel]

These functions are called from _PyTrash_begin/_PyTrash_end.

https://bugs.python.org/issue39947

[vstinner]

I'm not sure that this change is correct.

These functions are called from _PyTrash_begin/_PyTrash_end.

Right. But _PyTrash_begin/_PyTrash_end became regular functions, so _PyTrash_thread_deposit_object() and _PyTrash_thread_destroy_chain() don't need to be exported.

They are only exported for the stable ABI.

Maybe the export commands should be moved into object.c? These functions don't need technically to be part of the API.

PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyObject*);
PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);

cc @encukou

[iritkatriel]

I see. I read the comment to mean "this is no longer used", which I found confusing.

While we're here - do we need to keep the bodies of functions that we keep only for the ABI? Or could we replace the bodies of _PyTrash_deposit_object and _PyTrash_destroy_chain (which are really unused) with a comment saying "this is no longer used, left here for the stable ABI..." ?

[encukou]

I'm not that familiar with PyThrash, but the general idea is that macros are compiled into extensions, so extensions will do whatever the macros did in older versions of Python.
If it's now safe to do nothing when those macros call _PyTrash_deposit_object, then the function body can be removed.

[vstinner]

Until Python 3.8, Py_TRASHCAN_BEGIN_CONDITION emitted a call to the _PyTrash_thread_deposit_object() function. So any C extension built with the stable ABI would call _PyTrash_thread_deposit_object() function at the ABI level. If you modify _PyTrash_thread_deposit_object() to make it a no-op function, it will break these C extensions.

But. The Py_TRASHCAN_BEGIN_CONDITION API was not usable with the limited C API (on Python 3.8 and older) since it accessed to PyThreadState members like _tstate->trash_delete_nesting, whereas PyThreadState structure is opaque in the limited C API. So in Python 3.9, I moved the trashcan C API to the limited C API: https://docs.python.org/dev/whatsnew/3.9.html#id3

At the end, providing any backward compatibility with the stable ABI for Python 3.8 and older sounds useless and maybe we can simply remove the following 4 functions:

/* This is the old private API, invoked by the macros before 3.2.4.
   Kept for binary compatibility of extensions using the stable ABI. */
PyAPI_FUNC(void) _PyTrash_deposit_object(PyObject*);
PyAPI_FUNC(void) _PyTrash_destroy_chain(void);

/* This is the old private API, invoked by the macros before 3.9.
   Kept for binary compatibility of extensions using the stable ABI. */
PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyObject*);
PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void);

For the trashcan changes, see:

• https://bugs.python.org/issue38644
• https://bugs.python.org/issue39542

When I did these changes, I was afraid of breaking the C API, so I tried to reduce the risk by keeping _PyTrash_thread_deposit_object() and _PyTrash_thread_destroy_chain() "just in case". I was not brave enough to remove them.

[encukou]

That sounds correct. Let's remove them from 3.11?

[vstinner]

@encukou:

That sounds correct. Let's remove them from 3.11?

Ok, I created PR #26869 to remove the 4 functions.

[vstinner]

Ok, I created PR #26869 to remove the 4 functions.

I merged my PR and so removed these 4 functions. Thanks for the reminder @iritkatriel ;-)