Fix annotation scope, deadlock, MRO, and HEAPTYPE issues#7087
Conversation
Annotation scope: - Remove module-level annotation re-scan that created phantom sub_tables, breaking annotation closure for comprehensions - Add async comprehension check in symbol table with is_in_async_context(); annotation/type-params scopes are always non-async - Save/restore CompileContext in enter/exit_annotation_scope to reset in_async_scope Deadlock prevention: - Fix TypeVar/ParamSpec/TypeVarTuple __default__ and evaluate_default by cloning lock contents before acquiring a second lock or calling Python Other fixes: - Add HEAPTYPE flag to Generic for correct pickle behavior - Guard heaptype_ext access in name_inner/set___name__/ set___qualname__ with safe checks instead of unwrap - Fix MRO error message to include base class names - Add "format" to varnames in TypeAlias annotation scopes - Fix single-element tuple repr to include trailing comma
📝 WalkthroughWalkthroughThis PR refactors annotation scope management in the compiler to explicitly carry and restore CompileContext, strengthens type immutability error handling, improves TypeVar locking semantics, and adjusts tuple type representation formatting. Changes span compilation, type resolution, and stdlib implementations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
Sorry, something went wrong.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] lib: cpython/Lib/ctypes dependencies:
dependent tests: (1 tests)
[x] lib: cpython/Lib/dataclasses.py dependencies:
dependent tests: (6 tests)
[x] lib: cpython/Lib/typing.py dependencies:
dependent tests: (12 tests)
Legend:
|
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/vm/src/stdlib/typevar.rs (1)
95-122: ⚠️ Potential issue | 🟠 Major
__bound__and__constraints__still hold their locks while calling into Python — same deadlock risk you fixed in__default__.Lines 101 and 116 call
evaluate_constraints.call()/evaluate_bound.call()while theconstraints/boundmutex guard is alive. If the evaluator callback triggers re-entrant access to__constraints__or__bound__on the sameTypeVar, it will deadlock. Apply the same scoped-lock + clone-first pattern used in__default__.Proposed fix
#[pygetset] fn __constraints__(&self, vm: &VirtualMachine) -> PyResult { - let mut constraints = self.constraints.lock(); - if !vm.is_none(&constraints) { - return Ok(constraints.clone()); + { + let constraints = self.constraints.lock(); + if !vm.is_none(&constraints) { + return Ok(constraints.clone()); + } } let r = if !vm.is_none(&self.evaluate_constraints) { - *constraints = self.evaluate_constraints.call((1i32,), vm)?; - constraints.clone() + let result = self.evaluate_constraints.call((1i32,), vm)?; + *self.constraints.lock() = result.clone(); + result } else { vm.ctx.empty_tuple.clone().into() }; Ok(r) } #[pygetset] fn __bound__(&self, vm: &VirtualMachine) -> PyResult { - let mut bound = self.bound.lock(); - if !vm.is_none(&bound) { - return Ok(bound.clone()); + { + let bound = self.bound.lock(); + if !vm.is_none(&bound) { + return Ok(bound.clone()); + } } let r = if !vm.is_none(&self.evaluate_bound) { - *bound = self.evaluate_bound.call((1i32,), vm)?; - bound.clone() + let result = self.evaluate_bound.call((1i32,), vm)?; + *self.bound.lock() = result.clone(); + result } else { vm.ctx.none() }; Ok(r) }
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 2494-2497: The code currently inserts "format" into
current_code_info().metadata.varnames and later assumes its local slot is 0;
update emit_format_validation() (and the similar spots at the other occurrences)
to resolve the local slot via get_local_var_index("format") instead of
hardcoding 0: use get_local_var_index("format") to obtain the parameter index
and emit loads/validation against that index, leaving the varnames insertion
as-is but making the load robust to symbol ordering changes.
🧹 Nitpick comments (2)
crates/vm/src/builtins/type.rs (1)
850-877: Inconsistent name formatting in heaptype_ext error vs. sibling setter.Line 865 uses
self.name()(which strips the module prefix viarsplit_once('.')), while the analogous guard inset___name__at line 1162 usesself.slot_name()(which preserves the full dotted name). Thecheck_set_special_type_attrhelper at line 1138 also usesslot_name(). For static types whereheaptype_extisNone, these yield different output (e.g."typing.TypeAliasType"vs"TypeAliasType").Consider using
self.slot_name()here for consistency:Suggested fix
let heap_type = self.heaptype_ext.as_ref().ok_or_else(|| { vm.new_type_error(format!( "cannot set '__qualname__' attribute of immutable type '{}'", - self.name() + self.slot_name() )) })?;crates/vm/src/stdlib/typevar.rs (1)
182-192: Consider extracting the duplicated__default__/evaluate_defaultlogic into shared helpers.The
__default__body (lines 141–155, 506–520, 704–718) andevaluate_defaultbody (lines 183–192, 524–533, 722–731) are identical acrossTypeVar,ParamSpec, andTypeVarTuple. A free function taking(&PyMutex<PyObjectRef>, &PyMutex<PyObjectRef>, &VirtualMachine)would eliminate the triplication and ensure future lock-pattern fixes only need to be applied once.As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."
Also applies to: 523-533, 721-731
Sorry, something went wrong.
426019e
into
RustPython:main
Feb 11, 2026
…7087) * Fix annotation scope, deadlock, MRO, and HEAPTYPE issues Annotation scope: - Remove module-level annotation re-scan that created phantom sub_tables, breaking annotation closure for comprehensions - Add async comprehension check in symbol table with is_in_async_context(); annotation/type-params scopes are always non-async - Save/restore CompileContext in enter/exit_annotation_scope to reset in_async_scope Deadlock prevention: - Fix TypeVar/ParamSpec/TypeVarTuple __default__ and evaluate_default by cloning lock contents before acquiring a second lock or calling Python Other fixes: - Add HEAPTYPE flag to Generic for correct pickle behavior - Guard heaptype_ext access in name_inner/set___name__/ set___qualname__ with safe checks instead of unwrap - Fix MRO error message to include base class names - Add "format" to varnames in TypeAlias annotation scopes - Fix single-element tuple repr to include trailing comma * Unmark failing markers
Summary by CodeRabbit