C files generated by Cython set tp_print to 0: PyTypeObject.tp_print removed

@ncoghlan

BPO	37250
Nosy	@ncoghlan, @scoder, @encukou, @ambv, @jdemeyer, @zooba, @willingc
PRs	bpo-37250: define tp_print for backwards compatibility #14009 [3.8] bpo-37250: put back tp_print for backwards compatibility #14193

^{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 2019-07-01.13:40:34.626>
created_at = <Date 2019-06-12.13:08:30.505>
labels = ['interpreter-core', 'build', '3.8']
title = 'C files generated by Cython set tp_print to 0: PyTypeObject.tp_print removed'
updated_at = <Date 2019-07-01.13:40:34.626>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-07-01.13:40:34.626>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2019-07-01.13:40:34.626>
closer = 'ncoghlan'
components = ['Interpreter Core']
creation = <Date 2019-06-12.13:08:30.505>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 37250
keywords = ['patch']
message_count = 69.0
messages = ['345337', '345340', '345341', '345342', '345343', '345344', '345345', '345346', '345347', '345348', '345349', '345350', '345351', '345352', '345353', '345355', '345356', '345357', '345359', '345382', '345383', '345402', '345405', '345409', '345459', '345591', '345965', '345966', '345969', '345988', '345989', '345990', '345992', '345994', '345996', '345998', '346013', '346064', '346067', '346069', '346075', '346083', '346084', '346085', '346087', '346099', '346100', '346105', '346123', '346125', '346128', '346153', '346161', '346163', '346177', '346185', '346190', '346194', '346213', '346218', '346220', '346222', '346223', '346238', '346373', '346388', '346424', '346438', '346507']
nosy_count = 7.0
nosy_names = ['ncoghlan', 'scoder', 'petr.viktorin', 'lukasz.langa', 'jdemeyer', 'steve.dower', 'willingc']
pr_nums = ['14009', '14193']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue37250'
versions = ['Python 3.8']

Copy of Stefan Behnel's msg345305 in bpo-37221:
"""
Note that PyCode_New() is not the only change in 3.8 beta1 that breaks Cython generated code. The renaming of "tp_print" to "tp_vectorcall" is equally disruptive, because Cython has (or had) a work-around for CPython (mis-)behaviour that reset the field explicitly to NULL after calling PyType_Ready(), which could set it arbitrarily without good reason.

So, either revert that field renaming, too, or ignore Cython generated modules for the reasoning about the change in this ticket.

I'm fine with keeping things as they are now in beta-1, but we could obviously adapt to whatever beta-2 wants to change again.
"""

There are 2 problems:

source compatibility
ABI compatibility

The following change removed PyTypeObject.tp_print and replaced it with PyTypeObject.tp_vectorcall_offset:

commit aacc77f
Author: Jeroen Demeyer <J.Demeyer@UGent.be>
Date: Wed May 29 20:31:52 2019 +0200

bpo-36974: implement PEP-590 (GH-13185)


Co-authored-by: Jeroen Demeyer <J.Demeyer@UGent.be>
Co-authored-by: Mark Shannon <mark@hotpy.org>

== ABI compatibility ==

In term of ABI, it means that C extensions using static type ("static PyTypeObject mytype = { .... };") is broken by this change.

Honestly, I'm not sure if we ever provided any forward compatibility for static types. bpo-32388 removed "cross-version binary compatibility" on purpose in Python 3.8. It's an on-going topic, see also my notes about ABI and PyTypeObject:
https://pythoncapi.readthedocs.io/type_object.html

Maybe we can simply ignore this part of the problem.

== Source compatibility ==

Cython generates C code setting tp_print explicitly to NULL.

To avoid depending on Cython at installation, most (if not all) projects using Cython include C files generated by Cython in files they distribute (like tarballs). Because of that, the commit aacc77f requires all these projects to regenerate their C files using Cython.

In Fedora, we fixed many Python packages to always re-run Cython to regenerate all C files. But Fedora is just one way to distribute packages, it doesn't solve the problem of files distributed on PyPI, nor other Linux distribution (for example).

Jeroen Demeyer proposed PR 14009 to fix the source compatibility:

   #define tp_print tp_vectorcall

"printfunc tp_print;" has been replaced with "Py_ssize_t tp_vectorcall_offset;": the type was basically a pointer and has been replaced with an integer.

With "#define tp_print tp_vectorcall", "type->tp_print = NULL;" becomes "type->tp_vectorcall = NULL;".

If -Werror is used, "type->tp_vectorcall = NULL;" would fail with a compilation error, since NULL is not exactly an integer, no? I'm not sure that it's an issue, I'm just thinking aloud.

Actually, Cython generates code setting tp_print to 0 (not NULL).

inherit_slots() uses:

        /* Inherit tp_vectorcall_offset only if tp_call is not overridden */
        if (!type->tp_call) {
            COPYSLOT(tp_vectorcall_offset);
        }

PyType_Ready() contains the following assertion:

    /* Consistency checks for PEP 590:
     * - Py_TPFLAGS_METHOD_DESCRIPTOR requires tp_descr_get
     * - _Py_TPFLAGS_HAVE_VECTORCALL requires tp_call and
     *   tp_vectorcall_offset > 0
     * To avoid mistakes, we require this before inheriting.
     */
    if (type->tp_flags & Py_TPFLAGS_METHOD_DESCRIPTOR) {
        _PyObject_ASSERT((PyObject *)type, type->tp_descr_get != NULL);
    }
    if (type->tp_flags & _Py_TPFLAGS_HAVE_VECTORCALL) {
        _PyObject_ASSERT((PyObject *)type, type->tp_vectorcall_offset > 0);
        _PyObject_ASSERT((PyObject *)type, type->tp_call != NULL);
    }

I understand that tp_vectorcall_offset=0 is fine and the expected value for a type which doesn't have the flag _Py_TPFLAGS_HAVE_VECTORCALL.

IMHO the simplest and safest option for this issue is to revert tp_print removal and move tp_vectorcall_offset at the end of PyTypeObject.

Is PEP-590 fully public in Python 3.8? It seems like _Py_TPFLAGS_HAVE_VECTORCALL at least is private.

Maybe we can attempt again to remove tp_print from Python 3.9? Once Cython will handle tp_print removal? Or even keep it in Python 3.9?

Python 3 had unused tp_print for 10 years and nobody complained. There are a few PyTypeObject instances in a Python process. A single pointer isn't a giant waste of memory.

If -Werror is used, "type->tp_vectorcall = NULL;" would fail with a compilation error, since NULL is not exactly an integer, no?

No because tp_vectorcall is a function pointer. You're confusing with tp_vectorcall_offset which is an integer.

No because tp_vectorcall is a function pointer. You're confusing with tp_vectorcall_offset which is an integer.

Oh. I didn't notice that your PR makes tp_print an alias to tp_vectorcall rather than tp_vectorcall_offset. Ok.

If we decide to not reintroduce tp_print, tp_print removal must be documented at:
https://docs.python.org/dev/whatsnew/3.8.html#changes-in-the-c-api

I understand that tp_vectorcall_offset=0 is fine and the expected value for a type which doesn't have the flag _Py_TPFLAGS_HAVE_VECTORCALL.

Not necessarily. There are subtleties involved when subclassing: there are cases where tp_vectorcall_offset needs to be non-zero despite _Py_TPFLAGS_HAVE_VECTORCALL not being set. See also PR 13858.

Cython issue: cython/cython#2976

IMHO the simplest and safest option for this issue is to revert tp_print removal and move tp_vectorcall_offset at the end of PyTypeObject.

That's a drastic solution to a rather simple problem. PR 14009 would fix Cython just fine.

me:

I understand that tp_vectorcall_offset=0 is fine and the expected value for a type which doesn't have the flag _Py_TPFLAGS_HAVE_VECTORCALL.

Jeroen Demeyer:

Not necessarily. There are subtleties involved when subclassing: there are cases where tp_vectorcall_offset needs to be non-zero despite _Py_TPFLAGS_HAVE_VECTORCALL not being set. See also PR 13858.

Cython generates C code which looks like:

if (PyType_Ready(type) < 0) { ... handle error ... }
type->tp_print = 0;

This code can cause subtle and annoying issue if PR 13858 is merged. So that's another argument in favor of reintroducing tp_print in Python 3.8.

--

Cython has already been fixed to no longer set tp_print to 0 on Python 3.8:

cython/cython@f10a0a3

But again, this problem is not about correctness, but more about practical backward compatibility issues (see my first message).

scientific projects that depend on it may lag longer as rebuilds and releases will need to happen on PyPI, conda, and conda-forge.

Keep in mind that binary artifacts (wheels, Conda/distro packages) should not be affected. They need to be re-built for Python 3.8 *either way*, because the ABI changed.

The only artifacts that putting tp_print back in helps are sdists that include pre-generated Cython output, plus possibly projects that pin Cython to an older version.

Specifically, for any platform without a C compiler installed, users aren't helped by putting tp_print back in.

So:

(1) Status-quo (keep everything as is in 3.8b1):

binary/compiled distributions need to be re-built (*1)
sdists need to be re-generated with an up-to-date Cython (*2)
projects that pin Cython need to update it.
+ pure source distributions (where compiling involves running Cython) works as before (*3)
+ type objects do not waste memory
+ Cython can implement vectorcall optimizations for past Python versions (before 3.8)

(2) Apply PR 14193:

binary/compiled distributions still need to be re-built (*1)
+ sdists DO NOT need to be re-generated (*2)
+ projects that pin Cython DO NOT need to update it.
+ pure source distributions (where compiling involves running Cython) works as before (*3)
type objects get 8 bytes larger
+ Cython can implement vectorcall optimizations for past Python versions (before 3.8)

(3) Put back tp_print the way it was in 3.7:

binary/compiled distributions still need to be re-built (*1)
+ sdists DO NOT need to be re-generated (*2)
+ projects that pin Cython DO NOT need to update it.
+ pure source distributions (where compiling involves running Cython) works as before (*3)
type objects get 8 bytes larger
Cython can NOT implement vectorcall optimizations for past (pre-3.8) Python versions

*1: applicable to most wheel consumers, conda users, etc.
*2: applicable to sdist consumers with a compiler
*3: should be applicable to Conda/Fedora/Debian/... packagers (not so much end users)

Is four months really an unreasonable timeline? I thought most of these packages released more frequently than CPython.

Note that maintainers of these packages need to take an action (build wheels) anyway to support Python 3.8 (on compiler-less platforms). They need to test, and ideally even read "Porting to Python 3.8" and adjust the code.
Is re-generating Cython output, and bumping the version, too much of an *added* burden here?

If a project doesn't release 3.8 wheels by Python 3.8.0rc1, we don't get feedback on it from PyPI users (but Conda & Linux distros should be unaffected, if they run Cython on build).

FWIW, I agree totally with Petr's analysis.

Jeroen's analysis ignores some points, has technical inaccuracies (nobody can use a static type compiled for 3.7 with 3.8), and is based on precedents that do not apply to this situation. (All of which I've mentioned above, but I assume nobody is reading the whole repetitive thread anymore.)

based on precedents that do not apply to this situation

I'm curious what you mean here...

It isn't the actively maintained projects that publish wheel files that I'm worried about fixing - it's the sdist only projects that would otherwise only need recompilation to work on the new Python version.

With this fixed, and the PyCode_New -> PyCode_New + PyCode_WithPosArgs fix from bpo-37221 merged, then that long tail will get another few years of life, and will start emitting deprecation warnings for the part that's actually going to go away in Python 3.9 (i.e. the tp_print slot).

My opinion would be different if tp_print had actually been emitting compile time warnings for the past 10 years, but it hasn't been - we just haven't been using it for anything.

I'm curious what you mean here...

msg345409

With this fixed, and the PyCode_New -> PyCode_New + PyCode_WithPosArgs fix from bpo-37221 merged, then that long tail will get another few years of life, and will start emitting deprecation warnings for the part that's actually going to go away in Python 3.9 (i.e. the tp_print slot).

By definition (as unmaintained packages), that's a few more years until death. Nobody is looking at their compile time warnings (that would require maintenance ;) ), so nothing will happen until it actually breaks. Keeping the field for one more version makes less sense than keeping it forever.

Thanks Petr for the wonderful summary.

Of the pros/cons that you mention (if I am understanding correctly), I think that the issue comes down to sdist consumers and regeneration.

If sdist consumers will need to regenerate an sdist?
How many sdist consumers will likely be impacted?
If impacted, how difficult would the regeneration be for a scientist who is moderately comfortable with compilation?
Is there any guidance or information that can be given to sdist consumers?

Do we have any good metrics/insights on Question 2 and 3?

---

While TypeObject size, updating a pinned version, and vectorcall optimization for prior versions are considerations, I believe that they are secondary:

Typeobject size: Nice to have a smaller size but probably not a concern of the sdist consumer or general user.
Update a pinned Cython version: General expectation is it should work with the environment which it was pinned within. There should not be an expectation that the pinned version will work if other parts of the environment change.
vectorcall optimization extends to prior versions: While perhaps nice to have, it is likely more an added benefit instead of an implicit contract or expectation.

If sdist consumers will need to regenerate an sdist?

They would need to compile from the source, e.g. clone from Git and make an sdist (or install) from that.

How many sdist consumers will likely be impacted?

I don't know. Depends on how fast people migrate to Python 3.8.

If impacted, how difficult would the regeneration be for a scientist who is moderately comfortable with compilation?

See (1.)

Is there any guidance or information that can be given to sdist consumers?

"Raise the issue with the sdist maintainers" is probably their best bet.

---

Realistically, I expect those people to stay on Python 3.7- until their dependencies are updated to (and also tested with!) 3.8.

I've seen anecdotal evidence of this hitting people who want to use real-world projects to benchmark Python 3.8+: https://mail.python.org/archives/list/python-dev@python.org/message/TF2JI7G5ZMCGUMM3AWNSCQDYVFNRPMQ4/

If we "stop discussing and just [Apply PR 14193]", people wen't be blocked from trying out 3.8, even if sdist maintainers don't care about 3.8 yet.

There's also the question of complying with our own deprecation policy, as the approval to ditch tp_print API compatibility in PEP-590 relied on the fact that it has been deprecated since Python 3.0.

However, it turns out the associated programmatic deprecation had never actually been implemented, which means it is now appropriate to provide that deprecation warning belatedly in Python 3.8, and then upgrade to full removal in Python 3.9.

That way the folks building affected sdists from source will get the additional warnings to tell them "Hey, you have a looming maintainability problem to solve (and a year or two to solve it)", rather than the current situation, which is "You have a dependency management crisis *now*, and until you solve it, you can't even start testing on Python 3.8, since you won't even be able to build your dependencies, let alone install them".

Thank you, Petr, for bringing this discussion back on a solid basis, actually twice already.

And sorry for causing this problem in the first place. The "tp_print = 0" was necessary to fix misbehaviour in Py2, and we should have restricted it to Py2 back then (that line has been there since late 2013).

I think keeping some kind of a "tp_print" named field in PyTypeObject for now is the best we can do. I like Jeroen's PR (thanks for taking care of so many things at a time, Jeroen!), and I think we should just go with it.

I'm happy to release a new point release for Cython, as soon as it's clear what CPython will do, and how Cython can detect the change to adapt to it.

Thank you Petr. Looking at your response, the full discussion here, and the fact that we don't have an easy way to put a number on who would be impacted, I agree that merging the PR seems the best course of action.

Thanks Stefan for additional follow up with Cython too.

New changeset d917cfe by Petr Viktorin (Jeroen Demeyer) in branch '3.8':
[3.8] bpo-37250: put back tp_print for backwards compatibility (GH-14193)
d917cfe

Navigation Menu

Repository navigation

C files generated by Cython set tp_print to 0: PyTypeObject.tp_print removed #81431

46 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

C files generated by Cython set tp_print to 0: PyTypeObject.tp_print removed #81431

Description

Activity

vstinner commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

jdemeyer commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

jdemeyer commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

jdemeyer commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

jdemeyer commented on Jun 12, 2019

vstinner commented on Jun 12, 2019

46 remaining items

encukou commented on Jun 21, 2019

zooba commented on Jun 21, 2019

jdemeyer commented on Jun 21, 2019

ncoghlan commented on Jun 21, 2019

zooba commented on Jun 21, 2019

zooba commented on Jun 21, 2019

willingc commented on Jun 21, 2019

encukou commented on Jun 24, 2019

ncoghlan commented on Jun 24, 2019

scoder commented on Jun 24, 2019

willingc commented on Jun 24, 2019

encukou commented on Jun 25, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions