typevar as module #6834
Merged
typevar as module #6834
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughA significant refactoring reorganizes typing-related types into a dedicated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/vm/src/stdlib/typevar.rs`:
- Around line 423-530: ParamSpec::__default__ currently evaluates
self.evaluate_default on every call but doesn't cache the result; modify the
method so that after calling the evaluate_default callable you store the result
into self.default_value (acquire the same lock as used for evaluate_default,
e.g., let mut default_value = self.default_value.lock(); *default_value =
evaluated_result.clone(); ) and then return that cached value; ensure the
initial NoDefault check still uses vm.ctx.typing_no_default and that you return
the cached default on subsequent calls instead of re-invoking evaluate_default.
- Around line 268-383: The TypeVar implementation (Constructor::py_new for
TypeVar) is not validating that the provided constraints are actual types nor
that the name is a string; likewise ParamSpec and TypeVarTuple constructors need
name validation and ParamSpec needs its bound validated with type_check. Fix: in
TypeVar::py_new ensure the extracted name is validated as a str (use vm.to_str
or equivalent/type_check for "name must be a str") and run type_check on each
constraint value (and on bound where present) before storing them; mirror this
logic in the ParamSpec constructor (validate name and call type_check on bound)
and in the TypeVarTuple constructor (validate name as str), referencing the
functions/constructors named py_new for TypeVar, ParamSpec, and TypeVarTuple and
the existing type_check helper to perform the validations and return TypeError
on invalid inputs.
Comment on lines
+268
to
383
| impl Constructor for TypeVar { | ||
| type Args = FuncArgs; | ||
|
|
||
| fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { | ||
| let typevar = <Self as Constructor>::py_new(&cls, args, vm)?; | ||
| let obj = typevar.into_ref_with_type(vm, cls)?; | ||
| let obj_ref: PyObjectRef = obj.into(); | ||
| set_module_from_caller(&obj_ref, vm)?; | ||
| Ok(obj_ref) | ||
| } | ||
| fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { | ||
| let typevar = <Self as Constructor>::py_new(&cls, args, vm)?; | ||
| let obj = typevar.into_ref_with_type(vm, cls)?; | ||
| let obj_ref: PyObjectRef = obj.into(); | ||
| set_module_from_caller(&obj_ref, vm)?; | ||
| Ok(obj_ref) | ||
| } | ||
|
|
||
| fn py_new(_cls: &Py<PyType>, args: Self::Args, vm: &VirtualMachine) -> PyResult<Self> { | ||
| let mut kwargs = args.kwargs; | ||
| // Parse arguments manually | ||
| let (name, constraints) = if args.args.is_empty() { | ||
| // Check if name is provided as keyword argument | ||
| if let Some(name) = kwargs.swap_remove("name") { | ||
| (name, vec![]) | ||
| fn py_new(_cls: &Py<PyType>, args: Self::Args, vm: &VirtualMachine) -> PyResult<Self> { | ||
| let mut kwargs = args.kwargs; | ||
| // Parse arguments manually | ||
| let (name, constraints) = if args.args.is_empty() { | ||
| // Check if name is provided as keyword argument | ||
| if let Some(name) = kwargs.swap_remove("name") { | ||
| (name, vec![]) | ||
| } else { | ||
| return Err( | ||
| vm.new_type_error("TypeVar() missing required argument: 'name' (pos 1)") | ||
| ); | ||
| } | ||
| } else if args.args.len() == 1 { | ||
| (args.args[0].clone(), vec![]) | ||
| } else { | ||
| return Err( | ||
| vm.new_type_error("TypeVar() missing required argument: 'name' (pos 1)") | ||
| ); | ||
| let name = args.args[0].clone(); | ||
| let constraints = args.args[1..].to_vec(); | ||
| (name, constraints) | ||
| }; | ||
|
|
||
| let bound = kwargs.swap_remove("bound"); | ||
| let covariant = kwargs | ||
| .swap_remove("covariant") | ||
| .map(|v| v.try_to_bool(vm)) | ||
| .transpose()? | ||
| .unwrap_or(false); | ||
| let contravariant = kwargs | ||
| .swap_remove("contravariant") | ||
| .map(|v| v.try_to_bool(vm)) | ||
| .transpose()? | ||
| .unwrap_or(false); | ||
| let infer_variance = kwargs | ||
| .swap_remove("infer_variance") | ||
| .map(|v| v.try_to_bool(vm)) | ||
| .transpose()? | ||
| .unwrap_or(false); | ||
| let default = kwargs.swap_remove("default"); | ||
|
|
||
| // Check for unexpected keyword arguments | ||
| if !kwargs.is_empty() { | ||
| let unexpected_keys: Vec<String> = kwargs.keys().map(|s| s.to_string()).collect(); | ||
| return Err(vm.new_type_error(format!( | ||
| "TypeVar() got unexpected keyword argument(s): {}", | ||
| unexpected_keys.join(", ") | ||
| ))); | ||
| } | ||
| } else if args.args.len() == 1 { | ||
| (args.args[0].clone(), vec![]) | ||
| } else { | ||
| let name = args.args[0].clone(); | ||
| let constraints = args.args[1..].to_vec(); | ||
| (name, constraints) | ||
| }; | ||
|
|
||
| let bound = kwargs.swap_remove("bound"); | ||
| let covariant = kwargs | ||
| .swap_remove("covariant") | ||
| .map(|v| v.try_to_bool(vm)) | ||
| .transpose()? | ||
| .unwrap_or(false); | ||
| let contravariant = kwargs | ||
| .swap_remove("contravariant") | ||
| .map(|v| v.try_to_bool(vm)) | ||
| .transpose()? | ||
| .unwrap_or(false); | ||
| let infer_variance = kwargs | ||
| .swap_remove("infer_variance") | ||
| .map(|v| v.try_to_bool(vm)) | ||
| .transpose()? | ||
| .unwrap_or(false); | ||
| let default = kwargs.swap_remove("default"); | ||
|
|
||
| // Check for unexpected keyword arguments | ||
| if !kwargs.is_empty() { | ||
| let unexpected_keys: Vec<String> = kwargs.keys().map(|s| s.to_string()).collect(); | ||
| return Err(vm.new_type_error(format!( | ||
| "TypeVar() got unexpected keyword argument(s): {}", | ||
| unexpected_keys.join(", ") | ||
| ))); | ||
| } | ||
|
|
||
| // Check for invalid combinations | ||
| if covariant && contravariant { | ||
| return Err(vm.new_value_error("Bivariant type variables are not supported.")); | ||
| } | ||
|
|
||
| if infer_variance && (covariant || contravariant) { | ||
| return Err(vm.new_value_error("Variance cannot be specified with infer_variance")); | ||
| } | ||
|
|
||
| // Handle constraints and bound | ||
| let (constraints_obj, evaluate_constraints) = if !constraints.is_empty() { | ||
| // Check for single constraint | ||
| if constraints.len() == 1 { | ||
| return Err(vm.new_type_error("A single constraint is not allowed")); | ||
| // Check for invalid combinations | ||
| if covariant && contravariant { | ||
| return Err(vm.new_value_error("Bivariant type variables are not supported.")); | ||
| } | ||
| if bound.is_some() { | ||
| return Err(vm.new_type_error("Constraints cannot be used with bound")); | ||
| } | ||
| let constraints_tuple = vm.ctx.new_tuple(constraints); | ||
| (constraints_tuple.clone().into(), constraints_tuple.into()) | ||
| } else { | ||
| (vm.ctx.none(), vm.ctx.none()) | ||
| }; | ||
|
|
||
| // Handle bound | ||
| let (bound_obj, evaluate_bound) = if let Some(bound) = bound { | ||
| if vm.is_none(&bound) { | ||
| (vm.ctx.none(), vm.ctx.none()) | ||
| } else { | ||
| // Type check the bound | ||
| let bound = type_check(bound, "Bound must be a type.", vm)?; | ||
| (bound, vm.ctx.none()) | ||
| if infer_variance && (covariant || contravariant) { | ||
| return Err(vm.new_value_error("Variance cannot be specified with infer_variance")); | ||
| } | ||
| } else { | ||
| (vm.ctx.none(), vm.ctx.none()) | ||
| }; | ||
|
|
||
| // Handle default value | ||
| let (default_value, evaluate_default) = if let Some(default) = default { | ||
| (default, vm.ctx.none()) | ||
| } else { | ||
| // If no default provided, use NoDefault singleton | ||
| (vm.ctx.typing_no_default.clone().into(), vm.ctx.none()) | ||
| }; | ||
| // Handle constraints and bound | ||
| let (constraints_obj, evaluate_constraints) = if !constraints.is_empty() { | ||
| // Check for single constraint | ||
| if constraints.len() == 1 { | ||
| return Err(vm.new_type_error("A single constraint is not allowed")); | ||
| } | ||
| if bound.is_some() { | ||
| return Err(vm.new_type_error("Constraints cannot be used with bound")); | ||
| } | ||
| let constraints_tuple = vm.ctx.new_tuple(constraints); | ||
| (constraints_tuple.clone().into(), constraints_tuple.into()) | ||
| } else { | ||
| (vm.ctx.none(), vm.ctx.none()) | ||
| }; | ||
|
|
||
| Ok(Self { | ||
| name, | ||
| bound: parking_lot::Mutex::new(bound_obj), | ||
| evaluate_bound, | ||
| constraints: parking_lot::Mutex::new(constraints_obj), | ||
| evaluate_constraints, | ||
| default_value: parking_lot::Mutex::new(default_value), | ||
| evaluate_default: PyMutex::new(evaluate_default), | ||
| covariant, | ||
| contravariant, | ||
| infer_variance, | ||
| }) | ||
| } | ||
| } | ||
| // Handle bound | ||
| let (bound_obj, evaluate_bound) = if let Some(bound) = bound { | ||
| if vm.is_none(&bound) { | ||
| (vm.ctx.none(), vm.ctx.none()) | ||
| } else { | ||
| // Type check the bound | ||
| let bound = type_check(bound, "Bound must be a type.", vm)?; | ||
| (bound, vm.ctx.none()) | ||
| } | ||
| } else { | ||
| (vm.ctx.none(), vm.ctx.none()) | ||
| }; | ||
|
|
||
| impl TypeVar { | ||
| pub fn new( | ||
| vm: &VirtualMachine, | ||
| name: PyObjectRef, | ||
| evaluate_bound: PyObjectRef, | ||
| evaluate_constraints: PyObjectRef, | ||
| ) -> Self { | ||
| Self { | ||
| name, | ||
| bound: parking_lot::Mutex::new(vm.ctx.none()), | ||
| evaluate_bound, | ||
| constraints: parking_lot::Mutex::new(vm.ctx.none()), | ||
| evaluate_constraints, | ||
| default_value: parking_lot::Mutex::new(vm.ctx.typing_no_default.clone().into()), | ||
| evaluate_default: PyMutex::new(vm.ctx.none()), | ||
| covariant: false, | ||
| contravariant: false, | ||
| infer_variance: false, | ||
| // Handle default value | ||
| let (default_value, evaluate_default) = if let Some(default) = default { | ||
| (default, vm.ctx.none()) | ||
| } else { | ||
| // If no default provided, use NoDefault singleton | ||
| (vm.ctx.typing_no_default.clone().into(), vm.ctx.none()) | ||
| }; | ||
|
|
||
| Ok(Self { | ||
| name, | ||
| bound: parking_lot::Mutex::new(bound_obj), | ||
| evaluate_bound, | ||
| constraints: parking_lot::Mutex::new(constraints_obj), | ||
| evaluate_constraints, | ||
| default_value: parking_lot::Mutex::new(default_value), | ||
| evaluate_default: PyMutex::new(evaluate_default), | ||
| covariant, | ||
| contravariant, | ||
| infer_variance, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CPython typing.TypeVar name parameter type validation requirements
💡 Result:
Short answer: the TypeVar "name" is intended to be a str and the docs treat name as a string, but CPython’s typing.TypeVar constructor does not perform a strict runtime isinstance(name, str) check in the implementation — it simply stores the passed value as name. In other words, you should pass a string (e.g. TypeVar("T")); type checkers and the typing docs expect a str, and other tooling may rely on that, even though the CPython typing implementation does not raise if you pass a non-str. [1][2]
Sources
- typing.TypeVar doc (shows name / name is a str). [1]
- CPython typing.py implementation: TypeVar.init assigns self.name = name without explicit name-type validation. [2]
References
[1] Python typing.TypeVar docs.
[2] CPython Lib/typing.py (TypeVar implementation).
🌐 Web query:
CPython typing TypeVar constraints validation as types
💡 Result:
Short answer
- TypeVar constraints must be concrete types (not type variables or parameterized generics); the typing machinery validates that when you create the TypeVar. [1][2][3]
- At runtime the TypeVar stores the constraints in constraints (and bound for bounds) but you cannot use a TypeVar as a runtime type in isinstance/issubclass (isinstance(x, T) raises TypeError). [1][3]
- Type checkers treat subtypes as the listed constraint’s base type for the TypeVar (e.g., MyStr subclass of str is treated as str for the TypeVar). [1][2]
References
[1] Python typing docs (TypeVar constraints, constraints, runtime behavior).
[2] PEPs (PEP 484 / PEP 695 / PEP 696) on constraints/defaults and that constrained types must be concrete.
[3] typing implementation: TypeVar construction uses a runtime _type_check to ensure constraints are types (and raises on invalid constraints).
🏁 Script executed:
# First, let's examine the file structure and the three constructors mentioned
wc -l crates/vm/src/stdlib/typevar.rsRepository: RustPython/RustPython
Length of output: 101
🏁 Script executed:
# Let's look at the ParamSpec constructor (548-619)
sed -n '548,619p' crates/vm/src/stdlib/typevar.rsRepository: RustPython/RustPython
Length of output: 2934
🏁 Script executed:
# Let's look at the TypeVarTuple constructor (735-782)
sed -n '735,782p' crates/vm/src/stdlib/typevar.rsRepository: RustPython/RustPython
Length of output: 2074
🏁 Script executed:
# Check if there's any type_check usage for ParamSpec bound elsewhere
rg -A 3 -B 3 "ParamSpec" crates/vm/src/stdlib/typevar.rs | grep -A 5 -B 5 "bound"Repository: RustPython/RustPython
Length of output: 880
🏁 Script executed:
# Look at the actual ParamSpec struct definition to understand its fields
rg "struct ParamSpec" crates/vm/src/stdlib/typevar.rs -A 10Repository: RustPython/RustPython
Length of output: 1177
Validate TypeVar constraints and type-parameter names.
TypeVar constraints must be validated as types: CPython's typing module enforces constraint validation via _type_check and raises TypeError on invalid constraints, but RustPython stores them without validation. Additionally, the name parameter should be validated as str in TypeVar, ParamSpec, and TypeVarTuple for consistency and better error messages (though CPython doesn't strictly enforce this at runtime). ParamSpec's bound parameter is also not type-checked, unlike TypeVar's bound handling.
🔧 Suggested fix (TypeVar constraints and name validation)
@@
- let (name, constraints) = if args.args.is_empty() {
+ let (name, constraints) = if args.args.is_empty() {
// Check if name is provided as keyword argument
if let Some(name) = kwargs.swap_remove("name") {
(name, vec![])
} else {
@@
- let bound = kwargs.swap_remove("bound");
+ let name = name
+ .downcast::<crate::builtins::PyStr>()
+ .map_err(|_| vm.new_type_error("TypeVar() expects a string name".to_owned()))?
+ .into();
+
+ let bound = kwargs.swap_remove("bound");
@@
- let (constraints_obj, evaluate_constraints) = if !constraints.is_empty() {
+ let constraints = constraints
+ .into_iter()
+ .map(|c| type_check(c, "TypeVar constraints must be types.", vm))
+ .collect::<PyResult<Vec<_>>>()?;
+ let (constraints_obj, evaluate_constraints) = if !constraints.is_empty() {Apply name validation to ParamSpec (548–619) and TypeVarTuple (735–782) constructors. Additionally, validate ParamSpec's bound parameter using type_check.
🤖 Prompt for AI Agents
In `@crates/vm/src/stdlib/typevar.rs` around lines 268 - 383, The TypeVar
implementation (Constructor::py_new for TypeVar) is not validating that the
provided constraints are actual types nor that the name is a string; likewise
ParamSpec and TypeVarTuple constructors need name validation and ParamSpec needs
its bound validated with type_check. Fix: in TypeVar::py_new ensure the
extracted name is validated as a str (use vm.to_str or equivalent/type_check for
"name must be a str") and run type_check on each constraint value (and on bound
where present) before storing them; mirror this logic in the ParamSpec
constructor (validate name and call type_check on bound) and in the TypeVarTuple
constructor (validate name as str), referencing the functions/constructors named
py_new for TypeVar, ParamSpec, and TypeVarTuple and the existing type_check
helper to perform the validations and return TypeError on invalid inputs.
Comment on lines
+423
to
530
| impl ParamSpec { | ||
| #[pymethod] | ||
| fn __mro_entries__(&self, _bases: PyObjectRef, vm: &VirtualMachine) -> PyResult { | ||
| Err(vm.new_type_error("Cannot subclass an instance of ParamSpec")) | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn args(zelf: crate::PyRef<Self>, vm: &VirtualMachine) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| let psa = ParamSpecArgs { | ||
| __origin__: self_obj, | ||
| }; | ||
| Ok(psa.into_ref(&vm.ctx).into()) | ||
| } | ||
| #[pygetset] | ||
| fn __name__(&self) -> PyObjectRef { | ||
| self.name.clone() | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn kwargs(zelf: crate::PyRef<Self>, vm: &VirtualMachine) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| let psk = ParamSpecKwargs { | ||
| __origin__: self_obj, | ||
| }; | ||
| Ok(psk.into_ref(&vm.ctx).into()) | ||
| } | ||
| #[pygetset] | ||
| fn args(zelf: crate::PyRef<Self>, vm: &VirtualMachine) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| let psa = ParamSpecArgs { | ||
| __origin__: self_obj, | ||
| }; | ||
| Ok(psa.into_ref(&vm.ctx).into()) | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn __bound__(&self, vm: &VirtualMachine) -> PyObjectRef { | ||
| if let Some(bound) = self.bound.clone() { | ||
| return bound; | ||
| #[pygetset] | ||
| fn kwargs(zelf: crate::PyRef<Self>, vm: &VirtualMachine) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| let psk = ParamSpecKwargs { | ||
| __origin__: self_obj, | ||
| }; | ||
| Ok(psk.into_ref(&vm.ctx).into()) | ||
| } | ||
| vm.ctx.none() | ||
| } | ||
|
|
||
| #[pygetset] | ||
| const fn __covariant__(&self) -> bool { | ||
| self.covariant | ||
| } | ||
| #[pygetset] | ||
| fn __bound__(&self, vm: &VirtualMachine) -> PyObjectRef { | ||
| if let Some(bound) = self.bound.clone() { | ||
| return bound; | ||
| } | ||
| vm.ctx.none() | ||
| } | ||
|
|
||
| #[pygetset] | ||
| const fn __contravariant__(&self) -> bool { | ||
| self.contravariant | ||
| } | ||
| #[pygetset] | ||
| const fn __covariant__(&self) -> bool { | ||
| self.covariant | ||
| } | ||
|
|
||
| #[pygetset] | ||
| const fn __infer_variance__(&self) -> bool { | ||
| self.infer_variance | ||
| } | ||
| #[pygetset] | ||
| const fn __contravariant__(&self) -> bool { | ||
| self.contravariant | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn __default__(&self, vm: &VirtualMachine) -> PyResult { | ||
| // Check if default_value is NoDefault (not just None) | ||
| if !self.default_value.is(&vm.ctx.typing_no_default) { | ||
| return Ok(self.default_value.clone()); | ||
| #[pygetset] | ||
| const fn __infer_variance__(&self) -> bool { | ||
| self.infer_variance | ||
| } | ||
| // handle evaluate_default | ||
| let evaluate_default = self.evaluate_default.lock(); | ||
| if !vm.is_none(&evaluate_default) { | ||
| let default_value = evaluate_default.call((), vm)?; | ||
| return Ok(default_value); | ||
|
|
||
| #[pygetset] | ||
| fn __default__(&self, vm: &VirtualMachine) -> PyResult { | ||
| // Check if default_value is NoDefault (not just None) | ||
| if !self.default_value.is(&vm.ctx.typing_no_default) { | ||
| return Ok(self.default_value.clone()); | ||
| } | ||
| // handle evaluate_default | ||
| let evaluate_default = self.evaluate_default.lock(); | ||
| if !vm.is_none(&evaluate_default) { | ||
| let default_value = evaluate_default.call((), vm)?; | ||
| return Ok(default_value); | ||
| } | ||
| // Return NoDefault singleton | ||
| Ok(vm.ctx.typing_no_default.clone().into()) | ||
| } | ||
| // Return NoDefault singleton | ||
| Ok(vm.ctx.typing_no_default.clone().into()) | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn evaluate_default(&self, _vm: &VirtualMachine) -> PyObjectRef { | ||
| self.evaluate_default.lock().clone() | ||
| } | ||
| #[pygetset] | ||
| fn evaluate_default(&self, _vm: &VirtualMachine) -> PyObjectRef { | ||
| self.evaluate_default.lock().clone() | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn __reduce__(&self) -> PyResult { | ||
| Ok(self.name.clone()) | ||
| } | ||
| #[pymethod] | ||
| fn __reduce__(&self) -> PyResult { | ||
| Ok(self.name.clone()) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn has_default(&self, vm: &VirtualMachine) -> bool { | ||
| if !vm.is_none(&self.evaluate_default.lock()) { | ||
| return true; | ||
| #[pymethod] | ||
| fn has_default(&self, vm: &VirtualMachine) -> bool { | ||
| if !vm.is_none(&self.evaluate_default.lock()) { | ||
| return true; | ||
| } | ||
| // Check if default_value is not NoDefault | ||
| !self.default_value.is(&vm.ctx.typing_no_default) | ||
| } | ||
| // Check if default_value is not NoDefault | ||
| !self.default_value.is(&vm.ctx.typing_no_default) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn __typing_subst__( | ||
| zelf: crate::PyRef<Self>, | ||
| arg: PyObjectRef, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| _call_typing_func_object(vm, "_paramspec_subst", (self_obj, arg)) | ||
| } | ||
| #[pymethod] | ||
| fn __typing_subst__( | ||
| zelf: crate::PyRef<Self>, | ||
| arg: PyObjectRef, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| _call_typing_func_object(vm, "_paramspec_subst", (self_obj, arg)) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn __typing_prepare_subst__( | ||
| zelf: crate::PyRef<Self>, | ||
| alias: PyObjectRef, | ||
| args: PyObjectRef, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| _call_typing_func_object(vm, "_paramspec_prepare_subst", (self_obj, alias, args)) | ||
| #[pymethod] | ||
| fn __typing_prepare_subst__( | ||
| zelf: crate::PyRef<Self>, | ||
| alias: PyObjectRef, | ||
| args: PyObjectRef, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let self_obj: PyObjectRef = zelf.into(); | ||
| _call_typing_func_object(vm, "_paramspec_prepare_subst", (self_obj, alias, args)) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the structure of the file and locate TypeVar and TypeVarTuple implementations
fd -t f "typevar.rs" crates/vm/src/stdlib/Repository: RustPython/RustPython
Length of output: 96
🏁 Script executed:
# Get line count to understand file size
wc -l crates/vm/src/stdlib/typevar.rsRepository: RustPython/RustPython
Length of output: 101
🏁 Script executed:
# Find TypeVar and TypeVarTuple __default__ implementations in the file
rg -n "__default__" crates/vm/src/stdlib/typevar.rs -A 15Repository: RustPython/RustPython
Length of output: 2906
🏁 Script executed:
# Find the TypeVar struct and its implementation
rg -n "impl TypeVar" crates/vm/src/stdlib/typevar.rs -A 3Repository: RustPython/RustPython
Length of output: 694
🏁 Script executed:
# Find the TypeVarTuple struct and its implementation
rg -n "impl TypeVarTuple" crates/vm/src/stdlib/typevar.rs -A 3Repository: RustPython/RustPython
Length of output: 351
🌐 Web query:
CPython ParamSpec default behavior caching evaluate_default
💡 Result:
Short answer: ParamSpec (and other TypeVar-like) defaults are evaluated lazily and the evaluated value is cached — CPython uses an evaluate_default mechanism so the default expression is only run when the default is first needed, and that result is reused. Historically there were edge cases (e.g., defaults being evaluated during generic class creation or before forward references resolved); PEP 696/implementation discussion and recent CPython work (notably “evaluate_default” on typing.ParamSpec / TypeVarTuple) address and refine that behavior so defaults are truly lazily evaluated and cached. [1][2][3][4]
Sources
- PEP 696 (type defaults for TypeVar-like objects: semantics include lazy defaults / default rules). [1]
- CPython what's new / changelog noting “evaluate_default” on typing.ParamSpec and typing.TypeVarTuple (implementation change). [2]
- Implementation/discussion reporting earlier bug where defaults could be evaluated during class creation and explaining lazy/caching behavior. [3]
- PEP 649 (deferred evaluation of annotations) — relevant background on delayed evaluation and caching semantics. [4]
🌐 Web query:
CPython typing.ParamSpec __default__ implementation
💡 Result:
Short answer: CPython implements ParamSpec.default as part of PEP 696 (type-parameter defaults) — ParamSpec objects expose a lazily-evaluated default attribute (a sentinel typing.NoDefault when no default was supplied, and None when default=None was explicitly passed). The runtime also adds has_default() and an evaluate_default() helper; typing_extensions mirrors this behavior for older Pythons. (peps.python.org)
Citations:
Fix ParamSpec.default to cache evaluated defaults like TypeVar and TypeVarTuple.
Currently, ParamSpec::__default__ recomputes evaluate_default on every call and doesn't persist the value, unlike TypeVar and TypeVarTuple. This diverges from CPython behavior (per PEP 696), which caches the evaluated result. Update the implementation to mutate self.default_value after evaluation (wrap in lock() and use *default_value = evaluate_default.call(...)?;).
🤖 Prompt for AI Agents
In `@crates/vm/src/stdlib/typevar.rs` around lines 423 - 530,
ParamSpec::__default__ currently evaluates self.evaluate_default on every call
but doesn't cache the result; modify the method so that after calling the
evaluate_default callable you store the result into self.default_value (acquire
the same lock as used for evaluate_default, e.g., let mut default_value =
self.default_value.lock(); *default_value = evaluated_result.clone(); ) and then
return that cached value; ensure the initial NoDefault check still uses
vm.ctx.typing_no_default and that you return the cached default on subsequent
calls instead of re-invoking evaluate_default.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.