Skip to content

Remove Py_TPFLAGS_HAVE_VERSION_TAG flag? #86913

Closed
Closed
Remove Py_TPFLAGS_HAVE_VERSION_TAG flag?#86913
Labels
3.10 (EOL)end of lifetopic-C-API

Description

@vstinner
vstinner
opened on Dec 25, 2020
BPO 42747
Nosy @pitrou, @vstinner, @encukou, @serhiy-storchaka, @corona10, @pablogsal, @miss-islington, @shihai1991
PRs
  • bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VERSION_TAG no-op #27260
  • [3.10] bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VERSION_TAG no-op (GH-27260) #27306
    		•

    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-07-23.16:09:37.917>
created_at = <Date 2020-12-25.23:48:07.870>
labels = ['expert-C-API', '3.10']
title = 'Remove Py_TPFLAGS_HAVE_VERSION_TAG flag?'
updated_at = <Date 2021-07-23.16:09:37.916>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-07-23.16:09:37.916>
actor = 'petr.viktorin'
assignee = 'none'
closed = True
closed_date = <Date 2021-07-23.16:09:37.917>
closer = 'petr.viktorin'
components = ['C API']
creation = <Date 2020-12-25.23:48:07.870>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 42747
keywords = ['patch']
message_count = 14.0
messages = ['383782', '383809', '383821', '383855', '383971', '383974', '384040', '384041', '384051', '397873', '398050', '398061', '398069', '398077']
nosy_count = 8.0
nosy_names = ['pitrou', 'vstinner', 'petr.viktorin', 'serhiy.storchaka', 'corona10', 'pablogsal', 'miss-islington', 'shihai1991']
pr_nums = ['27260', '27306']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue42747'
versions = ['Python 3.10']

    Activity

    1. vstinner commented on Dec 25, 2020

      @vstinner
      vstinner
      on Dec 25, 2020
      MemberAuthor

      Since the PyTypeObject structure is excluded from the limited C API and the stable ABI on purpose (PEP-384), I don't see the purpose of the Py_TPFLAGS_HAVE_VERSION_TAG flag.

      Moreover, a new flag was added recently:

      #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000
/* Type has am_send entry in tp_as_async slot */
#define Py_TPFLAGS_HAVE_AM_SEND (1UL << 21)
#endif

      Should it be also removed?

      For example, Py_TPFLAGS_HAVE_FINALIZE was deprecated in bpo-32388 by:

      commit ada319b
      Author: Antoine Pitrou <antoine@python.org>
      Date: Wed May 29 22:12:38 2019 +0200

      bpo-32388: Remove cross-version binary compatibility requirement in tp_flags (GH-4944)

It is now allowed to add new fields at the end of the PyTypeObject struct without having to allocate a dedicated compatibility flag i

      n tp_flags.

      This will reduce the risk of running out of bits in the 32-bit tp_flags value.

      By the way, is it worth it to remove Py_TPFLAGS_HAVE_FINALIZE? Or is it going to break too many extension modules?

    2. vstinner
      added
      3.10 (EOL)end of life
      topic-C-API
      on Dec 25, 2020

    3. corona10 commented on Dec 26, 2020

      @corona10
      corona10
      on Dec 26, 2020
      Member

      Let's emit the deprecated message if Py_TPFLAGS_HAVE_VERSION_TAG is set ;)
      For example, PyType_Ready looks like a good place

    4. vstinner commented on Dec 26, 2020

      @vstinner
      vstinner
      on Dec 26, 2020
      MemberAuthor

      Maybe we can keep the constants but define them as 0 and deprecate them.

      The question is more if these constants solve a real ABI issue or not.

    5. shihai1991 commented on Dec 27, 2020

      @shihai1991
      shihai1991
      on Dec 27, 2020
      Member

      I don't see the purpose of the Py_TPFLAGS_HAVE_VERSION_TAG flag.

      IMO, There have no more exact description of Py_TPFLAGS_HAVE_VERSION_TAGin docs, so I perfer to remove it.

      By the way, is it worth it to remove Py_TPFLAGS_HAVE_FINALIZE? Or is it going to break too many extension modules?

      I am not found who use Py_TPFLAGS_HAVE_AM_SEND or Py_TPFLAGS_HAVE_FINALIZE in github.I perfer to keep it as soon as posssible if there have users using them.
      MAYBE we should do some surveys firstly?

    6. encukou commented on Dec 29, 2020

      @encukou
      encukou
      on Dec 29, 2020
      Member

      IMO, these flags are useless. Both the stable ABI and the version-specific builds of extension modules use the memory layout of the current interpreter and fill unset slots with NULL.

      Py_TPFLAGS_HAVE_AM_SEND is new in 3.10, so it can be removed (or replaced by a check for Py_TYPE(iter)->tp_as_async != NULL && Py_TYPE(iter)->tp_as_async->am_send != NULL).

      Py_TPFLAGS_HAVE_VERSION_TAG and Py_TPFLAGS_HAVE_FINALIZE are there since 2.6 and 3.8 respectively, and mentioned in documentation. Extensions that use the stable ABI can *check for them*. We cannot do a survey of all existing extension modules. The flags can't be removed without breaking stable ABI.

    7. serhiy-storchaka commented on Dec 29, 2020

      @serhiy-storchaka
      serhiy-storchaka
      on Dec 29, 2020
      Member

      I don't think that it was right thing to break binary compatibility. It virtually buried the stable ABI.

    8. vstinner commented on Dec 30, 2020

      @vstinner
      vstinner
      on Dec 30, 2020
      MemberAuthor

      Serhiy Storchaka:

      I don't think that it was right thing to break binary compatibility. It virtually buried the stable ABI.

      IMO there is a misunderstanding about the stable ABI. PyType_FromSpec() doesn't need Py_TPFLAGS_HAVE_xxx flags: this function allocates a heap type which *has* all these members (set to 0/NULL by default).

      And it's not possible to define a static type using the limited C API, since the PyTypeObject structure is excluded from it on purpose.

      See bpo-32388 for a similar discussion.

    9. vstinner commented on Dec 30, 2020

      @vstinner
      vstinner
      on Dec 30, 2020
      MemberAuthor

      Ah, I see that there is a misunderstanding. The flag is used for two things:

      (*) Create/Declare a type

      IMO Py_TPFLAGS_HAVE_VERSION_TAG and Py_TPFLAGS_HAVE_FINALIZE are useless for that.

      (*) Check if a type has the flag

      An extension built with the stable ABI *can* use PyType_HasFeature(type, Py_TPFLAGS_HAVE_VERSION_TAG).

      I'm only aware of PyQt which uses the stable ABI. Is there other projects using it? Do these projects check for Py_TPFLAGS_HAVE_VERSION_TAG or Py_TPFLAGS_HAVE_FINALIZE flag?

      If we cannot be used that no extension module built with the stable ABI for these flags, the safe option is to keep them, deprecate them, and always define them. For example, add the flags to Py_TPFLAGS_DEFAULT and/or add them in type_new() or PyType_Ready().

      --

      Py_TPFLAGS_HAVE_VERSION_TAG is really an internal tag. It should only be used by _PyType_Lookup().

      I'm not sure why someone would like to check explicitly for the Py_TPFLAGS_HAVE_FINALIZE flag. I would expect that only Python internals call the tp_finalize slot.

    10. encukou commented on Dec 30, 2020

      @encukou
      encukou
      on Dec 30, 2020
      Member

      I'm only aware of PyQt which uses the stable ABI. Is there other projects using it? Do these projects check for Py_TPFLAGS_HAVE_VERSION_TAG or Py_TPFLAGS_HAVE_FINALIZE flag?

      I think there are. Who knows how many; they're not required to register anywhere.

      If the flags do end up being removed, it would be nice to reserve the bits (in a comment), so they aren't reused too soon.

      Also, ISTM that PEP-387 applies, and requires warning for two releases. How can that be done for flags?

    11. encukou commented on Jul 20, 2021

      @encukou
      encukou
      on Jul 20, 2021
      Member

      The bit cannot be repurposed, since older extensions using the stable ABI might set it.
      It doesn't make much sense to remove the Py_TPFLAGS_HAVE_VERSION_TAG or Py_TPFLAGS_HAVE_FINALIZE defines; I'd let them stay to document that the bits are reserved.

    12. miss-islington commented on Jul 23, 2021

      @miss-islington
      miss-islington
      on Jul 23, 2021
      Contributor

      New changeset a4760cc by Petr Viktorin in branch 'main':
      bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VERSION_TAG no-op (GH-27260)
      a4760cc

    13. encukou commented on Jul 23, 2021

      @encukou
      encukou
      on Jul 23, 2021
      Member

      New changeset 632e8a6 by Miss Islington (bot) in branch '3.10':
      bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VERSION_TAG no-op (GH-27260) (GH-27306)
      632e8a6

    14. pablogsal commented on Jul 23, 2021

      @pablogsal
      pablogsal
      on Jul 23, 2021
      Member

      Is there anything left in this issue?

    15. encukou commented on Jul 23, 2021

      @encukou
      encukou
      on Jul 23, 2021
      Member

      I usually wait until buildbots are green before closing the bpo, but I don't think there's anything else.

    16. encukou
      closed this as completedon Jul 23, 2021
    17. encukou
      closed this as completedon Jul 23, 2021
    18. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.10 (EOL)end of lifetopic-C-API

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions