typing.TypeVar by youknowone · Pull Request #5834 · RustPython/RustPython

youknowone

youknowone commented
Jun 24, 2025
•

edited by coderabbitai Bot

Loading

Summary by CodeRabbit

New Features
- Enhanced support for Python's typing system, including full support for TypeVar default values and improved integration with the typing module.
- Introduced a new singleton type for typing defaults, accessible throughout the environment.
- Improved error messages for type-related errors.
Bug Fixes
- Tests for typing constructs and generic aliases are now treated as standard passing tests, reflecting improved reliability.
Documentation
- Expanded instructions for running Python test modules, offering more flexible and granular test execution options.

Walkthrough

The changes introduce enhanced support for Python's typing module in a Rust-based Python VM. This includes a comprehensive refactor of the internal _typing module, full implementation of TypeVar default values, and the addition of a NoDefault singleton type. Related test decorators are updated, error messages are improved, and new fields are added to core VM structures for typing support.

Changes

File(s)	Change Summary
.github/copilot-instructions.md	Expanded test instructions for RustPython, providing more granular commands for running specific modules and test functions.
Lib/test/test_typing.py	Removed `@unittest.expectedFailure` decorators from multiple test methods, treating them as standard passing tests; added one expected failure on `test_pickling`.
Lib/test/test_genericalias.py	Removed `@unittest.expectedFailure` decorators and TODO comments from `test_pickle` and `test_copy` methods.
Lib/test/test_types.py	Removed `@unittest.expectedFailure` decorators and TODO comments from `test_union_pickle` and `test_union_copy` methods.
vm/src/builtins/type.rs	Updated error message in `best_base` to use `slot_name()` instead of `name()` for base type representation.
vm/src/stdlib/typing.rs	Major refactor: Enhanced `_typing` module, implemented `TypeVar` default support, improved argument parsing and error handling, added `NoDefaultType` as a singleton, and improved integration with Python's typing functions.
vm/src/types/zoo.rs	Added `typing_no_default_type` field to `TypeZoo` and initialized it in `init()`.
vm/src/vm/context.rs	Added `typing_no_default` singleton field to `Context` and initialized it in `init_genesis()`.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant VM
    participant TypingModule
    participant PythonTyping

    User->>VM: Create TypeVar(name, constraints, bound, default)
    VM->>TypingModule: TypeVar.__new__(...)
    TypingModule->>TypingModule: Parse & validate arguments
    TypingModule->>PythonTyping: Call _type_check for constraints/bound
    PythonTyping-->>TypingModule: Return checked types
    TypingModule->>VM: Return TypeVar instance with default (or NoDefault)

sequenceDiagram
    participant VM
    participant TypingModule

    VM->>TypingModule: Access NoDefault singleton
    TypingModule->>VM: Return NoDefault instance from context

Poem

In the warren of types, a new path we unfold,
With TypeVars that default, and NoDefault of old.
The tests now all pass, no failures in sight,
As typing grows stronger, our code feels just right.
🐇✨

✨ Finishing Touches

📝 Generate Docstrings

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)

.github/copilot-instructions.md (1)
87-91: Quote the env-var to avoid word-splitting in bash

Shell will word-split ${TEST_MODULE} if it ever contains spaces or shell-metacharacters. Safer example:
-cargo run --release -- -m test ${TEST_MODULE}
+cargo run --release -- -m test "${TEST_MODULE}"
Pure documentation change – feel free to ignore if you don’t expect funky module names.
Lib/test/test_typing.py (1)
9709-9710: Add a descriptive reason to the TODO marker.

Per the repo’s guideline, comments accompanying unittest.expectedFailure should specify the RustPython limitation that triggers the failure. A brief reason makes triage easier and avoids the inevitable “why is this still xfail?” question in a few months.
-    # TODO: RUSTPYTHON
+    # TODO: RUSTPYTHON: NoDefault is not yet picklable

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a992d4 and 42c8539.

📒 Files selected for processing (6)

.github/copilot-instructions.md (1 hunks)
Lib/test/test_typing.py (1 hunks)
vm/src/builtins/type.rs (1 hunks)
vm/src/stdlib/typing.rs (5 hunks)
vm/src/types/zoo.rs (2 hunks)
vm/src/vm/context.rs (3 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

`**/*.rs`: Follow the default rustfmt code style; use 'cargo fmt' to format Rust code. Always run clippy to lint Rust code ('cargo clippy') before completing tasks, and fix any war...

**/*.rs: Follow the default rustfmt code style; use '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 your changes.

vm/src/builtins/type.rs
vm/src/types/zoo.rs
vm/src/vm/context.rs
vm/src/stdlib/typing.rs

`Lib/**/*`: Files in the Lib/ directory (copied from CPython) should be edited very conservatively; modifications should be minimal and only to work around RustPython limitations.

Lib/**/*: Files in the Lib/ directory (copied from CPython) should be edited very conservatively; modifications should be minimal and only to work around RustPython limitations.

Lib/test/test_typing.py

`Lib/test/**/*`: Tests in Lib/test often use markers such as '# TODO: RUSTPYTHON', 'unittest.skip("TODO: RustPython ")', or 'unittest.expectedFailure' with a '# TODO: RUSTP...

Lib/test/**/*: Tests in Lib/test often use markers such as '# TODO: RUSTPYTHON', 'unittest.skip("TODO: RustPython ")', or 'unittest.expectedFailure' with a '# TODO: RUSTPYTHON ' comment when modifications are made.

Lib/test/test_typing.py

`**/*.py`: Follow PEP 8 style for custom Python code. Use ruff for linting Python code.

**/*.py: Follow PEP 8 style for custom Python code.
Use ruff for linting Python code.

Lib/test/test_typing.py

⏰ Context from checks skipped due to timeout of 90000ms (6)

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

🔇 Additional comments (15)

vm/src/builtins/type.rs (1)

1498-1503: Good call – aligns with CPython’s tp_name

Switching from name() to slot_name() yields the fully-qualified type name (incl. module path) and matches the wording produced by CPython’s PyType.tp_name. No functional risk spotted.

vm/src/types/zoo.rs (2)

89-93: Field addition looks correct

typing_no_default_type is added in the struct and initialised below. Type is &'static Py<PyType> so #[derive(Clone)] still works and no extra trait bounds are needed.

181-185: Initialisation path is consistent

crate::stdlib::typing::NoDefault::init_builtin_type() returns the expected &'static Py<PyType> and mirrors how other singleton types are registered, so the new field is initialised coherently.

vm/src/vm/context.rs (3)

44-45: New singleton stored in Context

The extra field is correctly typed (PyRef<…>) and keeps ordering with struct initialisation later in the file.

283-287: Singleton instance creation mirrors existing pattern

create_object(NoDefault, NoDefault::static_type()) follows the same zero-sized-struct pattern used for None, Ellipsis, etc. No issues.

323-333: Struct construction remains in declaration order

typing_no_default is inserted between none and empty_tuple, matching the revised struct layout – prevents a potential field-init-shorthand compiler error.

vm/src/stdlib/typing.rs (9)

1-11: LGTM! Clean module structure with proper singleton access.

The module now properly extends the _typing module with the NoDefault singleton from the VM context, which is the correct pattern for singleton types in the VM.

22-39: Well-designed helper functions for typing module interaction.

Both functions properly handle VM interactions:

_call_typing_func_object correctly imports and calls typing module functions

type_check appropriately avoids bootstrapping issues by special-casing None

47-62: TypeVar struct properly extended for default value support.

The addition of default_value and evaluate_default fields, along with HAS_DICT flag and appropriate traits, correctly implements the infrastructure needed for TypeVar defaults.

64-132: Comprehensive property implementations with proper lazy evaluation.

All property getters are well-implemented:

mro_entries correctly prevents TypeVar subclassing

Properties use consistent locking patterns for thread safety

default property properly returns the NoDefault singleton when appropriate

134-162: TypeVar methods correctly implement Python typing semantics.

The methods properly handle:

typing_subst delegates to the Python typing module as expected

reduce supports pickling by returning the name

has_default correctly checks both lazy evaluation and NoDefault singleton

164-178: String representation correctly follows Python typing conventions.

The variance indicators (+, -, ~) properly represent covariant, contravariant, and invariant type variables respectively.

221-266: Robust validation logic for TypeVar constraints.

The implementation correctly validates:

Bivariant types are rejected

Variance and infer_variance are mutually exclusive

Single constraints are disallowed

Constraints and bounds cannot be used together

Bounds are properly type-checked

409-439: NoDefault singleton correctly implements Python typing.NoDefault semantics.

The implementation properly:

Enforces singleton pattern through VM context

Prevents attribute assignment with no_attr flag

Validates that no arguments are accepted

Provides appropriate string representations

298-316: make_typevar correctly initializes new default fields.

The function properly initializes the new default_value and evaluate_default fields to None, maintaining consistency with the TypeVar implementation.

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)

vm/src/stdlib/typing.rs (1)
181-226: Complex constructor with thorough validation - verify past duplicate removal issue.

The constructor implementation is comprehensive with proper argument parsing and validation. However, there was a past review comment about duplicate key removal on line 188 that needs verification.

Looking at the current code, I don't see the duplicate kwargs.swap_remove("name") call mentioned in the past review. The code appears to correctly handle the name parameter extraction. Let me verify this hasn't regressed:
#!/bin/bash
# Search for any duplicate swap_remove calls on the same key
rg -A 3 -B 3 'swap_remove.*"name"' --type rust

🧹 Nitpick comments (1)

vm/src/stdlib/typing.rs (1)

150-160: Consider simplifying the has_default method logic.

The has_default method has nested conditions that could be streamlined for better readability:

 fn has_default(&self, vm: &VirtualMachine) -> bool {
-    if !vm.is_none(&self.evaluate_default) {
-        return true;
-    }
-    let default_value = self.default_value.lock();
-    if vm.is_none(&default_value) {
-        return false;
-    }
-    // Check if default_value is NoDefault
-    !default_value.is(&vm.ctx.typing_no_default)
+    // Check evaluate_default first
+    if !vm.is_none(&self.evaluate_default) {
+        return true;
+    }
+    
+    // Check if default_value is set and not NoDefault
+    let default_value = self.default_value.lock();
+    !vm.is_none(&default_value) && !default_value.is(&vm.ctx.typing_no_default)
 }

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42c8539 and 0c31c5f.

📒 Files selected for processing (5)

Lib/test/test_typing.py (1 hunks)
vm/src/builtins/type.rs (1 hunks)
vm/src/stdlib/typing.rs (5 hunks)
vm/src/types/zoo.rs (2 hunks)
vm/src/vm/context.rs (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

vm/src/builtins/type.rs
vm/src/types/zoo.rs
vm/src/vm/context.rs
Lib/test/test_typing.py

🧰 Additional context used

📓 Path-based instructions (1)

`**/*.rs`: Follow the default rustfmt code style; use 'cargo fmt' to format Rust code. Always run clippy to lint Rust code ('cargo clippy') before completing tasks, and fix any war...

**/*.rs: Follow the default rustfmt code style; use '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 your changes.

vm/src/stdlib/typing.rs

⏰ Context from checks skipped due to timeout of 90000ms (11)

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

🔇 Additional comments (8)

vm/src/stdlib/typing.rs (8)

1-11: LGTM: Clean module organization.

The new module structure properly separates the internal _typing module from the public interface and correctly integrates the NoDefault singleton from the VM context.

22-39: Well-designed helper functions for typing integration.

The _call_typing_func_object function provides a clean abstraction for calling Python typing module functions, and type_check appropriately handles the bootstrapping issue by avoiding direct imports during initialization.

242-256: Robust constraint validation logic.

The constraint handling properly validates single constraints (which aren't allowed) and prevents mixing constraints with bounds. The tuple creation and assignment logic is correct.

259-269: Proper type checking for bounds.

The bound validation correctly calls type_check to ensure type safety, which is essential for TypeVar constraints. Good error handling for None values.

291-299: Smart module name detection from frame globals.

Setting __module__ from the current frame's globals is a clever way to maintain proper module attribution. The None check prevents setting invalid module names.

317-322: Update make_typevar for consistency.

Good that the make_typevar helper function has been updated to initialize the new default_value and evaluate_default fields, maintaining consistency with the enhanced TypeVar structure.

416-446: Excellent singleton implementation for NoDefaultType.

The NoDefaultType implementation properly enforces the singleton pattern by:

Rejecting any constructor arguments

Returning the singleton instance from VM context

Providing appropriate string representation

Implementing proper reduction for pickling

This is a textbook example of singleton implementation in Rust.

56-61: Verify thread safety with multiple mutexes.

The TypeVar struct now has three mutex-protected fields (bound, constraints, default_value). While each individual mutex is thread-safe, consider whether operations that need to access multiple fields simultaneously could lead to inconsistent states.
#!/bin/bash
# Check for patterns where multiple mutex fields might be accessed together
rg -A 10 -B 2 "\.lock" --type rust | rg -A 5 -B 5 "constraints|bound|default_value"

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)

vm/src/stdlib/typing.rs (1)
307-325: Consider using NoDefault singleton for consistency.

The make_typevar helper function initializes default_value with vm.ctx.none(), while the main constructor uses the NoDefault singleton when no default is provided. This inconsistency could lead to different behavior in has_default() method calls.

Consider updating for consistency:
-            default_value: parking_lot::Mutex::new(vm.ctx.none()),
+            default_value: parking_lot::Mutex::new(vm.ctx.typing_no_default.clone().into()),

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 616f5ab and 3c02e33.

📒 Files selected for processing (6)

Lib/test/test_genericalias.py (0 hunks)
Lib/test/test_types.py (0 hunks)
Lib/test/test_typing.py (1 hunks)
vm/src/stdlib/typing.rs (5 hunks)
vm/src/types/zoo.rs (2 hunks)
vm/src/vm/context.rs (3 hunks)

💤 Files with no reviewable changes (2)

Lib/test/test_genericalias.py
Lib/test/test_types.py

🚧 Files skipped from review as they are similar to previous changes (3)

vm/src/types/zoo.rs
vm/src/vm/context.rs
Lib/test/test_typing.py

🧰 Additional context used

📓 Path-based instructions (1)

`**/*.rs`: Follow the default rustfmt code style; use 'cargo fmt' to format Rust code. Always run clippy to lint Rust code ('cargo clippy') before completing tasks, and fix any war...

**/*.rs: Follow the default rustfmt code style; use '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 your changes.

vm/src/stdlib/typing.rs

⏰ Context from checks skipped due to timeout of 90000ms (11)

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

🔇 Additional comments (7)

vm/src/stdlib/typing.rs (7)

1-11: LGTM! Clean module setup with proper NoDefault integration.

The module structure is well-organized and correctly integrates the NoDefault singleton from the VM context into the typing module.

22-39: LGTM! Well-implemented helper functions.

The helper functions are cleanly implemented with proper error handling and good performance optimizations (like the None check in type_check).

46-118: LGTM! Excellent enhancement of TypeVar with proper thread safety.

The addition of default value support and variance properties is well-implemented. The use of parking_lot::Mutex provides appropriate thread safety, and the lazy evaluation pattern for constraints and bounds is efficient.

119-159: LGTM! Correct implementation of default value handling.

The default property correctly distinguishes between None and the NoDefault singleton, and the has_default method properly implements the logic for determining if a default value exists. The lazy evaluation pattern is consistent with other properties.

176-305: LGTM! Comprehensive and well-validated TypeVar constructor.

The constructor implementation is excellent with:

Proper argument parsing for both positional and keyword arguments

Thorough validation of variance combinations and constraint/bound conflicts

Clear and descriptive error messages

Correct initialization of default values using the NoDefault singleton

The duplicate key removal issue from the past review has been properly addressed

418-448: LGTM! Excellent singleton implementation of NoDefault.

The NoDefault type is properly implemented as a singleton with:

Correct singleton enforcement in the constructor by returning the context instance

Proper argument validation (rejecting any arguments)

Clean string representation and reduce method

Thread-safe singleton pattern through the VM context

449-532: LGTM! Supporting typing constructs are appropriately implemented.

The remaining typing constructs (TypeVarTuple, ParamSpec, TypeAliasType, Generic) are well-structured and follow consistent patterns. The Generic class correctly implements __class_getitem__ for generic alias creation.

This was referenced

Jun 25, 2025

This was referenced

Feb 4, 2026

typing.TypeVar by youknowone · Pull Request #5834 · RustPython/RustPython

youknowone commented Jun 24, 2025 • edited by coderabbitai Bot Loading

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowone commented
Jun 24, 2025
•

edited by coderabbitai Bot

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)