GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-135228: Create __dict__ and __weakref__ descriptors for object by encukou · Pull Request #136966 · python/cpython

Conversation

@encukou

@encukou encukou commented Jul 22, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Here's another possible way to solve #135228:
The __dict__ and __weakref__ descriptors don't really need to hold a strong reference to "their" type; they are generic and could be created for object instead.
Here's a proof of concept. A cleaner way to do this would be creating the descriptor objects once and caching them in the interpreter state.

This does involve a behaviour change -- before:

>>> class C: pass
>>> C.__dict__['__dict__']
<attribute '__dict__' of 'C' objects>
>>> C.__dict__['__dict__'].__objclass__
<class '__main__.C'>

After:

>>> class C: pass
>>> C.__dict__['__dict__']
<attribute '__dict__' of 'object' objects>
>>> C.__dict__['__dict__'].__objclass__
<class 'object'>
  • Issue: 3.14 regression: slot dataclasses classes leak original class #135228

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
JelleZijlstra and others added 5 commits July 20, 2025 16:57
@JelleZijlstra
pythongh-135228: When @DataClass(slots=True) replaces a dataclass, ma…
027f629 
…ke the original class collectible

An interesting hack, but more localized in scope than python#135230.

This may be a breaking change if people intentionally keep the original class around
when using `@dataclass(slots=True)`, and then use `__dict__` or `__weakref__` on the
original class.
@encukou
Keep just the test
c82d86b
@encukou
Create __dict__ and __weakref__ descriptors for object
9ef8632
@encukou
Adjust internals of inspect.getattr_static
696f1d0
@encukou
Merge in the main branch
58c7a39
@bedevere-app bedevere-app Bot mentioned this pull request Jul 22, 2025
3.14 regression: slot dataclasses classes leak original class #135228
Closed
serhiy-storchaka
serhiy-storchaka reviewed Jul 22, 2025
View reviewed changes
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@encukou

encukou commented Jul 22, 2025

Copy link
Copy Markdown
Member Author

I can polish this, but I think it'd be better to target 3.15, and follow through a bit more.
The descriptor objects themselves could be shared across all classes that need them (within an interpreter), but it's not as straightforward to plug that into the type creation machinery.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou added 4 commits July 30, 2025 17:05
@encukou
Share __dict__ and __weakref__ descriptors
d0d2f73
@encukou
Merge in the main branch
f709deb
@encukou
Adjust __repr__ of generic descriptors (ones whose __objclass__ is type)
dc541cd
@encukou
Add PyGetSetDef to c-analyzer's list of types
1e2fd8a
@encukou

encukou commented Jul 30, 2025

Copy link
Copy Markdown
Member Author

Here's a polished version: the descriptors are made for type, which means thet skip type-checking (they work on any object). The getter/setter functions already check if their argument has a __dict__/__weakref__.

And since they're the same for all objects, they can be shared across all types in an interpreter -- that is, cached in interpreter state.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou added 2 commits August 5, 2025 10:25
@encukou
Add blurb
9a40a98
@encukou
Merge in the main branch
71bd841
@encukou
encukou marked this pull request as ready for review August 5, 2025 08:44
@encukou
encukou requested review from ZeroIntensity, ericsnowcurrently and markshannon as code owners August 5, 2025 08:44
@bedevere-app bedevere-app Bot added the awaiting core review label Aug 5, 2025
ZeroIntensity
ZeroIntensity reviewed Aug 5, 2025
View reviewed changes
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@encukou encukou mentioned this pull request Aug 11, 2025
gh-135228: When @dataclass(slots=True) replaces a dataclass, make the original class collectible (take 2) #137047
Merged
encukou added 5 commits August 12, 2025 17:32
@encukou
Merge in the main branch
61230e2
@encukou
Stop calling _clear_type_descriptors
e414d29
@encukou
Remove sys._clear_type_descriptors
a71d91a
@encukou
Add tests for the generic descriptors
64ebb4f
@encukou
Add to What's New
699ea77
@encukou
encukou requested review from AA-Turner and ericvsmith as code owners August 12, 2025 16:34
@encukou

encukou commented Aug 12, 2025

Copy link
Copy Markdown
Member Author

This now partially reverts #137047. I don't think keeping sys._clear_type_descriptors is worth it. If removed, power users can test its presence to see whether it's needed.
JelleZijlstra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JelleZijlstra JelleZijlstra mentioned this pull request Aug 15, 2025
gh-135228: Break reference cycle between class and descriptors #135230
Closed
JelleZijlstra
JelleZijlstra approved these changes Aug 15, 2025
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a nice improvement! I think there's risk this will break some advanced introspection code, similar to the change you had to make to inspect, but if it goes in early in the 3.15 cycle, that should be OK.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_descr.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Aug 15, 2025
@JelleZijlstra JelleZijlstra mentioned this pull request Aug 15, 2025
Use sys._clear_type_descriptors() on 3.14 to break slot reference cycle python-attrs/attrs#1459
Merged
@serhiy-storchaka

serhiy-storchaka commented Aug 15, 2025

Copy link
Copy Markdown
Member

I suggest to wait some time, maybe until 3.14.1, so we can have chance to get some feedback about an alternate approach and decide whether it is worth to backport this approach to 3.14. We may also find completely different solution of the original problem in 3.15.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou
encukou commented Aug 15, 2025
View reviewed changes
Comment thread Doc/whatsnew/3.15.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
encukou
encukou commented Aug 15, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-08-05-10-22-15.gh-issue-136966.J5lrE0.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@encukou @JelleZijlstra
Add side effect note; apply review suggestions for test
1909b6d 
Co-authored-by: Jelle Zijlstra <[email protected]>
@encukou

encukou commented Aug 15, 2025

Copy link
Copy Markdown
Member Author

I suggest to wait some time [before backporting]

Definitely! Personally, I don't think this should be backported at all. (But I wouldn't have backported #137047 to a RC either if that was up to me...)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
Merge branch 'main' into hackweakref-descr
758669d
@encukou
encukou merged commit 7dfa048 into python:main Aug 18, 2025
@encukou
encukou deleted the hackweakref-descr branch August 18, 2025 12:25
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 18, 2025
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@encukou @JelleZijlstra @Agent-Hellboy
pythongh-135228: Create __dict__ and __weakref__ descriptors for obje…
a2ba4a5 
…ct (pythonGH-136966)

This partially reverts python#137047, keeping the tests for GC collectability of the
original class that dataclass adds `__slots__` to.
The reference leaks solved there are instead solved by having the `__dict__` &
`__weakref__` descriptors not tied to (and referencing) their class.

Instead, they're shared between all classes that need them (within
an interpreter).
The `__objclass__` ol the descriptors is set to `object`, since these
descriptors work with *any* object. (The appropriate checks were already
made in the get/set code, so the `__objclass__` check was redundant.)

The repr of these descriptors (and any others whose `__objclass__` is `object`)
now doesn't mention the objclass.

This change required adjustment of introspection code that checks
`__objclass__` to determine an object's “own” (i.e. not inherited) `__dict__`.
Third-party code that does similar introspection of the internals will also
need adjusting.


Co-authored-by: Jelle Zijlstra <[email protected]>
@mjp41 mjp41 mentioned this pull request Dec 11, 2025
Changes made in sprint for PLDI deadline. fxpl/cpython#62
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@ZeroIntensity ZeroIntensity ZeroIntensity left review comments
@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@encukou @serhiy-storchaka @JelleZijlstra @ZeroIntensity
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.