Prevent class reassignment across incompatible layouts #6521

Copilot · 2025-12-25T10:29:50Z

Guard class setter to reject incompatible layouts while still permitting module-style class swaps (e.g., lazy modules and request modules) when their layouts match, avoiding panics and keeping import/ensurepip flows working.
Move the regression coverage for layout-mismatch errors into extra_tests/snippets/builtin_type.py and remove the Rust-side unit test per review feedback.
Ran targeted rustpython-vm test to verify the build after the guard adjustment.

Original prompt

This section details on the original issue you should resolve

<issue_title>Panic when setting class with different layout</issue_title>
<issue_description>## Summary
Setting object's class attribute to a type with different layout cause panic

thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', vm/src/object/core.rs:813:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

which is an error in cpython

TypeError: __class__ assignment: 'TypeB' object layout differs from 'TypeA'

Detailed Explanation

class TypeA:
    def __init__(self) -> None:
        self.a = 1

class TypeB:
    __slots__ = "b"
    def __init__(self) -> None:
        self.b = 2

obj = TypeA()
assert obj.a == 1

obj.__class__ = TypeB
print(obj.b)
```</issue_description>

<agent_instructions>payload type_id() must be cheked</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@youknowone</author><body>
maybe similar thing cause both this one and  RustPython/RustPython#1979</body></comment_new>
</comments>

Fixes Panic when setting __class__ with different layout #4634

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Summary by CodeRabbit

Bug Fixes
- Enhanced validation for dynamic class reassignment to enforce object layout compatibility.
- Now raises TypeError when attempting to reassign an instance to a class with incompatible layout, slot differences, or dict-slot presence mismatches.
Tests
- Added test coverage for class layout compatibility constraints and dynamic class reassignment scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-25T10:29:55Z

📝 Walkthrough

Walkthrough

The PR adds layout compatibility checks for __class__ reassignment in RustPython's object implementation. When assigning a new class to an instance's __class__ attribute, the code now validates that both types have compatible memory layouts by comparing basicsize, slot definitions, and dict-slot presence. If layouts differ, it raises a descriptive TypeError instead of panicking.

Changes

Cohort / File(s)	Summary
Core `__class__` assignment logic `crates/vm/src/builtins/object.rs`	Added layout compatibility validation for `__class__` reassignment. Introduced `has_dict` predicate and `slots_equal` comparison helper functions. When either current or assigned type is mutable or a ModuleType subclass, validates that basicsize, slot names, and dict-slot presence match; raises TypeError if they differ. Caches `current_cls` variable for consistent class checks.
Test coverage `extra_tests/snippets/builtin_type.py`	Added test cases validating TypeError is raised when reassigning `__class__` to types with differing layouts (different basicsize or slots). Added test confirming reassignment succeeds when slot definitions match despite different class objects.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

__slots__ xor __dict__ , name mangling #6392: Modifies HAS_DICT propagation and slot name mangling logic that are dependencies for the new layout-compatibility checks in this PR.

Poem

🐰 A panic becomes proper grace,
When checking if types fit the space!
Slots align and basicsizes match,
No more crashes—we've sealed the hatch! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Prevent class reassignment across incompatible layouts' accurately summarizes the main change: adding a guard to reject class reassignment when object and target layouts are incompatible.
Linked Issues check	✅ Passed	The pull request fully addresses the requirements in issue #4634: it prevents panics during class reassignment by detecting layout incompatibility and raising a TypeError instead, matching CPython behavior.
Out of Scope Changes check	✅ Passed	All changes are directly related to the objective of preventing class reassignment across incompatible layouts; modifications to object.rs implement the guard and tests in builtin_type.py provide regression coverage as requested.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch copilot/fix-class-attribute-panic

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-12-25T11:41:25Z

Code has been automatically formatted

The code in this PR has been formatted using cargo fmt --all.
Please pull the latest changes before pushing again:

git pull origin copilot/fix-class-attribute-panic

youknowone · 2025-12-25T11:45:05Z

crates/vm/src/vm/interpreter.rs

Instead of adding test in rust code, add test test in builtin_type.py

youknowone · 2025-12-25T12:31:37Z

@copilot try again

Copilot · 2025-12-25T12:40:10Z

@copilot try again

Moved the regression coverage into builtin_type.py and removed the Rust unit test in 22490e3.

youknowone · 2025-12-25T16:32:32Z

@copilot investigate regressions in test_descr test_importlib test_venv

Copilot · 2025-12-25T16:44:06Z

@copilot investigate regressions in test_descr test_importlib test_venv

CI failures came from module class reassignment (_LazyModule/RequestModule) tripping the new layout guard. Relaxed the payload check so module subclasses can swap classes when layouts match in 9376bcd; ensurepip/importlib/venv now proceed.

Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>

youknowone · 2025-12-26T15:46:59Z

@coderabbitai review

coderabbitai · 2025-12-26T15:47:04Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

extra_tests/snippets/builtin_type.py (1)
280-291: Consider adding an assertion to verify the class was actually changed.

The test verifies that slot_a.__class__ = SlotA2 doesn't raise, but doesn't verify the assignment actually took effect.
🔎 Proposed addition
 slot_a = SlotA()
 slot_a.__class__ = SlotA2  # Should work
+assert type(slot_a) is SlotA2
+assert slot_a.__class__ is SlotA2
crates/vm/src/builtins/object.rs (1)

529-538: Itemsize check is missing—already noted in TODO comments.

The layout compatibility check covers basicsize, slot names/count, and HAS_DICT differences but omits itemsize comparison. This gap is already documented in TODO comments elsewhere in the codebase (crates/vm/src/builtins/object.rs and crates/vm/src/builtins/type.rs). Variable-size object handling is relevant for RustPython's buffer protocol and variable-size types, though the scope of user-defined variable-size classes may differ from CPython. Weakref layout differences are also not checked; consider whether RustPython's weakref handling requires explicit layout validation for class assignment.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b704f42 and e0090ff.

📒 Files selected for processing (2)

crates/vm/src/builtins/object.rs
extra_tests/snippets/builtin_type.py

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/builtins/object.rs

**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.py: In most cases, Python code should not be edited; bug fixes should be made through Rust code modifications only
Follow PEP 8 style for custom Python code
Use ruff for linting Python code

Files:

extra_tests/snippets/builtin_type.py

🧬 Code graph analysis (1)

extra_tests/snippets/builtin_type.py (1)

extra_tests/snippets/testutils.py (1)

assert_raises (5-12)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run tests under miri
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Ensure compilation on various targets
GitHub Check: Check the WASM package and demo
GitHub Check: Check Rust code with clippy

🔇 Additional comments (4)

crates/vm/src/builtins/object.rs (2)

500-502: Good refactoring to cache current_cls.

Caching instance.class() ensures consistency throughout the method, avoiding potential issues if the class were to change between checks (though unlikely in practice).

510-528: The positional slot comparison using zip is overly restrictive and does not match CPython's actual behavior.

CPython allows class reassignment even when slots are in different orders (e.g., ("x", "y") and ("y", "x")), but the current implementation rejects such cases because it compares slots positionally. This should either be fixed to allow reordered slots (like CPython does) or have a clear documented reason for the stricter validation.

Likely an incorrect or invalid review comment.

extra_tests/snippets/builtin_type.py (2)

243-260: Good coverage for dict vs slots layout mismatch.

This test correctly verifies that assigning a class with __slots__ to an instance of a class with __dict__ raises a TypeError with the appropriate message. The use of assert_raises context manager with message validation is good practice.

263-277: Nice edge case: same slot count, different slot names.

This test is valuable because it validates that the implementation compares slot names, not just slot counts. This catches a potential bug where only len() comparison might have been used.

Copilot AI assigned Copilot and youknowone Dec 25, 2025

Copilot started work on behalf of youknowone December 25, 2025 10:30 View session

Copilot AI changed the title ~~[WIP] Fix panic when setting __class__ with different layout~~ Prevent __class__ reassignment across incompatible layouts Dec 25, 2025

Copilot AI requested a review from youknowone December 25, 2025 10:55

Copilot finished work on behalf of youknowone December 25, 2025 10:55

youknowone requested changes Dec 25, 2025

View reviewed changes

crates/vm/src/vm/interpreter.rs

Copy link

Member

youknowone Dec 25, 2025

Choose a reason for hiding this comment

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

Instead of adding test in rust code, add test test in builtin_type.py

Copilot started work on behalf of youknowone December 25, 2025 11:45 View session

Copilot AI requested a review from youknowone December 25, 2025 11:53

Copilot started work on behalf of youknowone December 25, 2025 12:33 View session

Copilot finished work on behalf of youknowone December 25, 2025 12:40

Copilot started work on behalf of youknowone December 25, 2025 16:33 View session

Copilot finished work on behalf of youknowone December 25, 2025 16:46

Copilot AI and others added 6 commits December 27, 2025 00:33

Initial plan

3271b74

fix: guard __class__ reassignment layout

a3fb6fe

Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>

Auto-format: cargo fmt --all

fbdb400

test: move __class__ layout check to builtin_type snippet

71e9eb7

Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>

fix: allow module __class__ reassignment with matching layout

89e0801

Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>

Fix condition

e0090ff

youknowone force-pushed the copilot/fix-class-attribute-panic branch from 9376bcd to e0090ff Compare December 26, 2025 15:46

youknowone marked this pull request as ready for review December 26, 2025 15:46

coderabbitai bot reviewed Dec 26, 2025

View reviewed changes

youknowone approved these changes Dec 26, 2025

View reviewed changes

youknowone merged commit 27ab62d into main Dec 26, 2025
13 checks passed

youknowone deleted the copilot/fix-class-attribute-panic branch December 26, 2025 16:16

Prevent __class__ reassignment across incompatible layouts #6521

Prevent __class__ reassignment across incompatible layouts #6521

Uh oh!

Conversation

Copilot AI commented Dec 25, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Detailed Explanation

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

github-actions bot commented Dec 25, 2025

Uh oh!

youknowone Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

youknowone commented Dec 25, 2025

Uh oh!

Copilot AI commented Dec 25, 2025

Uh oh!

youknowone commented Dec 25, 2025

Uh oh!

Copilot AI commented Dec 25, 2025

Uh oh!

youknowone commented Dec 26, 2025

Uh oh!

coderabbitai bot commented Dec 26, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Prevent class reassignment across incompatible layouts #6521

Prevent class reassignment across incompatible layouts #6521

Copilot AI commented Dec 25, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 25, 2025 •

edited

Loading