Skip to content

gh-118033: Fix __weakref__ not set for generic dataclasses#118099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
sobolevn merged 6 commits into from
May 9, 2024
Merged

gh-118033: Fix __weakref__ not set for generic dataclasses #118099

sobolevn merged 6 commits into from
May 9, 2024

Conversation

@sobolevn
Copy link
Member

@sobolevnsobolevn commented Apr 19, 2024•
edited by bedevere-app bot
Loading

Looks like that classes with __weakrefoffset__ == 0 does not need to add __weakref__ slot to existing super ones.

@bedevere-appbedevere-appbot mentioned this pull request Apr 19, 2024
Closed
@sobolevn
Copy link
MemberAuthor

sobolevn commented Apr 19, 2024•
edited
Loading

No, this is not correct, we also need to check __dictoffset__ as well. For __dict__ case.

@sobolevn
Copy link
MemberAuthor

sobolevn commented Apr 19, 2024

Ubuntu failures are not related, there are some network problems right now

carljm
carljm approved these changes Apr 22, 2024
View reviewed changes
Copy link
Member

@carljmcarljm left a comment

Choose a reason for hiding this comment

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

I think this fix is correct; some suggestions and a question.

@sobolevnsobolevn requested a review from carljmMay 4, 2024 09:05
carljm
carljm approved these changes May 7, 2024
View reviewed changes
Copy link
Member

@carljmcarljm left a comment

Choose a reason for hiding this comment

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

This looks fine to me. Sorry I didn't see the notification sooner that you requested another review!

I still think the best version of this might look at __weakrefoffset__ and __dictrefoffset__ always to decide on those two slots, but I agree that's an edge case and can be handled separately; this is still a clear improvement and fixes the original bug.

@sobolevnsobolevn added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels May 9, 2024
@sobolevnsobolevn enabled auto-merge (squash) May 9, 2024 08:09
@sobolevnsobolevn disabled auto-merge May 9, 2024 08:35
@sobolevnsobolevn enabled auto-merge (squash) May 9, 2024 08:35
@sobolevnsobolevn merged commit fa9b9cb into python:mainMay 9, 2024
@miss-islington-app
Copy link

miss-islington-appbot commented May 9, 2024

Thanks @sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 9, 2024
@sobolevn@miss-islington
pythongh-118033: Fix__weakref__not set for generic dataclasses (p…
9725fb0
…ythonGH-118099) (cherry picked from commit fa9b9cb) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 9, 2024
@sobolevn@miss-islington
pythongh-118033: Fix__weakref__not set for generic dataclasses (p…
cf4a239
…ythonGH-118099) (cherry picked from commit fa9b9cb) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-app
Copy link

bedevere-appbot commented May 9, 2024

GH-118821 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13 bugs and security fixes label May 9, 2024
@bedevere-app
Copy link

bedevere-appbot commented May 9, 2024

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

@bedevere-appbedevere-appbot removed the needs backport to 3.12 only security fixes label May 9, 2024
sobolevn added a commit that referenced this pull request May 9, 2024
@miss-islington@sobolevn
[3.13]gh-118033: Fix__weakref__not set for generic dataclasses (G…
632682c
…H-118099) (#118821) gh-118033: Fix `__weakref__` not set for generic dataclasses (GH-118099) (cherry picked from commit fa9b9cb) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this pull request May 9, 2024
@miss-islington@sobolevn
[3.12]gh-118033: Fix__weakref__not set for generic dataclasses (G…
e953956
…H-118099) (#118822) gh-118033: Fix `__weakref__` not set for generic dataclasses (GH-118099) (cherry picked from commit fa9b9cb) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@sobolevnsobolevn mentioned this pull request May 9, 2024
Open
@JelleZijlstraJelleZijlstra mentioned this pull request May 28, 2024
Closed
@sobolevnsobolevn mentioned this pull request Sep 11, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljmcarljmcarljm approved these changes

@ericvsmithericvsmithAwaiting requested review from ericvsmithericvsmith is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sobolevn@carljm