Deprecate Py_TRASHCAN_SAFE_BEGIN/END

[iritkatriel]

BPO	44874
Nosy	@vstinner, @encukou, @ambv, @jdemeyer, @pablogsal, @iritkatriel
PRs	bpo-44874: deprecate Py_TRASHCAN_SAFE_BEGIN and Py_TRASHCAN_SAFE_END #27693 bpo-35983: improve and test old trashcan macros #12607 bpo-40608: restore protection against double-deallocate issue for subclasses of classes that use trashcan #20104

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-08-27.11:14:54.961>
created_at = <Date 2021-08-09.18:58:05.493>
labels = ['interpreter-core', '3.11']
title = 'Deprecate Py_TRASHCAN_SAFE_BEGIN/END'
updated_at = <Date 2021-08-30.13:39:03.973>
user = 'https://github.com/iritkatriel'

bugs.python.org fields:

activity = <Date 2021-08-30.13:39:03.973>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-08-27.11:14:54.961>
closer = 'lukasz.langa'
components = ['Interpreter Core']
creation = <Date 2021-08-09.18:58:05.493>
creator = 'iritkatriel'
dependencies = []
files = []
hgrepos = []
issue_num = 44874
keywords = ['patch']
message_count = 8.0
messages = ['399285', '399754', '399756', '399866', '400410', '400414', '400415', '400600']
nosy_count = 6.0
nosy_names = ['vstinner', 'petr.viktorin', 'lukasz.langa', 'jdemeyer', 'pablogsal', 'iritkatriel']
pr_nums = ['27693', '12607', '20104']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue44874'
versions = ['Python 3.11']

[iritkatriel]

The old trashcan macros - Py_TRASHCAN_SAFE_BEGIN/END are unsafe (see bpo-40608).

They were removed from the limited C API in 3.9:
https://github.com/python/cpython/blob/main/Doc/whatsnew/3.9.rst#removed-1

They should be removed altogether, in favour of Py_TRASHCAN_BEGIN/END. Since they are not documented, I think this would be done by changing the comment before their definition in Include/cpython/object.h.

[iritkatriel]

Following discussion on python-dev I added a compile-time deprecation warning.

With this, and after reverting PR27683 I get the warning below. The build succeeds and the tests pass.

% make -s -j2
Objects/frameobject.c:622:5: warning: 'UsingDeprecatedTrashcanMacro' is deprecated [-Wdeprecated-declarations]
    Py_TRASHCAN_SAFE_BEGIN(f)
    ^
./Include/cpython/object.h:545:9: note: expanded from macro 'Py_TRASHCAN_SAFE_BEGIN'
        UsingDeprecatedTrashcanMacro cond=1; \
        ^
./Include/cpython/object.h:542:1: note: 'UsingDeprecatedTrashcanMacro' has been explicitly marked deprecated here
Py_DEPRECATED(3.11) typedef int UsingDeprecatedTrashcanMacro;
^
./Include/pyport.h:509:54: note: expanded from macro 'Py_DEPRECATED'
#define Py_DEPRECATED(VERSION_UNUSED) __attribute__((__deprecated__))
                                                     ^

[iritkatriel]

Link to python-dev thread:

https://mail.python.org/archives/list/python-dev@python.org/message/LWP6MOLP5UW2TH3MROZQK4N64SX35N2B/

[ambv]

New changeset 31ee985 by Irit Katriel in branch 'main':
bpo-44874: deprecate Py_TRASHCAN_SAFE_BEGIN and Py_TRASHCAN_SAFE_END (GH-27693)
31ee985

[ambv]

This isn't closed yet because we need a decision whether to issue warnings about Py_TRASHCAN_SAFE_BEGIN/END usage in 3.10 as well. This was suggested by Petr here:
https://mail.python.org/archives/list/python-dev@python.org/message/KWFX6XX3HMZBQ2BYBVL7G74AIOPWO66Y/

I personally don't feel like it's worth going any further than 3.10: Python 3.8 is security fixes only at this point, we haven't received any reports from users about the breakage in 3.9 so far with 3.9.7 around the corner.

So the backport is up to Pablo: should we raise the compiler warning on 3.10 as well?

[pablogsal]

I prefer to not backport this to 3.10 because technically we cannot introduce new deprecations in an rc and I don't want someone compiling with -Werror to crash between rcs.

[ambv]

Thanks for your input, Pablo. In this case this can safely be closed.

Thank you Irit for finding this and providing the PR! ✨ 🍰 ✨

[vstinner]

Thanks Irit for making progress on this annoying old C API!