__type_params__ in __build_class__ #5883
Conversation
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, while the VM ensures these are available 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: Store type_params in .type_params local
Compiler->>Compiler: After body, assign to __type_params__
Compiler->>VM: Emit code referencing .type_params and set TYPE_PARAMS flag
VM->>VM: __build_class__ called
VM->>Namespace: If __type_params__ exists, set .type_params in namespace
VM->>ClassBody: Invoke class body function
VM->>Namespace: Remove .type_params after execution
VM->>ClassObject: After class creation, set __parameters__ from __type_params__
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 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
|
![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.
vm/src/stdlib/builtins.rs
Outdated
| // For PEP 695 classes, set __parameters__ from __type_params__ (CPython compatibility) | ||
| if let Ok(type_params) = class.get_attr(identifier!(vm, __type_params__), vm) { | ||
| if let Ok(type_params_tuple) = type_params.clone().downcast::<PyTuple>() { | ||
| if !type_params_tuple.is_empty() { | ||
| // Extract TypeVar/ParamSpec/TypeVarTuple objects from the tuple | ||
| let mut parameters = Vec::new(); | ||
| for param in type_params_tuple.iter() { | ||
| // Check if this is a TypeVar, ParamSpec, or TypeVarTuple | ||
| if param.get_attr(identifier!(vm, __name__), vm).is_ok() { | ||
| parameters.push(param.clone()); | ||
| } | ||
| } | ||
| if !parameters.is_empty() { | ||
| let parameters_tuple = PyTuple::new_ref(parameters, &vm.ctx); | ||
| class.set_attr(identifier!(vm, __parameters__), parameters_tuple, vm)?; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Correctly implements __parameters__ attribute for PEP 695 compatibility.
The code properly extracts type parameter objects and sets them as the __parameters__ attribute on the class, matching CPython's behavior.
The code uses the presence of __name__ attribute to identify TypeVar/ParamSpec/TypeVarTuple objects. While this should work correctly for objects coming from __type_params__, let's verify this matches CPython's exact behavior:
🌐 Web query:
How does CPython determine which objects from __type_params__ should be included in __parameters__ for PEP 695 generic classes?
💡 Result:
In CPython, when implementing PEP 695 for generic classes, the __type_params__ attribute is used to store the type parameters that parameterize the class. This attribute is a tuple containing instances of TypeVar, TypeVarTuple, and ParamSpec. The __parameters__ attribute, on the other hand, is a tuple that includes the type parameters that are directly used in the class's method signatures.
The determination of which objects from __type_params__ should be included in __parameters__ is based on the usage of these type parameters within the class's methods. Specifically, if a type parameter from __type_params__ is used in the signature of a method (for example, as a type annotation for a method parameter or return type), it is included in __parameters__. This inclusion ensures that the method signatures accurately reflect the types that the class is parameterized with, maintaining consistency and clarity in the class's interface.
This approach aligns with the goals of PEP 695, which aims to simplify the syntax for defining generic classes and to make the relationship between a class and its type parameters more explicit and intuitive. By clearly associating type parameters with method signatures, developers can more easily understand and utilize generic classes in their code.
Implementation of __parameters__ needs to filter by method‐signature usage
The CPython logic for PEP 695 only includes type parameters in __parameters__ if they actually appear in the class’s method signatures (as annotations). Simply checking for a __name__ attribute will pull in all TypeVar/ParamSpec/TypeVarTuple instances, not just those used on methods.
Please update the block in
vm/src/stdlib/builtins.rs (lines 984–1002)
so that, after collecting candidates from __type_params__, you further scan the class’s methods’ __annotations__ and include only those parameters that appear there.
[e.g.]
- // heuristic: has __name__
- if param.get_attr(__name__, vm).is_ok() {
- parameters.push(param.clone());
- }
+ // only include if this TypeVar/ParamSpec/TypeVarTuple
+ // is referenced in any method signature annotation:
+ if class
+ .get_methods()
+ .iter()
+ .any(|func| func.annotations().contains(¶m))
+ {
+ parameters.push(param.clone());
+ }— adjust to match your VM’s API for iterating methods and inspecting annotations.
This will ensure feature parity with CPython’s PEP 695 implementation.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In vm/src/stdlib/builtins.rs around lines 984 to 1002, the current code sets
__parameters__ by including all TypeVar/ParamSpec/TypeVarTuple objects from
__type_params__ without filtering by actual usage in method signatures. To fix
this, after collecting candidate parameters from __type_params__, iterate over
the class’s methods and inspect their __annotations__ to identify which
parameters are actually used. Then filter the collected parameters to include
only those found in these annotations before setting __parameters__. Use your
VM’s API to access methods and their annotations for this filtering step.
Summary by CodeRabbit
New Features
Documentation