gh-105227: Add PyType_GetDict() by ericsnowcurrently · Pull Request #105747 · python/cpython

ericsnowcurrently · 2023-06-13T20:27:31Z

This compensates for static builtin types having tp_dict set to NULL.

Issue: tp_dict slot of static builtin types is NULL in 3.12, without mention in the changelog or an alternative #105227

📚 Documentation preview 📚: https://cpython-previews--105747.org.readthedocs.build/

ericsnowcurrently · 2023-06-13T20:29:44Z

CC @The-Compiler. @tacaswell, @philthompson10

encukou · 2023-06-14T07:11:55Z

Doc/c-api/typeobj.rst

+   **<<>>**: Names in double angle brackets should be initially set to
+   ``NULL`` and treated as read-only after initialization.


Only marking up tp_dict implies it's the only one in this category. At least tp_bases & tp_mro should be marked too.

encukou · 2023-06-14T07:17:36Z

@Yhg1s needs to approve backporting to 3.12

encukou · 2023-06-14T07:20:00Z

Doc/c-api/type.rst

+   replacement for accessing :c:member:`~PyTypeObject.tp_dict` directly.
+   The returned dictionary must be treated as read-only.
+
+   This function isn't intended for general use.  It's meant for


If it's not meant for general use, would it be better to add it as unstable API?

To be honest, I think it makes more sense to say this is for general use.

erlend-aasland

Can you update Doc/data/refcounts.dat, so we get a "Return value: New reference" markup in the docs?

erlend-aasland · 2023-06-13T22:56:30Z

Doc/c-api/type.rst

+   This function isn't intended for general use.  It's meant for
+   specific embedding and language-binding cases, where direct access
+   to the dict is necessary and indirect access (e.g. via the proxy)
+   isn't adequate.  Extension modules may continue to use ``tp_dict``,
+   directly or indirectly, when setting up their own types.


Maybe elevate this paragraph to a .. note:, or maybe a .. warning:?

erlend-aasland · 2023-06-13T23:01:43Z

Misc/NEWS.d/next/C API/2023-06-13-14-24-55.gh-issue-105227.HDL9aF.rst

+The new :c:func:`PyType_GetDict` provides the dictionary for the given type
+object that is normally exposed by ``cls.__dict__``.  Normally it's
+sufficient to use :c:member:`~PyTypeObject.tp_dict`, but for the static
+builtin types ``tp_dict`` is now always ``NULL``.  ``PyType_GetDict()``


Formatting nit:

Suggested change

builtin types ``tp_dict`` is now always ``NULL``. ``PyType_GetDict()``

builtin types :c:member:`!tp_dict` is now always ``NULL``. :c:func:`!PyType_GetDict()`

Yhg1s

Approving the new API as RM: 👍

Yhg1s · 2023-06-15T14:47:07Z

Doc/c-api/type.rst

+   replacement for accessing :c:member:`~PyTypeObject.tp_dict` directly.
+   The returned dictionary must be treated as read-only.
+
+   This function isn't intended for general use.  It's meant for


To be honest, I think it makes more sense to say this is for general use.

tacaswell · 2023-06-20T15:13:29Z

~~The problem has also been fixed on the PyQt6 side without this PR (I must plead ignorance of exactly what those changes are), so I am not sure how critical this PR is.~~

Sorry, I last track of which branch I had built CPython from 🐑 .

What I meant to say is:

I can confirm that the latest version of pyqt6 works with this branch so this change is critical.

philthompson10 · 2023-06-20T15:24:34Z

PyQt now uses this call. Strictly speaking it could do the C equivalent of 'cls.dict' but the call is much nicer.

This should make pyqt6 build with Python 3.12. For more infor see: python#105747

This patch should make pyqt6 build with Python 3.12. For more info see: python#105747

ericsnowcurrently · 2023-06-28T09:40:13Z

I'm not available enough to merge this for a few weeks, so anyone that feels comfortable doing so is welcome to in the meantime. 😄

The-Compiler · 2023-07-10T14:19:23Z

Any updates on this? This currently blocks me (with qutebrowser, via PyQt) from testing Python 3.12 on CI.

encukou · 2023-07-10T15:07:13Z

Oh dear, I didn't realis a few weeks is all the time until Beta. And I was busy so it didn't reach the top of my TODO list.
I'll push some tests and a doc update.

miss-islington · 2023-07-10T16:41:07Z

Thanks @ericsnowcurrently for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

This compensates for static builtin types having `tp_dict` set to `NULL`. (cherry picked from commit a840806) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

bedevere-bot · 2023-07-10T16:41:18Z

GH-106600 is a backport of this pull request to the 3.12 branch.

gh-105227: Add PyType_GetDict() (GH-105747) This compensates for static builtin types having `tp_dict` set to `NULL`. (cherry picked from commit a840806) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

The-Compiler · 2023-07-11T10:31:06Z

Oh whoops, I wasn't even realizing how close to the Beta 4 I was commenting (apparently it was scheduled for yesterday, but not released yet?)!

Glad to hear this still made it in, thanks so much @ericsnowcurrently and @encukou!

Yhg1s · 2023-07-11T10:59:07Z

FWIW, this PR was one of the reasons 3.12b4 wasn't cut yesterday :)

erlend-aasland · 2023-07-11T20:21:58Z

Doc/c-api/typeobj.rst

+
+   .. versionchanged:: 3.12
+
+      Internals detail: For static builtin types, this is always ``NULL``.


Sorry for the post merge review, but FYI, there's a directive for implementation details: .. impl-detail::

ericsnowcurrently · 2023-07-17T15:44:34Z

Thanks for wrapping this up, @encukou!

ericsnowcurrently added 3 commits June 13, 2023 14:16

Add PyType_GetDict().

49f5e75

Add docs.

60d611e

Add a NEWS entry.

718bbb6

ericsnowcurrently added the needs backport to 3.12 only security fixes label Jun 13, 2023

ericsnowcurrently requested a review from ronaldoussoren June 13, 2023 20:27

ericsnowcurrently requested a review from markshannon as a code owner June 13, 2023 20:27

bedevere-bot added the awaiting core review label Jun 13, 2023

bedevere-bot mentioned this pull request Jun 12, 2023

tp_dict slot of static builtin types is NULL in 3.12, without mention in the changelog or an alternative #105227

Closed

ericsnowcurrently removed the request for review from markshannon June 13, 2023 20:57

Fix a typo.

269738d

encukou reviewed Jun 14, 2023

View reviewed changes

erlend-aasland reviewed Jun 14, 2023

View reviewed changes

Yhg1s reviewed Jun 15, 2023

View reviewed changes

hrnciar pushed a commit to fedora-python/cpython that referenced this pull request Jun 21, 2023

00402: Add PyType_GetDict()

8c20bb1

This should make pyqt6 build with Python 3.12. For more infor see: python#105747

hrnciar pushed a commit to fedora-python/cpython that referenced this pull request Jun 21, 2023

00402: Add PyType_GetDict()

f11d97d

This patch should make pyqt6 build with Python 3.12. For more info see: python#105747

encukou added 6 commits July 10, 2023 17:14

Docs: Mark this for general use (better than reading tp_dict directly!)

ac9a071

Linkify the NEWS entry

8368595

Add a rudimentary test

23b3a41

Docs: Revert the <<>> in the summary -- the info is in prose for now

7ed5d86

Merge in the main branch

8d9b9ff

Formatting nit

97e1d9c

encukou merged commit a840806 into python:main Jul 10, 2023

bedevere-bot removed the awaiting core review label Jul 10, 2023

bedevere-bot removed the needs backport to 3.12 only security fixes label Jul 10, 2023

erlend-aasland reviewed Jul 11, 2023

View reviewed changes

ericsnowcurrently deleted the pytype-getdict branch July 17, 2023 15:44

		<<>>: Names in double angle brackets should be initially set to
		``NULL`` and treated as read-only after initialization.

	builtin types ``tp_dict`` is now always ``NULL``. ``PyType_GetDict()``
	builtin types :c:member:`!tp_dict` is now always ``NULL``. :c:func:`!PyType_GetDict()`


		.. versionchanged:: 3.12

		Internals detail: For static builtin types, this is always ``NULL``.

Uh oh!

Conversation

ericsnowcurrently commented Jun 13, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericsnowcurrently commented Jun 13, 2023

Uh oh!

encukou Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

encukou commented Jun 14, 2023

Uh oh!

encukou Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

Yhg1s Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Jun 13, 2023

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Jun 13, 2023

Choose a reason for hiding this comment

Uh oh!

Yhg1s left a comment

Choose a reason for hiding this comment

Uh oh!

Yhg1s Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philthompson10 commented Jun 20, 2023

Uh oh!

ericsnowcurrently commented Jun 28, 2023

Uh oh!

The-Compiler commented Jul 10, 2023

Uh oh!

encukou commented Jul 10, 2023

Uh oh!

miss-islington commented Jul 10, 2023

Uh oh!

bedevere-bot commented Jul 10, 2023

Uh oh!

The-Compiler commented Jul 11, 2023

Uh oh!

Yhg1s commented Jul 11, 2023

Uh oh!

erlend-aasland Jul 11, 2023

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ericsnowcurrently commented Jun 13, 2023 •

edited by github-actions bot

Loading

tacaswell commented Jun 20, 2023 •

edited

Loading