PEP 353: Drop support for PyArg_ParseTuple() "#" formats when PY_SSIZE_T_CLEAN is not defined

[vstinner]

BPO	40943
Nosy	@vstinner, @methane, @serhiy-storchaka, @miss-islington
PRs	bpo-40943: Replace PY_FORMAT_SIZE_T with "z" #20781 bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats #20784 bpo-40943: Fix skipitem() didn't raise SystemError #25937 [3.10] bpo-40943: Fix skipitem() didn't raise SystemError (GH-25937) #25961

^{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-05-07.03:29:07.518>
created_at = <Date 2020-06-10.16:09:12.080>
labels = ['expert-C-API', '3.10']
title = 'PEP 353: Drop support for PyArg_ParseTuple() "#" formats when PY_SSIZE_T_CLEAN is not defined'
updated_at = <Date 2021-05-10.21:29:50.046>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2021-05-10.21:29:50.046>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-07.03:29:07.518>
closer = 'methane'
components = ['C API']
creation = <Date 2020-06-10.16:09:12.080>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 40943
keywords = ['patch']
message_count = 11.0
messages = ['371216', '371218', '371219', '371862', '380656', '393064', '393067', '393068', '393069', '393165', '393434']
nosy_count = 4.0
nosy_names = ['vstinner', 'methane', 'serhiy.storchaka', 'miss-islington']
pr_nums = ['20781', '20784', '25937', '25961']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue40943'
versions = ['Python 3.10']

[vstinner]

Follow-up of bpo-36381: In Python 3.8, PyArg_ParseTuple() and Py_BuildValue() formats using "int" when PY_SSIZE_T_CLEAN is not defined, but Py_ssize_t when PY_SSIZE_T_CLEAN is defined, were deprecated by:

commit d3c72a2
Author: Inada Naoki <songofacandy@gmail.com>
Date: Sat Mar 23 21:04:40 2019 +0900

bpo-36381: warn when no PY_SSIZE_T_CLEAN defined (GH-12473)

We will remove int support from 3.10 or 4.0.

I propose to drop support for these formats in Python 3.10. It is a backward incompatible change on purpose, to ensure that all C extensions are compatible with objects larger than 2 GB, and that all C extensions behave the same.

I'm not sure of the effects of this issue on bpo-27499 "PY_SSIZE_T_CLEAN conflicts with Py_LIMITED_API".

[vstinner]

New changeset d36cf5f by Victor Stinner in branch 'master':
bpo-40943: Replace PY_FORMAT_SIZE_T with "z" (GH-20781)
d36cf5f

[vstinner]

I started to write a long rationale to explain why "#" variants of formats must be deprecated, before I noticed that they are already deprecated! To not lost it, here is my rationale :-)

The C code base of Python switched from int to Py_ssize_t with PEP-353. For not break the backward compatibility, C extension modules have to opt-in for Py_ssize_t by defining PY_SSIZE_T_CLEAN macro in their code base. It affects a few format PyArg_ParseTuple() and Py_BuildValue():
https://docs.python.org/dev/c-api/arg.html

Currently, the documentation says "This behavior will change in a future Python version to only support Py_ssize_t and drop int support. It is best to always define PY_SSIZE_T_CLEAN."

Continue to use int by default prevents to accept content larger than 2 GB and causes issues with limited C API: bpo-27499.

I propose to raise a DeprecationWarning on C extension modules built without the PY_SSIZE_T_CLEAN macro defined.

#warning could be used to emit a warning when building a C extension without PY_SSIZE_T_CLEAN. The problem is that the compiler output is hidden by "pip install", only few developers pay attention to such warnings, and also we should not bother C extensions which don't use PyArg_ParseTuple() and Py_BuildValue().

I prefer to raise a DeprecationWarning exception at runtime since this warning can be made an error when using -Werror: it eases detection of deprecated modules without preventing to build or use a C extension using the deprecated functions.

Two releases after the DeprecationWarning is introduced, we can consider to always raise an exception. Again, I dislike the idea of always require PY_SSIZE_T_CLEAN macro when including "Python.h" header file. I suggest to only raise an exception at runtime when the few affected functions are called.

For example, PyArg_ParseTuple(args, "y*", &buffer) always Py_buffer which uses Py_ssize_t: the behavior does not depend on PY_SSIZE_T_CLEAN.

An alternative is to even deprecate the few "#" formats which are affected by PY_SSIZE_T_CLEAN.

[vstinner]

New changeset 37bb289 by Victor Stinner in branch 'master':
bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats (GH-20784)
37bb289

[vstinner]

libxml2 is hit by this issue:

• https://gitlab.gnome.org/GNOME/libxml2/-/issues/203
• https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/87 (my proposed fix)

[methane]

There is a still warning, not error.

cpython/Python/getargs.c

Lines 2535 to 2542 in 985ac01

	if (flags & FLAG_SIZE_T)
	(void) va_arg(*p_va, Py_ssize_t *);
	else {
	if (PyErr_WarnEx(PyExc_DeprecationWarning,
	"PY_SSIZE_T_CLEAN will be required for '#' formats", 1)) {
	return NULL;
	}
	(void) va_arg(*p_va, int *);

[serhiy-storchaka]

Why PR 20784 has been merged? Was not the plan to emit a deprecation warning first?

[methane]

Why PR 20784 has been merged? Was not the plan to emit a deprecation warning first?

We had been emitted DeprecationWarning since Python 3.8. See #56682.
#64983 changed the DeprecationWarning to SystemError.

[serhiy-storchaka]

Sorry, I was confused by Victor's long msg371219.

[miss-islington]

New changeset 569ca81 by Miss Islington (bot) in branch '3.10':
bpo-40943: Fix skipitem() didn't raise SystemError (GH-25937)
569ca81

[vstinner]

commit 4ebf4a6
Author: Inada Naoki <songofacandy@gmail.com>
Date: Fri May 7 11:56:48 2021 +0900

bpo-40943: Fix skipitem() didn't raise SystemError (GH-25937)

`convertitem()` raises `SystemError` when '#' is used without `PY_SSIZE_T_CLEAN`.
This commit makes `skipitem()` raise it too.