From c78d811fc478ed098b4b5c9bd79556784a918be8 Mon Sep 17 00:00:00 2001 From: dot Date: Tue, 13 Oct 2026 15:05:06 +0900 Subject: [PATCH] Finish frozendict with borrowed PyAnyDict views Use the landed PyRef target model for a transparent dict/frozendict header view, retaining objects explicitly with PyRef. Keep targets sealed and preserve mutable-only entry points. Borrow frame globals from the object header rather than movable reference wrappers. Unify exact-TypeError key diagnostics with final CPython semantics, including callback failure propagation and no explicit cause. Keep the Python regressions in builtin_dict and test reference layout/ownership in Rust. Preserve existing canonical library updates. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/capi/src/ceval.rs | 10 +- crates/capi/src/dictobject.rs | 12 +- crates/vm/src/builtins/dict.rs | 272 +++++++++++++++------- crates/vm/src/builtins/frame.rs | 4 +- crates/vm/src/builtins/function.rs | 6 +- crates/vm/src/builtins/mappingproxy.rs | 2 +- crates/vm/src/builtins/mod.rs | 2 +- crates/vm/src/builtins/type.rs | 12 +- crates/vm/src/frame.rs | 30 +-- crates/vm/src/function/protocol.rs | 6 +- crates/vm/src/object/core.rs | 6 + crates/vm/src/scope.rs | 8 +- crates/vm/src/stdlib/_testinternalcapi.rs | 23 +- crates/vm/src/stdlib/builtins.rs | 6 +- crates/vm/src/vm/mod.rs | 4 +- extra_tests/snippets/builtin_dict.py | 103 ++++++++ src/shell/helper.rs | 8 +- 17 files changed, 368 insertions(+), 146 deletions(-) diff --git a/crates/capi/src/ceval.rs b/crates/capi/src/ceval.rs index f571ea0db..036c25945 100644 --- a/crates/capi/src/ceval.rs +++ b/crates/capi/src/ceval.rs @@ -3,10 +3,10 @@ use crate::pystate::with_vm; use crate::unicodeobject::decode_fsdefault_and_size; use crate::util::{CStrExt, FfiPtrExt}; use core::ffi::{CStr, c_char, c_int}; -use rustpython_vm::builtins::{PyAnyDictRef, PyCode}; +use rustpython_vm::builtins::{PyAnyDict, PyCode}; use rustpython_vm::function::ArgMapping; use rustpython_vm::scope::Scope; -use rustpython_vm::{AsObject, PyObject, TryFromObject}; +use rustpython_vm::{AsObject, PyObject, PyRef, TryFromObject}; use rustpython_vm::{PyObjectRef, version}; #[unsafe(no_mangle)] @@ -32,8 +32,10 @@ pub unsafe extern "C" fn PyEval_EvalCode( ) -> *mut PyObject { with_vm(|vm| { let code = unsafe { co.assume_borrowed_and_cast::(vm) }?; - let globals = - PyAnyDictRef::try_from_object(vm, unsafe { globals.assume_borrowed() }.to_owned())?; + let globals = PyRef::::try_from_object( + vm, + unsafe { globals.assume_borrowed() }.to_owned(), + )?; let locals = unsafe { locals.assume_borrowed_or_opt() } .map(|obj| ArgMapping::try_from_object(vm, obj.to_owned())) .transpose()?; diff --git a/crates/capi/src/dictobject.rs b/crates/capi/src/dictobject.rs index 3ca9304e3..1e6fab1b1 100644 --- a/crates/capi/src/dictobject.rs +++ b/crates/capi/src/dictobject.rs @@ -4,7 +4,7 @@ use crate::pystate::with_vm; use crate::util::{CStrExt, FfiPtrExt}; use core::ffi::{c_char, c_int}; use core::ptr::NonNull; -use rustpython_vm::builtins::{PyAnyDictRef, PyDict, PyFrozenDict}; +use rustpython_vm::builtins::{PyAnyDict, PyDict, PyFrozenDict}; use rustpython_vm::{AsObject, Py, PyPayload, PyResult, VirtualMachine}; define_py_check!(fn PyDict_Check, types.dict_type); @@ -13,8 +13,8 @@ define_py_check!(fn PyDictKeys_Check, types.dict_keys_type); define_py_check!(fn PyDictValues_Check, types.dict_values_type); define_py_check!(fn PyDictItems_Check, types.dict_items_type); -fn any_dict(dict: &PyObject, vm: &VirtualMachine) -> PyResult { - PyAnyDictRef::from_object(dict) +fn any_dict<'a>(dict: &'a PyObject, vm: &VirtualMachine) -> PyResult<&'a PyAnyDict> { + PyAnyDict::from_object(dict) .ok_or_else(|| vm.new_system_error("bad argument to internal function")) } @@ -79,7 +79,7 @@ pub unsafe extern "C" fn PyDict_SetItemString( #[unsafe(no_mangle)] pub unsafe extern "C" fn PyDict_GetItem(dict: *mut PyObject, key: *mut PyObject) -> *mut PyObject { with_vm(|vm| { - let Some(dict) = PyAnyDictRef::from_object(unsafe { dict.assume_borrowed() }) else { + let Some(dict) = PyAnyDict::from_object(unsafe { dict.assume_borrowed() }) else { return core::ptr::null_mut(); }; let key = unsafe { key.assume_borrowed() }; @@ -117,7 +117,7 @@ pub unsafe extern "C" fn PyDict_GetItemString( let Ok(key) = (unsafe { key.try_as_str(vm) }) else { return core::ptr::null_mut(); }; - let Some(dict) = PyAnyDictRef::from_object(unsafe { dict.assume_borrowed() }) else { + let Some(dict) = PyAnyDict::from_object(unsafe { dict.assume_borrowed() }) else { return core::ptr::null_mut(); }; @@ -347,7 +347,7 @@ pub unsafe extern "C" fn PyDict_Next( value: *mut *mut PyObject, ) -> c_int { with_vm(|_vm| { - let Some(dict) = PyAnyDictRef::from_object(unsafe { dict.assume_borrowed() }) else { + let Some(dict) = PyAnyDict::from_object(unsafe { dict.assume_borrowed() }) else { return false; }; let index = unsafe { *pos } as usize; diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index a483812d6..1d376c72e 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -41,9 +41,17 @@ pub(crate) fn key_hash( vm: &VirtualMachine, ) -> PyResult { match key.key_hash(vm) { + // Exact TypeError only: exception subclasses retain their identity. Err(exc) if exc.class().is(vm.ctx.exceptions.type_error) => { let name = key.to_pyobject(vm).class().fully_qualified_name(vm)?; - let message = exc.as_object().str(vm)?; + // Match BaseException_str without the diagnostic formatter's + // fallback: argument __str__/__repr__ failures must propagate. + let args = exc.args(); + let message = match args.as_slice() { + [] => vm.ctx.empty_str.to_owned(), + [arg] => arg.str(vm)?, + _ => args.as_object().str(vm)?, + }; Err(vm.new_type_error(format!( "cannot use '{name}' as a {mapping_name} key ({message})" ))) @@ -61,71 +69,108 @@ pub struct PyDict { } pub type PyDictRef = PyRef; -/// A checked reference to a dict or frozendict, preserving the object's mutability. -/// Its single-pointer layout also supports readonly object members such as __globals__. +/// A borrowed view of a dict or frozendict, including subclasses. +/// +/// This is an erased header view, not a shared payload layout. Every payload +/// access downcasts to the actual alternative, preserving its mutability. #[repr(transparent)] -#[derive(Debug, Clone)] -pub struct PyAnyDictRef(PyObjectRef); - -impl super::descriptor::MemberLayout for PyAnyDictRef { - const KIND: super::descriptor::MemberKind = super::descriptor::MemberKind::Object; +pub struct PyAnyDict { + object: PyObject, } -unsafe impl Traverse for PyAnyDictRef { +// SAFETY: this owner contributes exactly one strong reference to the object. +unsafe impl Traverse for PyRef { fn traverse(&self, tracer: &mut TraverseFn<'_>) { - self.0.traverse(tracer); + tracer(self.as_object()); } } -impl Borrow for PyAnyDictRef { - fn borrow(&self) -> &PyObject { - &self.0 - } -} - -impl From for PyAnyDictRef { - fn from(dict: PyDictRef) -> Self { - Self(dict.into()) - } -} -impl From> for PyAnyDictRef { - fn from(dict: PyRef) -> Self { - Self(dict.into()) - } -} -impl From for PyObjectRef { - fn from(dict: PyAnyDictRef) -> Self { - dict.0 - } -} -impl crate::convert::ToPyObject for PyAnyDictRef { +impl crate::convert::ToPyObject for PyRef { fn to_pyobject(self, _vm: &VirtualMachine) -> PyObjectRef { - self.into() + self.into_object() } } -impl TryFromObject for PyAnyDictRef { - fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { - Self::from_object(&obj).ok_or_else(|| { - vm.new_type_error(format!( + +impl Borrow for PyAnyDict { + fn borrow(&self) -> &PyObject { + &self.object + } +} + +impl Borrow for PyRef { + fn borrow(&self) -> &PyAnyDict { + self + } +} + +impl ToOwned for PyAnyDict { + type Owned = PyRef; + + fn to_owned(&self) -> PyRef { + // SAFETY: this borrowed view already guarantees a dictionary payload. + unsafe { PyRef::::from_object_unchecked(self.object.to_owned()) } + } +} + +impl From for PyRef { + fn from(dict: PyDictRef) -> Self { + // SAFETY: the typed owner guarantees the dict alternative. + unsafe { Self::from_object_unchecked(dict.into()) } + } +} + +impl From> for PyRef { + fn from(dict: PyRef) -> Self { + // SAFETY: the typed owner guarantees the frozendict alternative. + unsafe { Self::from_object_unchecked(dict.into()) } + } +} + +impl From> for PyObjectRef { + fn from(dict: PyRef) -> Self { + dict.into_object() + } +} + +// SAFETY: the transparent PyRef owner has the same layout as PyObjectRef. +// A successful check proves the actual dict or frozendict payload invariant. +unsafe impl crate::convert::TransmuteFromObject for PyRef { + fn check(vm: &VirtualMachine, obj: &PyObject) -> PyResult<()> { + if PyAnyDict::from_object(obj).is_some() { + Ok(()) + } else { + Err(vm.new_type_error(format!( "expected dict or frozendict, got {}", obj.class().name() - )) - }) + ))) + } } } -impl PyAnyDictRef { - pub fn from_object(obj: &PyObject) -> Option { - (obj.downcastable::() || obj.downcastable::()) - .then(|| Self(obj.to_owned())) +impl TryFromObject for PyRef { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + ::check(vm, &obj)?; + // SAFETY: the shared checker proved the union view's payload invariant. + Ok(unsafe { Self::from_object_unchecked(obj) }) + } +} + +impl PyAnyDict { + pub fn from_object(obj: &PyObject) -> Option<&Self> { + if obj.downcast_ref::().is_some() || obj.downcast_ref::().is_some() { + // SAFETY: PyAnyDict is transparent over PyObject and the payload was checked. + Some(unsafe { &*(obj as *const PyObject).cast::() }) + } else { + None + } } pub(crate) fn as_mutable(&self) -> Option<&Py> { - self.0.downcast_ref() + self.object.downcast_ref() } pub(crate) fn is_frozen(&self) -> bool { - self.0.downcastable::() + self.object.downcastable::() } // The raw payload is private: callers outside the VM cannot use it to mutate a frozendict. @@ -133,7 +178,7 @@ impl PyAnyDictRef { if let Some(dict) = self.as_mutable() { dict } else { - &self.0.downcast_ref::().unwrap().dict + &self.object.downcast_ref::().unwrap().dict } } @@ -187,7 +232,7 @@ impl PyAnyDictRef { if let Some(dict) = self.as_mutable() { dict.inner_getitem_opt(key, vm) } else { - self.0 + self.object .downcast_ref::() .unwrap() .inner_getitem_opt(key, vm) @@ -201,10 +246,10 @@ impl PyAnyDictRef { if let Some(dict) = self.as_mutable() { return dict.get_item_opt(key, vm); } - if self.0.class().is(vm.ctx.types.frozendict_type) { + if self.object.class().is(vm.ctx.types.frozendict_type) { return self.inner_getitem_opt(key, vm); } - match self.0.get_item(key, vm) { + match self.object.get_item(key, vm) { Ok(value) => Ok(Some(value)), Err(exc) if exc.fast_isinstance(vm.ctx.exceptions.key_error) => Ok(None), Err(exc) => Err(exc), @@ -214,7 +259,7 @@ impl PyAnyDictRef { if let Some(dict) = self.as_mutable() { dict.get_item(key, vm) } else { - self.0.get_item(key, vm) + self.object.get_item(key, vm) } } pub fn contains_key(&self, key: &K, vm: &VirtualMachine) -> bool { @@ -240,7 +285,7 @@ impl PyAnyDictRef { } } -impl<'a> IntoIterator for &'a PyAnyDictRef { +impl<'a> IntoIterator for &'a PyAnyDict { type Item = (PyObjectRef, PyObjectRef); type IntoIter = DictIter<'a>; fn into_iter(self) -> Self::IntoIter { @@ -354,7 +399,7 @@ impl PyDict { override_existing: bool, vm: &VirtualMachine, ) -> PyResult<()> { - if let Some(other_dict) = PyAnyDictRef::from_object(&other) + if let Some(other_dict) = PyAnyDict::from_object(&other) && other_dict.uses_builtin_iter(vm) { return self.merge_dict(other_dict.as_dict(), override_existing, vm); @@ -520,22 +565,7 @@ impl PyDict { /// Hashing up front also means a `__hash__` that fails only intermittently /// is still reported. fn hash_or_unhashable(key: &K, vm: &VirtualMachine) -> PyResult { - match key.key_hash(vm) { - Ok(hash) => Ok(hash), - // Exact `TypeError` only: a `__hash__` raising a *subclass* of - // `TypeError` (e.g. a user `MyTypeError`) must propagate unchanged, - // matching CPython's exact-type check. - Err(cause) if cause.class().is(vm.ctx.exceptions.type_error) => { - let message = cause.as_object().str(vm)?; - let key = key.to_pyobject(vm); - let name = key.class().fully_qualified_name(vm)?; - let err = - vm.new_type_error(format!("cannot use '{name}' as a dict key ({message})")); - err.set_cause(Some(cause)); - Err(err) - } - Err(other) => Err(other), - } + key_hash(key, "dict", vm) } /// Set item variant which can be called with multiple @@ -945,7 +975,7 @@ impl Comparable for PyDict { op: PyComparisonOp, vm: &VirtualMachine, ) -> PyResult { - op.eq_only(|| match PyAnyDictRef::from_object(other) { + op.eq_only(|| match PyAnyDict::from_object(other) { Some(other) => { zelf.payload .compare_entries(other.as_dict(), PyComparisonOp::Eq, true, vm) @@ -1411,7 +1441,7 @@ impl PyFrozenDict { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - if let Some(other_dict) = PyAnyDictRef::from_object(&other) + if let Some(other_dict) = PyAnyDict::from_object(&other) && other_dict.uses_builtin_iter(vm) { return dict.merge_dict(other_dict.as_dict(), true, vm); @@ -1436,8 +1466,7 @@ impl PyFrozenDict { } pub(crate) fn union(a: &PyObject, b: &PyObject, vm: &VirtualMachine) -> PyResult { - let (Some(left), Some(right)) = - (PyAnyDictRef::from_object(a), PyAnyDictRef::from_object(b)) + let (Some(left), Some(right)) = (PyAnyDict::from_object(a), PyAnyDict::from_object(b)) else { return Ok(vm.ctx.not_implemented()); }; @@ -1649,7 +1678,7 @@ impl Comparable for PyFrozenDict { op: PyComparisonOp, vm: &VirtualMachine, ) -> PyResult { - op.eq_only(|| match PyAnyDictRef::from_object(other) { + op.eq_only(|| match PyAnyDict::from_object(other) { Some(other) => zelf .dict .compare_entries(other.as_dict(), PyComparisonOp::Eq, true, vm), @@ -1803,7 +1832,7 @@ impl ExactSizeIterator for DictIter<'_> { trait DictView: PyPayload + PyClassDef + Iterable + Representable { type ReverseIter: PyPayload + core::fmt::Debug; - fn dict(&self) -> &PyAnyDictRef; + fn dict(&self) -> &PyAnyDict; fn item(vm: &VirtualMachine, key: PyObjectRef, value: PyObjectRef) -> PyObjectRef; fn __len__(&self) -> usize { @@ -1832,11 +1861,11 @@ macro_rules! dict_view { #[pyclass(module = false, name = $class_name, unhashable = $unhashable, traverse)] #[derive(Debug)] pub(crate) struct $name { - pub(crate) dict: PyAnyDictRef, + pub(crate) dict: PyRef, } impl $name { - pub(crate) fn new(dict: PyAnyDictRef) -> Self { + pub(crate) fn new(dict: PyRef) -> Self { $name { dict } } } @@ -1844,7 +1873,7 @@ macro_rules! dict_view { impl DictView for $name { type ReverseIter = $reverse_iter_name; - fn dict(&self) -> &PyAnyDictRef { + fn dict(&self) -> &PyAnyDict { &self.dict } @@ -1907,7 +1936,7 @@ macro_rules! dict_view { /// from then on. #[pytraverse(skip)] changed: PyAtomic, - pub(crate) internal: PyMutex>, + pub(crate) internal: PyMutex>>, } impl PyPayload for $iter_name { @@ -1918,7 +1947,7 @@ macro_rules! dict_view { } impl $iter_name { - pub(crate) fn new(dict: PyAnyDictRef) -> Self { + pub(crate) fn new(dict: PyRef) -> Self { $iter_name { size: dict.size(), changed: Radium::new(false), @@ -2014,7 +2043,7 @@ macro_rules! dict_view { changed: PyAtomic, #[pytraverse(skip)] remaining: PyAtomic, - internal: PyMutex>, + internal: PyMutex>>, } impl PyPayload for $reverse_iter_name { @@ -2025,7 +2054,7 @@ macro_rules! dict_view { } impl $reverse_iter_name { - pub(crate) fn new(dict: PyAnyDictRef) -> Self { + pub(crate) fn new(dict: PyRef) -> Self { let size = dict.size(); // usize::MAX represents the position before the first entry. let position = size.entries_size.wrapping_sub(1); @@ -2261,7 +2290,7 @@ impl PyDictKeys { #[pygetset] fn mapping(zelf: PyRef) -> PyMappingProxy { - PyMappingProxy::from_any_dict(zelf.dict().clone()) + PyMappingProxy::from_any_dict(zelf.dict().to_owned()) } } @@ -2324,7 +2353,7 @@ impl PyDictItems { } #[pygetset] fn mapping(zelf: PyRef) -> PyMappingProxy { - PyMappingProxy::from_any_dict(zelf.dict().clone()) + PyMappingProxy::from_any_dict(zelf.dict().to_owned()) } } @@ -2386,7 +2415,7 @@ impl AsNumber for PyDictItems { impl PyDictValues { #[pygetset] fn mapping(zelf: PyRef) -> PyMappingProxy { - PyMappingProxy::from_any_dict(zelf.dict().clone()) + PyMappingProxy::from_any_dict(zelf.dict().to_owned()) } } @@ -2607,3 +2636,78 @@ pub(crate) fn init(context: &'static Context) { .vectorcall .store(Some(vectorcall_frozendict)); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn anydict_reference_layout_and_ownership() { + static_assertions::assert_not_impl_any!(PyAnyDict: PyPayload); + static_assertions::assert_type_eq_all!( + ::Target, + PyAnyDict + ); + static_assertions::assert_eq_size!(PyRef, PyObjectRef, Option>); + static_assertions::assert_eq_align!(PyRef, PyObjectRef); + static_assertions::assert_eq_size!(PyAnyDict, PyObject); + static_assertions::assert_eq_align!(PyAnyDict, PyObject); + #[cfg(feature = "threading")] + static_assertions::assert_impl_all!(PyRef: Send, Sync); + #[cfg(not(feature = "threading"))] + static_assertions::assert_not_impl_any!(PyRef: Send, Sync); + + let ctx = Context::genesis(); + let dict: PyRef = PyDict::default().into_ref(ctx).into(); + let frozen: PyRef = PyFrozenDict::from_dict(PyDict::default()) + .into_ref(ctx) + .into(); + for owner in [dict, frozen] { + let object = owner.as_object() as *const PyObject; + assert_eq!(owner.as_object().strong_count(), 1); + let borrowed = PyAnyDict::from_object(owner.as_object()).unwrap(); + assert!(core::ptr::eq(borrowed.as_object(), object)); + assert_eq!(owner.as_object().strong_count(), 1); + assert_eq!(borrowed.as_mutable().is_none(), borrowed.is_frozen()); + let copy = borrowed.to_owned(); + assert_eq!(owner.as_object().strong_count(), 2); + let clone = owner.clone(); + assert_eq!(owner.as_object().strong_count(), 3); + let erased = clone.into_object(); + assert_eq!(erased.strong_count(), 3); + assert!(core::ptr::eq(&*erased, object)); + let mut visited = 0; + owner.traverse(&mut |edge| { + assert!(core::ptr::eq(edge, object)); + visited += 1; + }); + assert_eq!(visited, 1); + drop(copy); + drop(owner); + assert_eq!(erased.strong_count(), 1); + } + } + + #[test] + fn anydict_checked_reference_conversion() { + crate::Interpreter::without_stdlib(Default::default()).enter(|vm| { + let dict = PyDict::default().into_pyobject(vm); + let frozen = PyFrozenDict::from_dict(PyDict::default()).into_pyobject(vm); + for object in [dict, frozen] { + let owner = PyRef::::try_from_object(vm, object.clone()).unwrap(); + assert_eq!(object.strong_count(), 2); + assert!(core::ptr::eq(owner.as_object(), &*object)); + let erased: PyObjectRef = owner.into(); + assert_eq!(object.strong_count(), 2); + assert!(core::ptr::eq(&*erased, &*object)); + drop(erased); + assert_eq!(object.strong_count(), 1); + } + let unrelated: PyObjectRef = vm.ctx.new_int(1000).into(); + assert!(PyAnyDict::from_object(&unrelated).is_none()); + let error = PyRef::::try_from_object(vm, unrelated.clone()).unwrap_err(); + assert!(error.fast_isinstance(vm.ctx.exceptions.type_error)); + assert_eq!(unrelated.strong_count(), 1); + }); + } +} diff --git a/crates/vm/src/builtins/frame.rs b/crates/vm/src/builtins/frame.rs index 49e1d4ad5..be8ddd9f3 100644 --- a/crates/vm/src/builtins/frame.rs +++ b/crates/vm/src/builtins/frame.rs @@ -2,7 +2,7 @@ */ -use super::{PyAnyDictRef, PyAsyncGen, PyCode, PyCoroutine, PyGenerator, PyIntRef}; +use super::{PyAnyDict, PyAsyncGen, PyCode, PyCoroutine, PyGenerator, PyIntRef}; use crate::{ AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, @@ -668,7 +668,7 @@ impl Py { } #[pygetset] - pub fn f_globals(&self) -> PyAnyDictRef { + pub fn f_globals(&self) -> PyRef { self.iframe().globals().to_owned() } diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 59d443124..40e51c6b3 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -2,7 +2,7 @@ mod jit; use super::{ - PyAnyDictRef, PyAsyncGen, PyCode, PyCoroutine, PyDictRef, PyGenerator, PyList, PyModule, PyStr, + PyAnyDict, PyAsyncGen, PyCode, PyCoroutine, PyDictRef, PyGenerator, PyList, PyModule, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, object, }; use crate::common::hash::PyHash; @@ -66,7 +66,7 @@ fn format_missing_args( pub struct PyFunction { pub(crate) code: PyAtomicRef, #[pymember(name = "__globals__")] - pub(crate) globals: PyAnyDictRef, + pub(crate) globals: PyRef, #[pymember(name = "__builtins__")] pub(crate) builtins: PyObjectRef, #[pymember(name = "__closure__")] @@ -179,7 +179,7 @@ impl PyFunction { #[inline] pub(crate) fn new( code: PyRef, - globals: PyAnyDictRef, + globals: PyRef, vm: &VirtualMachine, ) -> PyResult { let name = PyMutex::new(code.obj_name.to_owned()); diff --git a/crates/vm/src/builtins/mappingproxy.rs b/crates/vm/src/builtins/mappingproxy.rs index 0c302de82..e1e0283d1 100644 --- a/crates/vm/src/builtins/mappingproxy.rs +++ b/crates/vm/src/builtins/mappingproxy.rs @@ -68,7 +68,7 @@ impl Constructor for PyMappingProxy { } impl PyMappingProxy { - pub(crate) fn from_any_dict(dict: super::PyAnyDictRef) -> Self { + pub(crate) fn from_any_dict(dict: PyRef) -> Self { Self { mapping: MappingProxyInner::Mapping(ArgMapping::new(dict.into())), } diff --git a/crates/vm/src/builtins/mod.rs b/crates/vm/src/builtins/mod.rs index a31ba4ffa..438cd9e50 100644 --- a/crates/vm/src/builtins/mod.rs +++ b/crates/vm/src/builtins/mod.rs @@ -20,7 +20,7 @@ pub use complex::PyComplex; pub(crate) mod coroutine; pub use coroutine::PyCoroutine; pub(crate) mod dict; -pub use dict::{PyAnyDictRef, PyDict, PyDictRef, PyFrozenDict}; +pub use dict::{PyAnyDict, PyDict, PyDictRef, PyFrozenDict}; pub(crate) mod enumerate; pub use enumerate::PyEnumerate; pub(crate) mod filter; diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index be75baf55..153842056 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -1,5 +1,5 @@ use super::{ - PyAnyDictRef, PyClassMethod, PyDict, PyDictRef, PyList, PyStaticMethod, PyStr, PyStrInterned, + PyAnyDict, PyClassMethod, PyDict, PyDictRef, PyList, PyStaticMethod, PyStr, PyStrInterned, PyStrRef, PyTupleRef, PyUtf8StrRef, PyWeak, mappingproxy::PyMappingProxy, object, union_, }; use crate::{ @@ -1798,8 +1798,14 @@ impl Constructor for PyType { })); } - let (name, bases, dict, kwargs): (PyStrRef, PyTupleRef, PyAnyDictRef, KwArgs) = + let (name, bases, dict, kwargs): (PyStrRef, PyTupleRef, PyObjectRef, KwArgs) = args.clone().bind_for(vm, Self::NAME)?; + let Some(dict) = PyAnyDict::from_object(&dict) else { + return Err(vm.new_type_error(format!( + "type.__new__() argument 3 must be dict or frozendict, not {}", + dict.class().fully_qualified_name(vm)? + ))); + }; let (metatype, base, bases, base_is_type) = if bases.as_slice().is_empty() { let base = vm.ctx.types.object_type.to_owned(); @@ -1849,7 +1855,7 @@ impl Constructor for PyType { let copied = vm.ctx.new_dict(); // Like dict.copy(), an empty table bypasses subclass hooks. if !dict.is_empty() { - copied.merge_object(dict.into(), vm)?; + copied.merge_object(dict.to_owned().into(), vm)?; } copied }; diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index c7e693710..a043838d4 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -6,10 +6,10 @@ use crate::{ AsObject, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, PyStackRef, TryFromObject, VirtualMachine, builtins::{ - PyAnyDictRef, PyBaseException, PyBaseExceptionRef, PyBaseObject, PyBytes, PyCode, - PyCoroutine, PyDict, PyDictRef, PyFloat, PyFrozenSet, PyGenerator, PyInt, PyInterpolation, - PyList, PyModule, PyProperty, PySet, PySlice, PyStr, PyStrInterned, PyTemplate, - PyTraceback, PyType, PyUtf8Str, + PyAnyDict, PyBaseException, PyBaseExceptionRef, PyBaseObject, PyBytes, PyCode, PyCoroutine, + PyDict, PyDictRef, PyFloat, PyFrozenSet, PyGenerator, PyInt, PyInterpolation, PyList, + PyModule, PyProperty, PySet, PySlice, PyStr, PyStrInterned, PyTemplate, PyTraceback, + PyType, PyUtf8Str, builtin_func::PyNativeFunction, descriptor::{PyMemberDescriptor, PyMethodDescriptor}, frame::stack_analysis, @@ -108,7 +108,7 @@ pub fn current_thread_frame_materialize(vm: &VirtualMachine) -> Option Option { +pub fn current_globals() -> Option> { let ptr = crate::vm::thread::get_current_frame(); if ptr.is_null() { return None; @@ -923,7 +923,7 @@ pub struct InterpreterFrame { // Borrowed pointers — owned by FrameObject or by PyFunction on caller's stack. pub(crate) code: *const Py, pub(crate) func_obj: *const PyObject, // nullable - pub(crate) globals: *const PyAnyDictRef, + pub(crate) globals: *const PyAnyDict, pub(crate) builtins: *const PyObject, /// Unified storage for local variables and evaluation stack. @@ -983,7 +983,7 @@ impl InterpreterFrame { #[inline(always)] pub(crate) unsafe fn new( code: &Py, - globals: &PyAnyDictRef, + globals: &PyAnyDict, builtins: &PyObject, func_obj: Option<&PyObject>, localsplus: LocalsPlus, @@ -1027,7 +1027,7 @@ impl InterpreterFrame { Some(obj) => obj as *const PyObject, None => core::ptr::null(), }, - globals: globals as *const PyAnyDictRef, + globals: globals as *const PyAnyDict, builtins: builtins as *const PyObject, localsplus, locals, @@ -1060,7 +1060,7 @@ impl InterpreterFrame { #[inline(always)] pub(crate) unsafe fn new_on_datastack<'a>( code: &Py, - globals: &PyAnyDictRef, + globals: &PyAnyDict, builtins: &PyObject, func_obj: Option<&PyObject>, locals: FrameLocals, @@ -1266,7 +1266,7 @@ impl InterpreterFrame { // Create a full FrameObject with its own InterpreterFrame copy. // The FrameObject owns references to the same objects (code, globals, etc). let code: PyRef = self.code().to_owned(); - let globals = self.globals().clone(); + let globals = self.globals().to_owned(); let builtins: PyObjectRef = self.builtins().to_owned(); let func_obj: Option = self.func_obj().map(|o| o.to_owned()); @@ -1362,7 +1362,7 @@ impl InterpreterFrame { #[cold] fn materialize_slow_chain(&self, vm: &VirtualMachine) -> FrameObjectRef { let code: PyRef = self.code().to_owned(); - let globals = self.globals().clone(); + let globals = self.globals().to_owned(); let builtins: PyObjectRef = self.builtins().to_owned(); let func_obj: Option = self.func_obj().map(|o| o.to_owned()); @@ -1419,7 +1419,7 @@ impl InterpreterFrame { /// Borrowed globals dict. #[inline(always)] - pub fn globals(&self) -> &PyAnyDictRef { + pub fn globals(&self) -> &PyAnyDict { // SAFETY: established by `new` / `init_iframe_ptrs`. unsafe { &*self.globals } } @@ -1534,7 +1534,7 @@ pub struct FrameObject { // raw pointers. Wrapped in Option so Traverse::clear can release them, // allowing GC cycle collection to reclaim referenced objects. pub(crate) owned_code: Option>, - pub(crate) owned_globals: Option, + pub(crate) owned_globals: Option>, pub(crate) owned_builtins: Option, pub(crate) owned_func_obj: Option, @@ -1853,7 +1853,7 @@ impl FrameObject { fn init_iframe_ptrs(self_: &Py) { let iframe = unsafe { self_.iframe_mut() }; iframe.code = &**self_.owned_code.as_ref().unwrap() as *const Py; - iframe.globals = self_.owned_globals.as_ref().unwrap() as *const PyAnyDictRef; + iframe.globals = &**self_.owned_globals.as_ref().unwrap() as *const PyAnyDict; iframe.builtins = &**self_.owned_builtins.as_ref().unwrap() as *const PyObject; iframe.func_obj = match &self_.owned_func_obj { Some(obj) => &**obj as *const PyObject, @@ -2931,7 +2931,7 @@ pub(crate) struct ExecutingFrame<'a> { code: &'a Py, localsplus: &'a mut LocalsPlus, locals: &'a FrameLocals, - globals: &'a PyAnyDictRef, + globals: &'a PyAnyDict, builtins: &'a PyObject, /// Cached downcast of builtins to PyDict for fast LOAD_GLOBAL. /// Only set when both globals and builtins are exact dict types (not diff --git a/crates/vm/src/function/protocol.rs b/crates/vm/src/function/protocol.rs index f2f6ad370..c9d8df5b1 100644 --- a/crates/vm/src/function/protocol.rs +++ b/crates/vm/src/function/protocol.rs @@ -1,7 +1,7 @@ use super::IntoFuncArgs; use crate::{ - AsObject, PyObject, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine, - builtins::{PyAnyDictRef, PyDictRef, iter::PySequenceIterator}, + AsObject, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine, + builtins::{PyAnyDict, PyDictRef, iter::PySequenceIterator}, convert::ToPyObject, object::{Traverse, TraverseFn}, protocol::{PyIter, PyIterIter, PyMapping}, @@ -157,7 +157,7 @@ impl ArgMapping { #[inline(always)] #[must_use] - pub fn from_anydict_exact(dict: PyAnyDictRef) -> Self { + pub fn from_anydict_exact(dict: PyRef) -> Self { Self { obj: dict.into() } } diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index d22d6ef74..d72a84af3 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -2666,6 +2666,7 @@ mod ref_target_sealed { impl Sealed for T {} impl Sealed for super::PyObject {} impl Sealed for crate::builtins::set::PyAnySet {} + impl Sealed for crate::builtins::PyAnyDict {} impl Sealed for super::PyTuple> {} } @@ -2709,6 +2710,11 @@ pub trait PyTypedRefTarget: PyRefTarget {} impl>> PyTypedRefTarget for T {} impl PyTypedRefTarget for crate::builtins::set::PyAnySet {} +impl PyTypedRefTarget for crate::builtins::PyAnyDict {} + +impl PyRefTarget for crate::builtins::PyAnyDict { + type Target = Self; +} impl PyRefTarget for crate::builtins::set::PyAnySet { type Target = Self; diff --git a/crates/vm/src/scope.rs b/crates/vm/src/scope.rs index ed67b6830..976cf7507 100644 --- a/crates/vm/src/scope.rs +++ b/crates/vm/src/scope.rs @@ -1,6 +1,6 @@ use crate::{ - VirtualMachine, - builtins::{PyAnyDictRef, PyDictRef}, + PyRef, VirtualMachine, + builtins::{PyAnyDict, PyDictRef}, function::ArgMapping, }; use alloc::fmt; @@ -8,7 +8,7 @@ use alloc::fmt; #[derive(Clone)] pub struct Scope { pub locals: Option, - pub globals: PyAnyDictRef, + pub globals: PyRef, } impl fmt::Debug for Scope { @@ -21,7 +21,7 @@ impl fmt::Debug for Scope { impl Scope { #[inline] #[must_use] - pub fn new(locals: Option, globals: impl Into) -> Self { + pub fn new(locals: Option, globals: impl Into>) -> Self { Self { locals, globals: globals.into(), diff --git a/crates/vm/src/stdlib/_testinternalcapi.rs b/crates/vm/src/stdlib/_testinternalcapi.rs index 24f790412..f378eed11 100644 --- a/crates/vm/src/stdlib/_testinternalcapi.rs +++ b/crates/vm/src/stdlib/_testinternalcapi.rs @@ -185,8 +185,8 @@ mod _testinternalcapi { &mut counts, args.globalnames.into_option(), args.attrnames.into_option(), - globalsns.as_ref(), - builtinsns.as_ref(), + globalsns.as_deref(), + builtinsns.as_deref(), vm, )?; counts_to_dict(&counts, vm) @@ -214,8 +214,8 @@ mod _testinternalcapi { &mut counts, args.globalnames.into_option(), None, - globalsns.as_ref(), - builtinsns.as_ref(), + globalsns.as_deref(), + builtinsns.as_deref(), vm, )?; if builtinsns.is_some() { @@ -1127,13 +1127,14 @@ fn expandtabs(input: &str, tab_size: usize) -> String { type CodeNamespaces = ( PyRef, - Option, - Option, + Option>, + Option>, ); fn code_or_function(obj: &PyObject, vm: &VirtualMachine) -> PyResult { if let Ok(func) = obj.to_owned().downcast::() { - let builtins = crate::builtins::PyAnyDictRef::from_object(&func.builtins); + let builtins = + crate::builtins::PyAnyDict::from_object(&func.builtins).map(ToOwned::to_owned); return Ok(( (*func.code).to_owned(), Some(func.globals.clone()), @@ -1287,8 +1288,8 @@ fn set_unbound_var_counts( counts: &mut VarCounts, globalnames: Option, attrnames: Option, - globalsns: Option<&crate::builtins::PyAnyDictRef>, - builtinsns: Option<&crate::builtins::PyAnyDictRef>, + globalsns: Option<&crate::builtins::PyAnyDict>, + builtinsns: Option<&crate::builtins::PyAnyDict>, vm: &VirtualMachine, ) -> PyResult<()> { let globalnames = optional_set(globalnames, "globalnames", vm)?; @@ -1324,8 +1325,8 @@ fn identify_unbound_names( code: &Py, globalnames: Option>, attrnames: Option>, - globalsns: Option<&crate::builtins::PyAnyDictRef>, - builtinsns: Option<&crate::builtins::PyAnyDictRef>, + globalsns: Option<&crate::builtins::PyAnyDict>, + builtinsns: Option<&crate::builtins::PyAnyDict>, vm: &VirtualMachine, ) -> PyResult<(UnboundCounts, i32)> { let mut seen_globals = HashSet::new(); diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index ab07eb634..1fcac52ab 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -11,7 +11,7 @@ mod builtins { AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine, builtins::{ - PyAnyDictRef, PyByteArray, PyBytes, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, + PyAnyDict, PyByteArray, PyBytes, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyUtf8StrRef, enumerate::PyReverseSequenceIterator, function::{PyCell, PyCellRef, PyFunction}, @@ -462,7 +462,7 @@ mod builtins { Some(globals) => { validate_globals_dict(&globals, vm, func_name)?; - let globals = PyAnyDictRef::try_from_object(vm, globals)?; + let globals = PyRef::::try_from_object(vm, globals)?; ( globals.clone(), self.locals @@ -754,7 +754,7 @@ mod builtins { } #[pyfunction] - fn globals(vm: &VirtualMachine) -> PyAnyDictRef { + fn globals(vm: &VirtualMachine) -> PyRef { vm.current_globals() } diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 8cf93881d..0183355c2 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -24,7 +24,7 @@ mod vm_ops; use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, builtins::{ - self, PyAnyDictRef, PyBaseExceptionRef, PyBaseObject, PyDict, PyFrozenSet, PyInt, PyList, + self, PyAnyDict, PyBaseExceptionRef, PyBaseObject, PyDict, PyFrozenSet, PyInt, PyList, PyModule, PySet, PyStr, PyStrInterned, PyStrRef, PyTypeRef, PyUtf8Str, PyUtf8StrInterned, PyWeak, code::PyCode, @@ -3113,7 +3113,7 @@ impl VirtualMachine { .locals(self) } - pub fn current_globals(&self) -> PyAnyDictRef { + pub fn current_globals(&self) -> PyRef { let ptr = crate::vm::thread::get_current_frame(); if !ptr.is_null() { return unsafe { (*ptr).globals().to_owned() }; diff --git a/extra_tests/snippets/builtin_dict.py b/extra_tests/snippets/builtin_dict.py index ef4b3ec1e..b4983c7aa 100644 --- a/extra_tests/snippets/builtin_dict.py +++ b/extra_tests/snippets/builtin_dict.py @@ -2125,3 +2125,106 @@ frozendict(**kwargs) -> new immutable dictionary initialized with the name=value frozen_result = unittest.TextTestRunner(verbosity=2).run(frozen_suite) assert frozen_result.wasSuccessful() assert not frozen_result.skipped + + +# Final 3.15 rewrites exact hash TypeError without an explicit cause, and +# propagates formatting failures instead of replacing them with diagnostics. +def check_mapping_hash_error_context(): + class SpecialTypeError(TypeError): + pass + + for mapping_type in (dict, frozendict): + for case in ( + "plain", + "empty", + "multiple", + "bad_str", + "bad_repr", + "subclass", + "subclass_bad_str", + "list", + ): + for handled in (False, True): + for operation in ("getitem", "get", "contains"): + callback_error = RuntimeError("format failed") + callbacks = [] + + class BadText: + def __str__(self): + callbacks.append("str") + raise callback_error + + def __repr__(self): + callbacks.append("repr") + raise callback_error + + args = { + "plain": ("hash failed",), + "empty": (), + "multiple": ("one", 2), + "bad_str": (BadText(),), + "bad_repr": ("one", BadText()), + "subclass": ("hash failed",), + "subclass_bad_str": (BadText(),), + "list": (), + }[case] + error_type = ( + SpecialTypeError if case.startswith("subclass") else TypeError + ) + original = error_type(*args) + outer = ValueError("outer") + + class Key: + def __hash__(self): + raise original + + key = [] if case == "list" else Key() + mapping = mapping_type() + + def invoke(): + try: + if operation == "getitem": + mapping[key] + elif operation == "get": + mapping.get(key) + else: + key in mapping + except Exception as error: + assert error.__cause__ is None + assert error.__context__ is (outer if handled else None) + assert error.__suppress_context__ is False + if case.startswith("subclass"): + assert error is original + assert callbacks == [] + elif case in ("bad_str", "bad_repr"): + assert error is callback_error + assert callbacks == [ + "str" if case == "bad_str" else "repr" + ] + else: + assert type(error) is TypeError + assert error is not original + name = "list" if case == "list" else Key.__qualname__ + message = { + "plain": "hash failed", + "empty": "", + "multiple": "('one', 2)", + "list": "unhashable type: 'list'", + }[case] + assert str(error) == ( + f"cannot use '{name}' as a {mapping_type.__name__} key ({message})" + ) + else: + raise AssertionError("hash failure was not propagated") + + if handled: + try: + raise outer + except ValueError: + invoke() + else: + invoke() + + +if hasattr(builtins, "frozendict"): + check_mapping_hash_error_context() diff --git a/src/shell/helper.rs b/src/shell/helper.rs index 1b6d3d72a..dfd3571bf 100644 --- a/src/shell/helper.rs +++ b/src/shell/helper.rs @@ -1,14 +1,14 @@ #![cfg_attr(target_arch = "wasm32", allow(dead_code))] use rustpython_vm::{ - AsObject, PyResult, TryFromObject, VirtualMachine, - builtins::{PyAnyDictRef, PyStrRef}, + AsObject, PyRef, PyResult, TryFromObject, VirtualMachine, + builtins::{PyAnyDict, PyStrRef}, function::ArgIterable, identifier, }; pub(super) struct ShellHelper<'vm> { vm: &'vm VirtualMachine, - globals: PyAnyDictRef, + globals: PyRef, } fn reverse_string(s: &mut String) { @@ -54,7 +54,7 @@ fn split_idents_on_dot(line: &str) -> Option<(usize, Vec)> { } impl<'vm> ShellHelper<'vm> { - pub(super) const fn new(vm: &'vm VirtualMachine, globals: PyAnyDictRef) -> Self { + pub(super) const fn new(vm: &'vm VirtualMachine, globals: PyRef) -> Self { ShellHelper { vm, globals } }