recurse into decorator-wrapped methods when rewriting closure cells for slotted classes#1583
Open
HrachShah wants to merge 4 commits into
Open
Conversation
os.environb yields bytes on POSIX and the documented use case
for to_bool is reading values out of environment variables. The
truthy and falsy lookup tuples only contain str and int, so a
bytes('true') input raised 'Cannot convert value to bool: b\'true\''
even though its ASCII-decoded value is the canonical truthy
literal.
Decode bytes and bytearray as ASCII before the lowercase + lookup
step, matching the str path. Non-ASCII bytes raise ValueError
('Cannot convert value to bool: ...') the same way an unknown
string does, since the decoded text would still not match any
known literal. Unknown byte values raise the same ValueError.
When a class is rebuilt from attrs.fields() via attrs.make_class, the 'these' mapping contains Attribute instances rather than _CountingAttr objects. from_counting_attr was only reading the private _default, _validator, and _converter attributes, which Attribute does not expose, so the rebuild crashed with "'Attribute' object has no attribute '_default'". Detect an Attribute argument and read the public default/validator/ converter attributes directly. Add a regression test and a changelog entry.
…or slotted classes
for more information, see https://pre-commit.ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does
Fixes #1038 —
super()and__class__references in decorated methods on slottedattrsclasses now resolve correctly.The bug
@attr.s(slots=True)rebuilds the class viatype(self._cls)(...)so it can attach a fresh__slots__. After the rebuild, every method on the class has a closure cell that still points at the pre-rebuild class, so the existing closure-cell rewrite loop walks the class dict and rewrites eachcell.cell_contents is self._clsto the new class.The walker only looks at each
cls.__dict__entry one level deep. When a method is wrapped by a decorator, the entry in the class dict is the wrapper function. The wrapper's closure cell contains the original (unwrapped) method, and that method is the one whose own closure cell bakes the reference to__class__(because thesuper()/__class__use lives inside the method body, not inside the decorator's wrapper). The walker never descends into the wrapper's cell contents, so the inner method's__class__cell is left pointing at the old class.On non-slotted attrs classes this is invisible because the class is never rebuilt. On slotted classes, calling the decorated method raises
TypeError: super(type, obj): obj must be an instance or subtype of typebecausesuper()looks up__class__from the baked closure cell, which still points at the pre-rebuild class.The fix
_ClassBuilder._create_slots_classnow factors the cell-rewriting into a_rewrite_closure_cellshelper that recurses: when a cell's contents are themselves a function, the helper descends into that function's own__closure__and rewrites the inner cells too. That way, a wrapper-around-method sees the inner method's__class__cell rewritten, andsuper()/__class__resolve to the new class.The recursion only descends into cells whose contents are functions; it does not look inside arbitrary callables. The same classmethod / staticmethod / property unwrapping that the original loop did is preserved.
Tests
New test in
tests/test_slots.py::TestClosureCellRewriting::test_decorated_method_with_supercovers bothslots=Trueandslots=Falseand exercises:super()in a decorated method on a subclass (the issue's exact reproducer).__class__in a decorated method on a class with no inheritance.Pre-fix, the
slots=Trueparametrization of the new test fails withTypeError: super(type, obj): obj must be an instance or subtype of type. Post-fix, both parametrizations pass.Verification
python -m pytest tests/test_slots.py -v→ 55 passed, 1 skipped (was 53 passed, 1 skipped before; +2 from the new parametrized test).python -m pytest tests/test_slots.py tests/test_make.py tests/test_dunders.py tests/test_functional.py tests/test_annotations.py tests/test_setattr.py→ 947 passed, 5 skipped (no regressions).python -m pytest tests/ --ignore=tests/dataclass_transform_example.py --ignore=tests/strategies.py(excluding the pre-existing collection errors in those two files unrelated to this change) → 1841 passed, 6 skipped.Backport
This bug is reproducible on the 3.14 zlib header branch and on
main; the fix lives entirely in_ClassBuilder._create_slots_classand applies unchanged to both. Maintainers, please cherry-pick to the supported release branches as appropriate.