Visitar URL original
Make the PySubclass layout contract unsafe by 1ndahous3 · Pull Request #9012 · RustPython/RustPython · GitHub
Skip to content

Make the PySubclass layout contract unsafe - #9012

Merged
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:pysubclass_unsafe_contract
Oct 8, 2026
Merged

Make the PySubclass layout contract unsafe#9012
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:pysubclass_unsafe_contract

Conversation

@1ndahous3

@1ndahous3 1ndahous3 commented Oct 8, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Make PySubclass an unsafe trait: manual implementations must guarantee a valid base prefix, matching payload offsets, and compatible object alignment for safe base conversions.
  • Preserve automatic implementations for macro-generated subclasses and document the layout guarantees of native exception implementations.

Extracted from #8963 as requested in #8963 (comment). The allocation-based cast checks remain in #8963.

API changes

  • Manual implementations now require unsafe impl PySubclass. Callers of base conversions and users of generated implementations need no additional unsafe blocks.

AI assistance

Written with Codex (GPT-6), reviewed by a human before submission.

Summary by CodeRabbit

  • Safety
    • Subclass relationships now require an explicit unsafe implementation, making the layout and lifetime guarantees required for safe conversions explicit.
    • Built-in class, exception, and struct-sequence subclass implementations have been updated to satisfy the new requirement.
  • Compatibility
    • Custom implementations of the subclassing trait must now be declared unsafe; ordinary implementations will no longer compile.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 522ae3b5-04c4-40fb-97bb-581c7d1a017b
📥 Commits

Reviewing files that changed from the base of the PR and between 54e47cd and 988e088.

📒 Files selected for processing (5)
  • crates/derive-impl/src/pyclass.rs
  • crates/derive-impl/src/pystructseq.rs
  • crates/vm/src/class.rs
  • crates/vm/src/exception_group.rs
  • crates/vm/src/exceptions.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

PySubclass is now an unsafe trait with documented layout and validity requirements. Generated implementations for transparent classes and struct sequences, and implementations for built-in exception types, are now declared unsafe.

Changes

PySubclass safety contract

Layer / File(s) Summary
Define the trait safety requirements
crates/vm/src/class.rs		 PySubclass is now an unsafe trait. Its documentation defines layout and validity requirements and adds a compile-fail example for a safe manual implementation.
Update generated and built-in implementations
crates/derive-impl/src/pyclass.rs, crates/derive-impl/src/pystructseq.rs, crates/vm/src/exception_group.rs, crates/vm/src/exceptions.rs		 Generated transparent-class and struct-sequence implementations, along with built-in exception implementations, are now unsafe. Safety comments describe the associated layout checks.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 988e0

This change makes the PySubclass layout contract explicit by marking the trait unsafe. Callers and macro users see no behavior change, and no merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 clearly and concisely describes the main change: making the PySubclass layout contract unsafe.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@1ndahous3 1ndahous3 mentioned this pull request Oct 8, 2026
Open
1 task done
youknowone
youknowone approved these changes Oct 8, 2026
View reviewed changes

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you so much!

@youknowone
youknowone merged commit 06389eb into RustPython:main Oct 8, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

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

@1ndahous3 @youknowone