__type_params__ in __build_class__#5883
Conversation
Sorry, something went wrong.
WalkthroughThe changes add support for handling type parameters (as per PEP 695) in both the compiler and runtime. The compiler now compiles and stores type parameters for classes and functions in a dedicated local variable and symbol table scope, while the VM manages these parameters during class body execution and assigns them to the class’s Changes
Sequence Diagram(s)sequenceDiagram
participant Compiler
participant VM
participant Namespace
participant ClassBody
participant ClassObject
Compiler->>Compiler: Compile class/function with type_params
Compiler->>Compiler: Push special symbol table scope
Compiler->>Compiler: Compile and store type_params in .type_params local
Compiler->>Compiler: Compile body
Compiler->>Compiler: Load .type_params and store as __type_params__ local
Compiler->>Compiler: Pop special symbol table scope
Compiler->>VM: Emit code referencing .type_params and set TYPE_PARAMS flag
VM->>VM: __build_class__ called
VM->>Namespace: If __type_params__ exists and non-empty, set .type_params in namespace
VM->>ClassBody: Invoke class body function
VM->>Namespace: Remove .type_params from namespace after execution
VM->>ClassObject: After class creation, set __type_params__ attribute from function if present
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)error: failed to load source for dependency Caused by: Caused by: Caused by: 📜 Recent review detailsConfiguration used: .coderabbit.yml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (11)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
vm/src/stdlib/builtins.rs (1)
935-948: Implementation correctly handles type parameters for PEP 695 classes.The code properly retrieves
__type_params__from the function object and makes it available in the namespace during class body execution. Good use of type checking withdowncast.Consider whether silently ignoring the
get_attrerror is intentional. If__type_params__is expected but missing, you might want to log or handle this case explicitly.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Lib/test/test_typing.pyis excluded by!Lib/**
📒 Files selected for processing (3)
compiler/codegen/src/compile.rs(4 hunks)compiler/codegen/src/symboltable.rs(1 hunks)vm/src/stdlib/builtins.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rs`: Follow the default rustfmt code style (`cargo fmt` to format) Always ...
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your 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
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
compiler/codegen/src/symboltable.rsvm/src/stdlib/builtins.rscompiler/codegen/src/compile.rs
🧠 Learnings (1)
vm/src/stdlib/builtins.rs (1)
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T10:08:48.851Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run tests under miri
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
🔇 Additional comments (6)
compiler/codegen/src/symboltable.rs (1)
1270-1270: LGTM! Good documentation addition.The comment clearly explains the purpose of the subsequent code block.
vm/src/stdlib/builtins.rs (1)
961-965: Proper cleanup of temporary namespace entry.Good practice to remove the internal
.type_paramskey after use, maintaining CPython compatibility. The use of.ok()is appropriate since the key may not exist in all cases.compiler/codegen/src/compile.rs (4)
1215-1215: LGTM: Clear documentation improvement.The added comment helpfully clarifies the purpose of the type parameter compilation process.
1668-1674: LGTM: Correct type parameter compilation and storage.The implementation properly:
- Guards the type parameter handling with a conditional check
- Manages symbol table scope correctly by pushing a new table
- Reuses existing
compile_type_paramsfunctionality- Uses the well-chosen
.type_paramsvariable name to avoid conflicts- Employs the appropriate
StoreLocalinstruction for the scope
1696-1707: LGTM: Correct PEP 695 type_params implementation.This properly implements the PEP 695 requirement by:
- Making type parameters available as
__type_params__during class body execution- Using
LoadNameAnyto correctly access.type_paramsfrom the enclosing scope- Using
StoreLocalto properly set the variable in the class body scope- Including clear comments explaining the PEP 695 compliance purpose
1741-1745: LGTM: Proper type parameter preparation for class creation.This correctly implements the final phase of type parameter handling by:
- Loading the previously compiled
.type_paramsfrom the appropriate scope- Setting the
TYPE_PARAMSflag to inform the runtime that type parameters are present- Using the correct
LoadNameAnyinstruction for cross-scope accessThe implementation ensures type parameters are properly passed to the
__build_class__builtin.
Sorry, something went wrong.
youknowone
force-pushed
the
type-params
branch
3 times, most recently
from
1373ebb to
1a1b6f3
Compare
July 3, 2025 04:36
youknowone
merged commit 8a2a6af
into
RustPython:main
Jul 3, 2025
Summary by CodeRabbit
New Features
Documentation
Chores
__class_getitem__as class methods on built-in types (bytearray,bytes,memoryview,range, andslice) pending upstream Python support.