gh-143969: Fix frozen+slotted dataclass __setattr__ crash ambiguity #143971
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
sobolevn
left a comment
sobolevn
left a comment
There was a problem hiding this comment.
Please, add a test case. And please, do not change lines that are just changed because of the formatting. Only change lines that are directly related to your change :)
Thanks for the PR!
dawdameet
force-pushed
the
fix-frozen-slots-setattr
branch
from
3d83ed6 to
9f63ad5
Compare
Lib/dataclasses.py
Outdated
| ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', | ||
| f' super(cls, self).__setattr__(name, value)'), | ||
| ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', | ||
| ' object.__setattr__(self, name, value)'), |
There was a problem hiding this comment.
I don't think that this fix is correct, because it ignores parent's __setattr__ method.
The root cause of the bug might be something related to the __closure__ cell of the property? See https://github.com/python-attrs/attrs/blob/28526a872ec882c69d5d30932c1a520ef2520309/src/attr/_make.py#L959-L990 for the context.
Or it might be not :)
There was a problem hiding this comment.
I am really sorry, I pushed the object.setattr change just before seeing your comment!
You are right, bypassing super breaks inheritance for any parent classes that override setattr. I will revert the object change locally and look into the closure / class cell solution you linked. I'll update the PR once I have a fix that respects the MRO. Thank you for the heads up! I'm learning a lot from this!
dawdameet
force-pushed
the
fix-frozen-slots-setattr
branch
from
9f63ad5 to
1c3e0c9
Compare
…osure cells When a dataclass is frozen and uses slots, the class object is replaced. This causes generated methods using super() to fail because their closure cells point to the old class. This fix updates the closure cells in the generated methods to point to the new class.
dawdameet
force-pushed
the
fix-frozen-slots-setattr
branch
from
1c3e0c9 to
34cfdda
Compare
| @@ -0,0 +1,2 @@ | |||
| Fixed a crash in frozen slotted dataclasses where assigning to an attribute | |||
| could raise an internal TypeError instead of failing cleanly. | |||
There was a problem hiding this comment.
| could raise an internal TypeError instead of failing cleanly. | |
| could raise an internal :exc:`TypeError` instead of failing cleanly. |
3 participants
Fixed Misleading "Not an instance or subtype" error when assigning to property of frozen, slotted dataclass.
Fix a crash in frozen dataclasses with slots where the generated
setattr and delattr methods used super(cls, self).
When slots rebuild the class, the captured cls no longer matches the
instance type, causing a TypeError at runtime.
Use object.setattr and object.delattr instead to preserve frozen
semantics safely.
gh-143969