diff --git a/Cargo.lock b/Cargo.lock index 9a0b3b46c9b..f14234a3d38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3731,6 +3731,7 @@ dependencies = [ "rustpython-pylib", "rustpython-stdlib", "rustpython-vm", + "static_assertions", ] [[package]] diff --git a/crates/capi/Cargo.toml b/crates/capi/Cargo.toml index 671c8d1640c..08c102b4a64 100644 --- a/crates/capi/Cargo.toml +++ b/crates/capi/Cargo.toml @@ -22,6 +22,7 @@ rustpython-stdlib = {workspace = true, features = ["threading"] } rustpython-pylib = { workspace = true } [dev-dependencies] +static_assertions = { workspace = true } pyo3 = { workspace = true, features = ["auto-initialize", "abi3t"] } [lints] diff --git a/crates/capi/src/pyerrors.rs b/crates/capi/src/pyerrors.rs index e43358ae348..0bf843aa442 100644 --- a/crates/capi/src/pyerrors.rs +++ b/crates/capi/src/pyerrors.rs @@ -5,7 +5,6 @@ use core::convert::Infallible; use core::ffi::{c_char, c_int}; use core::slice; use rustpython_vm::builtins::{PyBaseException, PyTraceback, PyTuple, PyType}; -use rustpython_vm::convert::IntoObject; use rustpython_vm::exceptions::ExceptionZoo; use rustpython_vm::{AsObject, PyResult}; diff --git a/crates/capi/src/util.rs b/crates/capi/src/util.rs index 5d0e2c26f5a..d8926f44d28 100644 --- a/crates/capi/src/util.rs +++ b/crates/capi/src/util.rs @@ -40,20 +40,12 @@ where impl FfiResult<*mut Py> for PyRef where - Self: Into, + T: rustpython_vm::object::PyRefTarget>, { const ERR_VALUE: *mut Py = core::ptr::null_mut(); fn into_output(self, _vm: &VirtualMachine) -> *mut Py { - self.into().into_raw().as_ptr().cast() - } -} - -impl FfiResult<*mut PyObject> for PyObjectRef { - const ERR_VALUE: *mut PyObject = core::ptr::null_mut(); - - fn into_output(self, _vm: &VirtualMachine) -> *mut PyObject { - self.into_raw().as_ptr() + PyObjectRef::from(self).into_raw().as_ptr().cast() } } @@ -324,7 +316,7 @@ impl FfiPtrExt for *mut Py { "Attempted to dereference NULL {}", type_name::() ); - unsafe { PyRef::from_raw(self.cast_const()) } + unsafe { PyRef::::from_raw(self.cast_const()) } } #[inline] @@ -384,6 +376,14 @@ mod tests { use core::ffi::{c_longlong, c_ulonglong}; use core::fmt::Debug; + #[test] + fn pyref_object_ffi_targets() { + use rustpython_vm::builtins::PyInt; + static_assertions::assert_impl_all!(PyObjectRef: FfiResult<*mut PyObject>); + static_assertions::assert_impl_all!(PyRef: FfiResult<*mut PyObject>, FfiResult<*mut Py>); + static_assertions::assert_not_impl_any!(PyObjectRef: FfiResult<*mut Py>); + } + #[test] fn ffi_result_err_value() { fn assert_error_value(value: Output) diff --git a/crates/stdlib/src/_sqlite3.rs b/crates/stdlib/src/_sqlite3.rs index d0bb95a6c9f..08bf755a23d 100644 --- a/crates/stdlib/src/_sqlite3.rs +++ b/crates/stdlib/src/_sqlite3.rs @@ -61,7 +61,6 @@ mod _sqlite3 { PyInt, PyIntRef, PyModule, PySlice, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, PyUtf8Str, PyUtf8StrRef, }, - convert::IntoObject, function::{ ArgCallable, ArgIterable, FsPath, FuncArgs, KwArgs, OptionalArg, PyComparisonValue, PySetterValue, TimeoutSeconds, diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index e8329fbb572..af7e5dcf37b 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -541,12 +541,6 @@ impl MemberCell for core::sync::atomic::AtomicU32 {} // One object pointer. Readonly members may be a plain pointer. Writable // members are an atomic cell: `PyAtomicRef` when the pointer is // never null, or `PyAtomicRef>` when it may be. -impl MemberLayout for PyObjectRef { - const KIND: MemberKind = MemberKind::Object; -} -impl MemberLayout for Option { - const KIND: MemberKind = MemberKind::Object; -} impl MemberLayout for crate::object::PyObjectCell { const KIND: MemberKind = MemberKind::Object; const ATOMIC: bool = true; diff --git a/crates/vm/src/builtins/frame_locals_proxy.rs b/crates/vm/src/builtins/frame_locals_proxy.rs index cbd21527da3..28da2341f64 100644 --- a/crates/vm/src/builtins/frame_locals_proxy.rs +++ b/crates/vm/src/builtins/frame_locals_proxy.rs @@ -145,10 +145,10 @@ impl FrameLocalsProxy { return Ok(vm.ctx.not_implemented()); } let result = self.snapshot(vm)?; - if other.downcast_ref::().is_some() { + if let Some(dict) = other.downcast_ref::() { // PyDict_Update reads a dict subclass's stored entries directly; // it does not dispatch to overridden mapping methods. - result.merge_dict(&other.downcast().unwrap(), true, vm)?; + result.merge_dict(dict, true, vm)?; } else { result.merge_object(other, vm)?; } diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 992f14ac2f4..9c2255d0084 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -16,7 +16,7 @@ use crate::{ rc::PyRc, wtf8::Wtf8Buf, }, - convert::ToPyResult, + convert::{ToPyResult, TransmuteFromObject}, dict_inner::{self, DictSize}, function::{ ArgIterable, FuncArgs, NameOthers, OptionalArg, PosArgs, PyArithmeticValue, @@ -70,7 +70,7 @@ impl PySet { fn op( &self, - other: AnySet, + other: PyRef, op: fn(&PySetInner, ArgIterable, &VirtualMachine) -> PyResult, vm: &VirtualMachine, ) -> PyResult { @@ -132,7 +132,7 @@ impl PyFrozenSet { fn op( &self, - other: AnySet, + other: PyRef, op: fn(&PySetInner, ArgIterable, &VirtualMachine) -> PyResult, vm: &VirtualMachine, ) -> PyResult { @@ -537,14 +537,14 @@ impl PySetInner { } fn update_internal(&self, iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - // check AnySet - if let Ok(any_set) = AnySet::try_from_object(vm, iterable.to_owned()) { + // check set or frozenset + if let Ok(any_set) = PyRef::::try_from_object(vm, iterable.to_owned()) { self.merge_set(any_set, vm) // check Dict } else if let Ok(dict) = iterable.to_owned().downcast_exact::(vm) { self.merge_dict(&dict, vm) } else { - // add iterable that is not AnySet or Dict + // add iterable that is not a set, frozenset, or dict for item in iterable.try_into_value::(vm)?.iter(vm)? { let item = item?; self.add(&item, vm)?; @@ -553,7 +553,7 @@ impl PySetInner { } } - fn merge_set(&self, any_set: AnySet, vm: &VirtualMachine) -> PyResult<()> { + fn merge_set(&self, any_set: PyRef, vm: &VirtualMachine) -> PyResult<()> { for (item, hash) in any_set.as_inner().content.keys_with_hashes() { self.add_known_hash(&item, hash, vm)?; } @@ -752,7 +752,7 @@ impl PySet { } fn __or__(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(self.op( other, PySetInner::union, @@ -768,7 +768,7 @@ impl PySet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { inner: self.inner.intersection(other.into_iterable(vm)?, vm)?, })) @@ -782,7 +782,7 @@ impl PySet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { inner: self.inner.difference_new(other.into_iterable(vm)?, vm)?, })) @@ -796,7 +796,7 @@ impl PySet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { inner: other .as_inner() @@ -812,7 +812,7 @@ impl PySet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(self.op( other, PySetInner::symmetric_difference, @@ -823,12 +823,20 @@ impl PySet { } } - fn __ior__(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { + fn __ior__( + zelf: PyRef, + set: PyRef, + vm: &VirtualMachine, + ) -> PyResult> { zelf.inner.merge_set(set, vm)?; Ok(zelf) } - fn __iand__(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { + fn __iand__( + zelf: PyRef, + set: PyRef, + vm: &VirtualMachine, + ) -> PyResult> { if !set.is(zelf.as_object()) { zelf.inner .intersection_update(core::iter::once(set.into_iterable(vm)?), vm)?; @@ -836,7 +844,11 @@ impl PySet { Ok(zelf) } - fn __isub__(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { + fn __isub__( + zelf: PyRef, + set: PyRef, + vm: &VirtualMachine, + ) -> PyResult> { if set.is(zelf.as_object()) { zelf.inner.clear(); } else { @@ -846,7 +858,11 @@ impl PySet { Ok(zelf) } - fn __ixor__(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { + fn __ixor__( + zelf: PyRef, + set: PyRef, + vm: &VirtualMachine, + ) -> PyResult> { if set.is(zelf.as_object()) { zelf.inner.clear(); } else { @@ -1068,10 +1084,7 @@ impl Comparable for PySet { impl Iterable for PySet { fn iter(zelf: PyRef, vm: &VirtualMachine) -> PyResult { - Ok(PySetIterator::new(AnySet { - object: zelf.into(), - }) - .into_pyobject(vm)) + Ok(PySetIterator::new(zelf.into()).into_pyobject(vm)) } } @@ -1081,7 +1094,7 @@ impl AsNumber for PySet { // Binary ops check both operands are sets (like CPython's set_sub, etc.) // This is needed because __rsub__ swaps operands: a.__rsub__(b) calls subtract(b, a) subtract: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1096,7 +1109,7 @@ impl AsNumber for PySet { } }), and: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1110,7 +1123,7 @@ impl AsNumber for PySet { } }), xor: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1124,7 +1137,7 @@ impl AsNumber for PySet { } }), or: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1139,32 +1152,48 @@ impl AsNumber for PySet { }), inplace_subtract: Some(|a, b, vm| { if let Some(a) = a.downcast_ref::() { - PySet::__isub__(a.to_owned(), AnySet::try_from_object(vm, b.to_owned())?, vm) - .to_pyresult(vm) + PySet::__isub__( + a.to_owned(), + PyRef::::try_from_object(vm, b.to_owned())?, + vm, + ) + .to_pyresult(vm) } else { Ok(vm.ctx.not_implemented()) } }), inplace_and: Some(|a, b, vm| { if let Some(a) = a.downcast_ref::() { - PySet::__iand__(a.to_owned(), AnySet::try_from_object(vm, b.to_owned())?, vm) - .to_pyresult(vm) + PySet::__iand__( + a.to_owned(), + PyRef::::try_from_object(vm, b.to_owned())?, + vm, + ) + .to_pyresult(vm) } else { Ok(vm.ctx.not_implemented()) } }), inplace_xor: Some(|a, b, vm| { if let Some(a) = a.downcast_ref::() { - PySet::__ixor__(a.to_owned(), AnySet::try_from_object(vm, b.to_owned())?, vm) - .to_pyresult(vm) + PySet::__ixor__( + a.to_owned(), + PyRef::::try_from_object(vm, b.to_owned())?, + vm, + ) + .to_pyresult(vm) } else { Ok(vm.ctx.not_implemented()) } }), inplace_or: Some(|a, b, vm| { if let Some(a) = a.downcast_ref::() { - PySet::__ior__(a.to_owned(), AnySet::try_from_object(vm, b.to_owned())?, vm) - .to_pyresult(vm) + PySet::__ior__( + a.to_owned(), + PyRef::::try_from_object(vm, b.to_owned())?, + vm, + ) + .to_pyresult(vm) } else { Ok(vm.ctx.not_implemented()) } @@ -1279,7 +1308,7 @@ impl PyFrozenSet { } fn __or__(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult> { - if let Ok(set) = AnySet::try_from_object(vm, other) { + if let Ok(set) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(self.op( set, PySetInner::union, @@ -1295,7 +1324,7 @@ impl PyFrozenSet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { inner: self.inner.intersection(other.into_iterable(vm)?, vm)?, ..Default::default() @@ -1310,7 +1339,7 @@ impl PyFrozenSet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { inner: self.inner.difference_new(other.into_iterable(vm)?, vm)?, ..Default::default() @@ -1325,7 +1354,7 @@ impl PyFrozenSet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { inner: other .as_inner() @@ -1342,7 +1371,7 @@ impl PyFrozenSet { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - if let Ok(other) = AnySet::try_from_object(vm, other) { + if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(self.op( other, PySetInner::symmetric_difference, @@ -1520,10 +1549,7 @@ impl Comparable for PyFrozenSet { impl Iterable for PyFrozenSet { fn iter(zelf: PyRef, vm: &VirtualMachine) -> PyResult { - Ok(PySetIterator::new(AnySet { - object: zelf.into(), - }) - .into_pyobject(vm)) + Ok(PySetIterator::new(zelf.into()).into_pyobject(vm)) } } @@ -1533,7 +1559,7 @@ impl AsNumber for PyFrozenSet { // Binary ops check both operands are sets (like CPython's set_sub, etc.) // __rsub__ swaps operands. Result type follows first operand's type. subtract: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1546,7 +1572,7 @@ impl AsNumber for PyFrozenSet { } }), and: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1558,7 +1584,7 @@ impl AsNumber for PyFrozenSet { } }), xor: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1570,7 +1596,7 @@ impl AsNumber for PyFrozenSet { } }), or: Some(|a, b, vm| { - if !AnySet::check(a, vm) || !AnySet::check(b, vm) { + if !PyRef::::check(a, vm) || !PyRef::::check(b, vm) { return Ok(vm.ctx.not_implemented()); } if let Some(a) = a.downcast_ref::() { @@ -1604,27 +1630,78 @@ impl Representable for PyFrozenSet { } } -struct AnySet { - object: PyObjectRef, +/// A borrowed view of either a set or a frozenset, including subclasses. +/// +/// This is an erased header view, not a shared payload layout: every payload +/// access still downcasts to the actual alternative. It cannot be allocated as +/// a payload or constructed without checking the object. +#[repr(transparent)] +pub(crate) struct PyAnySet { + object: PyObject, } -impl Borrow for AnySet { +impl Borrow for PyAnySet { #[inline(always)] fn borrow(&self) -> &PyObject { &self.object } } -impl AnySet { - /// Check if object is a set or frozenset (including subclasses) - /// Equivalent to CPython's PyAnySet_Check +impl Borrow for PyRef { + fn borrow(&self) -> &PyAnySet { + self + } +} + +impl ToOwned for PyAnySet { + type Owned = PyRef; + + fn to_owned(&self) -> PyRef { + // SAFETY: this borrowed view already guarantees a set payload. + unsafe { PyRef::::from_object_unchecked(self.object.to_owned()) } + } +} + +impl From> for PyRef { + fn from(set: PyRef) -> Self { + // SAFETY: the typed owner guarantees the set alternative. + unsafe { Self::from_object_unchecked(set.into()) } + } +} + +impl From> for PyRef { + fn from(set: PyRef) -> Self { + // SAFETY: the typed owner guarantees the frozenset alternative. + unsafe { Self::from_object_unchecked(set.into()) } + } +} + +impl From> for PyObjectRef { + fn from(set: PyRef) -> Self { + set.into_object() + } +} + +impl PyAnySet { + /// Check if object is a set or frozenset (including subclasses). + /// Equivalent to CPython's PyAnySet_Check. fn check(obj: &PyObject, vm: &VirtualMachine) -> bool { let ctx = &vm.ctx; obj.fast_isinstance(ctx.types.set_type) || obj.fast_isinstance(ctx.types.frozenset_type) } + fn as_inner(&self) -> &PySetInner { + extract_set(&self.object).expect("PyAnySet is always PySet or PyFrozenSet") + } +} + +impl PyRef { + fn check(obj: &PyObject, vm: &VirtualMachine) -> bool { + PyAnySet::check(obj, vm) + } + fn into_iterable(self, vm: &VirtualMachine) -> PyResult { - self.object.try_into_value(vm) + self.into_object().try_into_value(vm) } fn into_iterable_iter( @@ -1633,26 +1710,28 @@ impl AnySet { ) -> PyResult> { Ok(core::iter::once(self.into_iterable(vm)?)) } +} - fn as_inner(&self) -> &PySetInner { - match_class!(match self.object.as_object() { - ref set @ PySet => &set.inner, - ref frozen @ PyFrozenSet => &frozen.inner, - _ => unreachable!("AnySet is always PySet or PyFrozenSet"), // should not be called. - }) +// SAFETY: PyRef is a transparent PyRef owner. A successful check proves +// both the Python class and the actual set/frozenset payload of this object. +unsafe impl TransmuteFromObject for PyRef { + fn check(vm: &VirtualMachine, obj: &PyObject) -> PyResult<()> { + if PyAnySet::check(obj, vm) && extract_set(obj).is_some() { + Ok(()) + } else { + Err(vm.new_type_error(format!( + "{} is not a subtype of set or frozenset", + obj.class() + ))) + } } } -impl TryFromObject for AnySet { +impl TryFromObject for PyRef { fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { - let class = obj.class(); - if class.fast_issubclass(vm.ctx.types.set_type) - || class.fast_issubclass(vm.ctx.types.frozenset_type) - { - Ok(Self { object: obj }) - } else { - Err(vm.new_type_error(format!("{class} is not a subtype of set or frozenset"))) - } + ::check(vm, &obj)?; + // SAFETY: the shared checker proved the union view's payload invariant. + Ok(unsafe { Self::from_object_unchecked(obj) }) } } @@ -1663,7 +1742,7 @@ pub(crate) struct PySetIterator { /// records by writing a size no set can have. Sticky: what it makes the /// iterator answer, it answers from then on. changed: PyAtomic, - internal: PyMutex>, + internal: PyMutex>>, } impl fmt::Debug for PySetIterator { @@ -1681,7 +1760,7 @@ impl PyPayload for PySetIterator { } impl PySetIterator { - fn new(set: AnySet) -> Self { + fn new(set: PyRef) -> Self { Self { size: set.as_inner().content.size(), changed: Radium::new(false), @@ -1811,3 +1890,151 @@ pub(crate) fn init(context: &'static Context) { PySetIterator::extend_class(context, context.types.set_iterator_type); } + +#[cfg(test)] +mod reference_tests { + use super::*; + + #[test] + fn anyset_reference_layout_and_ownership() { + static_assertions::assert_not_impl_any!(PyAnySet: PyPayload); + static_assertions::assert_type_eq_all!( + ::Target, + PyAnySet + ); + static_assertions::assert_eq_size!(PyRef, PyObjectRef, Option>); + static_assertions::assert_eq_align!(PyRef, PyObjectRef); + static_assertions::assert_eq_size!(PyAnySet, PyObject); + static_assertions::assert_eq_align!(PyAnySet, 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 set: PyRef = PySet::default().into_ref(ctx).into(); + let frozen: PyRef = PyFrozenSet::default().into_ref(ctx).into(); + for owner in [set, frozen] { + let object = owner.as_object() as *const PyObject; + assert_eq!(owner.as_object().strong_count(), 1); + let borrowed: &PyAnySet = &owner; + assert!(core::ptr::eq(borrowed.as_object(), object)); + assert!(core::ptr::eq( + borrowed.as_inner(), + extract_set(owner.as_object()).unwrap() + )); + 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)); + drop(copy); + drop(owner); + assert_eq!(erased.strong_count(), 1); + } + } + + #[test] + fn anyset_checked_reference_conversion() { + crate::Interpreter::without_stdlib(Default::default()).enter(|vm| { + let set = PySet::default().into_pyobject(vm); + let frozen = PyFrozenSet::default().into_pyobject(vm); + for object in [set, 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(); + 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); + }); + } + + #[test] + fn anyset_typed_tuple_views_and_refcounts() { + use crate::builtins::PyTuple; + type Typed = PyTuple>; + type Nested = PyTuple>; + static_assertions::assert_not_impl_any!(PyObject: crate::object::PyTypedRefTarget); + static_assertions::assert_impl_all!(PyAnySet: crate::object::PyTypedRefTarget); + static_assertions::assert_impl_all!(Typed: crate::object::PyTypedRefTarget); + static_assertions::assert_eq_size!(Typed, PyTuple); + static_assertions::assert_eq_align!(Typed, PyTuple); + static_assertions::assert_eq_size!(Py, Py); + static_assertions::assert_eq_align!(Py, Py); + + crate::Interpreter::without_stdlib(Default::default()).enter(|vm| { + let set: PyRef = PySet::default().into_ref(&vm.ctx).into(); + let frozen: PyRef = PyFrozenSet::default().into_ref(&vm.ctx).into(); + let tuple = Typed::new_ref_typed(vec![set.clone(), frozen.clone()], &vm.ctx); + assert_eq!(set.as_object().strong_count(), 2); + assert_eq!(frozen.as_object().strong_count(), 2); + assert!( + tuple.as_slice()[0] + .as_object() + .downcast_ref::() + .is_some() + ); + assert!( + tuple.as_slice()[1] + .as_object() + .downcast_ref::() + .is_some() + ); + assert!(core::ptr::eq( + tuple.as_slice()[0].as_inner(), + set.as_inner() + )); + assert!(core::ptr::eq( + tuple.as_slice()[1].as_inner(), + frozen.as_inner() + )); + let erased = tuple.clone().into_untyped(); + assert!(core::ptr::eq(tuple.as_untyped(), &*erased)); + assert_eq!(erased.as_object().strong_count(), 2); + let checked = erased.try_into_typed::(vm).unwrap(); + assert!(core::ptr::eq(&*checked, &*tuple)); + assert_eq!(checked.as_object().strong_count(), 2); + let borrowed_copy = (*checked).to_owned(); + assert_eq!(tuple.as_object().strong_count(), 3); + let nested: PyRef = PyTuple::new_ref_typed(vec![borrowed_copy], &vm.ctx); + assert!(core::ptr::eq(&*nested.as_slice()[0], &*tuple)); + let untyped: PyTupleRef = nested.into(); + assert_eq!(tuple.as_object().strong_count(), 3); + drop(untyped); + drop(checked); + assert_eq!(tuple.as_object().strong_count(), 1); + drop(tuple); + assert_eq!(set.as_object().strong_count(), 1); + assert_eq!(frozen.as_object().strong_count(), 1); + }); + } + + #[test] + fn anyset_typed_tuple_rejects_invalid_element_without_leaking() { + use crate::builtins::PyTuple; + crate::Interpreter::without_stdlib(Default::default()).enter(|vm| { + let set = PySet::default().into_pyobject(vm); + let wrong: PyObjectRef = vm.ctx.new_int(1000).into(); + let tuple = PyTuple::new_ref(vec![set.clone(), wrong.clone()], &vm.ctx); + let error = tuple.clone().try_into_typed::(vm).unwrap_err(); + assert!(error.fast_isinstance(vm.ctx.exceptions.type_error)); + assert_eq!(tuple.as_object().strong_count(), 1); + assert!(core::ptr::eq(&*tuple.as_slice()[0], &*set)); + assert!(core::ptr::eq(&*tuple.as_slice()[1], &*wrong)); + assert_eq!(set.strong_count(), 2); + assert_eq!(wrong.strong_count(), 2); + drop(tuple); + assert_eq!(set.strong_count(), 1); + assert_eq!(wrong.strong_count(), 1); + }); + } +} diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index c8647057330..4421a67e818 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -4,7 +4,7 @@ use super::{ }; use crate::common::lock::LazyLock; use crate::common::{hash, hash::PyHash, lock::PyMutex, wtf8::wtf8_concat}; -use crate::object::{Traverse, TraverseFn}; +use crate::object::{PyTypedRefTarget, Traverse, TraverseFn}; use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, atomic_func, @@ -25,9 +25,12 @@ use crate::{ }; use alloc::fmt; use core::cell::{Cell, UnsafeCell}; +use core::marker::PhantomData; +use core::mem::ManuallyDrop; use core::ptr::NonNull; #[pyclass(module = false, name = "tuple", traverse = "manual")] +#[repr(transparent)] pub struct PyTuple { elements: TupleElements, } @@ -35,30 +38,41 @@ pub struct PyTuple { /// Tuple storage is immutable after publication, but marshal must publish a /// tuple in its reference table before recursively reading its children. /// This mirrors CPython's `PyTuple_New` followed by `PyTuple_SET_ITEM`. -struct TupleElements(UnsafeCell>); +// Every allocation stores erased owners, including when borrowed through a +// typed tuple view. The marker preserves the old storage's invariance and +// ownership relationship to R. +// Safe constructors and checked view conversions only admit R = PyRef, +// and establish each element's target invariant before publishing that view. +#[repr(transparent)] +struct TupleElements(UnsafeCell>, PhantomData>); unsafe impl Send for TupleElements {} unsafe impl Sync for TupleElements {} impl TupleElements { - const fn new(elements: Box<[R]>) -> Self { - Self(UnsafeCell::new(elements)) + fn as_slice(&self) -> &[R] { + // SAFETY: all admitted R are transparent PyRef owners with the same + // size/alignment as PyObjectRef. Their element invariants were proven + // by construction or checked conversion. Published elements cannot be + // replaced, and the returned shared slice borrows this storage. + let elements = unsafe { &*self.0.get() }; + unsafe { core::slice::from_raw_parts(elements.as_ptr().cast(), elements.len()) } } +} - fn as_slice(&self) -> &[R] { - // SAFETY: initialization writes happen only while the tuple is owned by - // the synchronous marshal decoder; afterwards the storage is immutable. - unsafe { &*self.0.get() } +impl TupleElements { + const fn new(elements: Box<[PyObjectRef]>) -> Self { + Self(UnsafeCell::new(elements), PhantomData) } - fn get_mut(&mut self) -> &mut Box<[R]> { + fn get_mut(&mut self) -> &mut Box<[PyObjectRef]> { self.0.get_mut() } /// # Safety /// The tuple must still be in its private initialization phase, and each /// placeholder index must be replaced at most once before it is observable. - unsafe fn set_initializing(&self, index: usize, value: R) { + unsafe fn set_initializing(&self, index: usize, value: PyObjectRef) { unsafe { (*self.0.get())[index] = value }; } } @@ -398,6 +412,38 @@ impl Py> { } impl Py { + fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { + match SequenceIndex::try_from_borrowed_object::(vm, needle)? { + SequenceIndex::Int(i) => { + let index = self + .elements + .wrap_index(i) + .ok_or_else(|| vm.new_index_error("tuple index out of range"))?; + Ok(self.elements[index].clone()) + } + SequenceIndex::Slice(slice) => { + let len = self.as_slice().len(); + let (range, step, slice_len) = slice.adjust_indices(len); + if slice_len != 0 + && range.start == 0 + && step == 1 + && slice_len == len + && self.class().is(vm.ctx.types.tuple_type) + { + Ok(self.to_owned().into()) + } else { + self.elements + .getitem_by_slice(vm, slice) + .map(|x| vm.ctx.new_tuple(x).into()) + } + } + } + } + + fn __getitem__(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { + self._getitem(needle, vm) + } + pub fn extract_tuple<'a, T: FromPyTuple<'a>>(&'a self, vm: &VirtualMachine) -> PyResult { T::from_pytuple(self, vm) } @@ -410,8 +456,14 @@ impl PyTuple> { ) -> PyRef { // SAFETY: PyRef has the same layout as PyObjectRef. unsafe { - let elements: Vec = - core::mem::transmute::>, Vec>(elements); + let mut elements = ManuallyDrop::new(elements); + // Preserve the buffer, length and capacity without assuming Vec's + // field order. Each transparent owner transfers its one reference. + let elements = Vec::from_raw_parts( + elements.as_mut_ptr().cast::(), + elements.len(), + elements.capacity(), + ); let tuple = PyRef::new_ref( PyTuple::new_unchecked(elements.into_boxed_slice()), tuple_type, @@ -424,8 +476,14 @@ impl PyTuple> { pub fn new_ref_typed(elements: Vec>, ctx: &Context) -> PyRef { // SAFETY: PyRef has the same layout as PyObjectRef unsafe { - let elements: Vec = - core::mem::transmute::>, Vec>(elements); + let mut elements = ManuallyDrop::new(elements); + // Preserve the buffer, length and capacity without assuming Vec's + // field order. Each transparent owner transfers its one reference. + let elements = Vec::from_raw_parts( + elements.as_mut_ptr().cast::(), + elements.len(), + elements.capacity(), + ); let tuple = PyTuple::::new_ref(elements, ctx); core::mem::transmute::, PyRef>(tuple) } @@ -469,26 +527,6 @@ impl PyTuple { Self::repeat(zelf, value, vm) } - fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object::(vm, needle)? { - SequenceIndex::Int(i) => { - let index = self - .elements - .wrap_index(i) - .ok_or_else(|| vm.new_index_error("tuple index out of range"))?; - Ok(self.elements[index].clone()) - } - SequenceIndex::Slice(slice) => self - .elements - .getitem_by_slice(vm, slice) - .map(|x| vm.ctx.new_tuple(x).into()), - } - } - - fn __getitem__(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - self._getitem(needle, vm) - } - fn _contains(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { for element in &self.elements { if vm.identical_or_equal(element, needle)? { @@ -565,9 +603,9 @@ impl AsMapping for PyTuple { length: atomic_func!(|mapping, _vm| { Ok(PyTuple::mapping_downcast(mapping).as_slice().len()) }), - subscript: atomic_func!(|mapping, needle, vm| PyTuple::mapping_downcast(mapping) - .payload - ._getitem(needle, vm)), + subscript: atomic_func!( + |mapping, needle, vm| PyTuple::mapping_downcast(mapping)._getitem(needle, vm) + ), ..PyMappingMethods::NOT_IMPLEMENTED }); &AS_MAPPING @@ -686,35 +724,42 @@ impl Representable for PyTuple { } impl PyRef> { - pub fn try_into_typed( + pub fn try_into_typed( self, vm: &VirtualMachine, - ) -> PyResult>>> { + ) -> PyResult>>> + where + PyRef: TransmuteFromObject, + { // Check that all elements are of the correct type for elem in self.as_slice() { as TransmuteFromObject>::check(vm, elem)?; } - // SAFETY: We just verified all elements are of type T + // SAFETY: every element satisfies T's view invariant. Transparent + // tuple wrappers retain the same erased storage for every marker; + // this transfers the owner without changing its allocation or count. Ok(unsafe { core::mem::transmute::>>>(self) }) } } -impl PyRef>> { +impl PyRef>> { #[must_use] pub fn into_untyped(self) -> PyRef { - // SAFETY: PyTuple> has the same layout as PyTuple + // SAFETY: transparent tuple wrappers have identical erased storage; + // forgetting the element guarantee does not change the allocation. unsafe { core::mem::transmute::>(self) } } } -impl Py>> { +impl Py>> { pub fn as_untyped(&self) -> &Py { - // SAFETY: PyTuple> has the same layout as PyTuple + // SAFETY: transparent tuple wrappers have identical erased storage; + // forgetting the element guarantee does not change the allocation. unsafe { core::mem::transmute::<&Self, &Py>(self) } } } -impl From>>> for PyTupleRef { +impl From>>> for PyTupleRef { #[inline] fn from(tup: PyRef>>) -> Self { tup.into_untyped() diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 1eed9b712a0..d22d6ef7475 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -1,15 +1,3 @@ -//! Essential types for object models -//! -//! +-------------------------+--------------+-----------------------+ -//! | Management | Typed | Untyped | -//! +-------------------------+------------------+-------------------+ -//! | Interpreter-independent | [`Py`] | [`PyObject`] | -//! | Reference-counted | [`PyRef`] | [`PyObjectRef`] | -//! | Weak | [`PyWeakRef`] | [`PyRef`] | -//! +-------------------------+--------------+-----------------------+ -//! -//! [`PyRef`] may looking like to be called as PyObjectWeak by the rule, -//! but not to do to remember it is a PyRef object. use super::{ PyAtomicRef, ext::{AsObject, PyRefExact, PyResult}, @@ -708,7 +696,7 @@ unsafe fn try_reuse_weakref(ptr: *mut Py) -> Option> { let node = unsafe { &*ptr }; node.ref_count .safe_inc() - .then(|| unsafe { PyRef::from_raw(ptr) }) + .then(|| unsafe { PyRef::::from_raw(ptr) }) } impl WeakRefList { @@ -889,7 +877,7 @@ impl WeakRefList { // Collect callback only if we can still acquire a strong ref. if wr.ref_count.safe_inc() { - let wr_ref = unsafe { PyRef::from_raw(wr as *const Py) }; + let wr_ref = unsafe { PyRef::::from_raw(wr as *const Py) }; let cb = unsafe { wr.payload.callback.get().replace(None) }; if let Some(cb) = cb { callbacks.push((wr_ref, cb)); @@ -940,7 +928,7 @@ impl WeakRefList { // Collect callback without invoking only if we can keep weakref alive. if wr.ref_count.safe_inc() { - let wr_ref = unsafe { PyRef::from_raw(wr as *const Py) }; + let wr_ref = unsafe { PyRef::::from_raw(wr as *const Py) }; let cb = unsafe { wr.payload.callback.get().replace(None) }; if let Some(cb) = cb { callbacks.push((wr_ref, cb)); @@ -974,7 +962,7 @@ impl WeakRefList { while let Some(node) = current { let wr = unsafe { node.as_ref() }; if wr.ref_count.safe_inc() { - v.push(unsafe { PyRef::from_raw(wr as *const Py) }); + v.push(unsafe { PyRef::::from_raw(wr as *const Py) }); } current = unsafe { WeakLink::pointers(node).as_ref().get_next() }; } @@ -1001,7 +989,7 @@ unsafe impl Link for WeakLink { #[inline(always)] unsafe fn from_raw(ptr: NonNull) -> Self::Handle { - unsafe { PyRef::from_raw(ptr.as_ptr()) } + unsafe { PyRef::::from_raw(ptr.as_ptr()) } } #[inline(always)] @@ -1513,38 +1501,11 @@ impl core::ops::DerefMut for FreeList { /// this reference counting is accounted for by this type. Use the `.clone()` /// method to create a new reference and increment the amount of references /// to the python object by 1. -#[repr(transparent)] -pub struct PyObjectRef { - ptr: NonNull, -} - -impl Clone for PyObjectRef { - #[inline(always)] - fn clone(&self) -> Self { - (**self).to_owned() - } -} - -cfg_select! { - feature = "threading" => { - unsafe impl Send for PyObjectRef {} - unsafe impl Sync for PyObjectRef {} - } - _ => {} -} +pub type PyObjectRef = PyRef; #[repr(transparent)] pub struct PyObject(Py); -impl Deref for PyObjectRef { - type Target = PyObject; - - #[inline(always)] - fn deref(&self) -> &PyObject { - unsafe { self.ptr.as_ref() } - } -} - impl ToOwned for PyObject { type Owned = PyObjectRef; @@ -1552,7 +1513,8 @@ impl ToOwned for PyObject { fn to_owned(&self) -> Self::Owned { self.0.ref_count.inc(); PyObjectRef { - ptr: NonNull::from(self), + ptr: NonNull::from(self).cast(), + marker: PhantomData, } } } @@ -1566,7 +1528,8 @@ impl PyObject { pub fn try_to_owned(&self) -> Option { if self.0.ref_count.safe_inc() { Some(PyObjectRef { - ptr: NonNull::from(self), + ptr: NonNull::from(self).cast(), + marker: PhantomData, }) } else { None @@ -1590,6 +1553,7 @@ impl PyObject { if ref_count.safe_inc() { Some(PyObjectRef { ptr: unsafe { NonNull::new_unchecked(ptr) }, + marker: PhantomData, }) } else { None @@ -1621,7 +1585,10 @@ impl PyObjectRef { #[inline(always)] #[must_use] pub const unsafe fn from_raw(ptr: NonNull) -> Self { - Self { ptr } + Self { + ptr: ptr.cast(), + marker: PhantomData, + } } /// Attempt to downcast this reference to a subclass. @@ -1653,7 +1620,8 @@ impl PyObjectRef { // manual impl to avoid assertion let obj = ManuallyDrop::new(self); PyRef { - ptr: obj.ptr.cast(), + ptr: obj.ptr, + marker: PhantomData, } } @@ -1698,7 +1666,7 @@ impl PyObject { } else { let head = unsafe { &*head_ptr }; if head.ref_count.safe_inc() { - Some(unsafe { PyRef::from_raw(head_ptr) }.into()) + Some(unsafe { PyRef::::from_raw(head_ptr) }.into()) } else { None } @@ -2400,20 +2368,6 @@ impl PyObject { } } -impl Borrow for PyObjectRef { - #[inline(always)] - fn borrow(&self) -> &PyObject { - self - } -} - -impl AsRef for PyObjectRef { - #[inline(always)] - fn as_ref(&self) -> &PyObject { - self - } -} - impl<'a, T: PyPayload> From<&'a Py> for &'a PyObject { #[inline(always)] fn from(py_ref: &'a Py) -> Self { @@ -2421,15 +2375,6 @@ impl<'a, T: PyPayload> From<&'a Py> for &'a PyObject { } } -impl Drop for PyObjectRef { - #[inline] - fn drop(&mut self) { - if self.0.ref_count.dec() { - unsafe { PyObject::drop_slow(self.ptr) } - } - } -} - impl fmt::Debug for PyObject { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // SAFETY: the vtable contains functions that accept payload types that always match up @@ -2438,12 +2383,6 @@ impl fmt::Debug for PyObject { } } -impl fmt::Debug for PyObjectRef { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_object().fmt(f) - } -} - const STACKREF_BORROW_TAG: usize = 1; /// A tagged stack reference to a Python object. @@ -2655,14 +2594,15 @@ impl Py { } } -impl ToOwned for Py { +impl> ToOwned for Py { type Owned = PyRef; #[inline(always)] fn to_owned(&self) -> Self::Owned { self.ref_count.inc(); PyRef { - ptr: NonNull::from(self), + ptr: NonNull::from(self).cast(), + marker: PhantomData, } } } @@ -2721,6 +2661,63 @@ impl fmt::Debug for Py { } } +mod ref_target_sealed { + pub trait Sealed {} + impl Sealed for T {} + impl Sealed for super::PyObject {} + impl Sealed for crate::builtins::set::PyAnySet {} + impl Sealed for super::PyTuple> {} +} + +/// Selects the borrowed view of a Python reference. +/// +/// Payloads borrow `Py`; `PyObject` borrows the erased object itself. +/// This trait is sealed so safe code cannot choose an unrelated layout. +/// Typed tuples are immutable views of the ordinary tuple allocation. +/// Internal union views borrow an erased header and dispatch on the payload; +/// they must not assume that their alternative payloads have the same layout. +/// +/// An arbitrary borrowed layout cannot be registered by downstream code: +/// +/// ```compile_fail +/// use rustpython_vm::object::PyRefTarget; +/// struct Unrelated; +/// impl PyRefTarget for Unrelated { +/// type Target = u8; +/// } +/// ``` +pub trait PyRefTarget: ref_target_sealed::Sealed { + type Target; +} + +impl PyRefTarget for T { + type Target = Py; +} + +impl PyRefTarget for PyObject { + type Target = Self; +} + +/// A non-erased reference target that can be an element of a typed tuple view. +/// +/// This is sealed through [`PyRefTarget`]. Payloads, registered union views and +/// recursively typed tuples qualify; `PyObject` does not, since ordinary tuples +/// already implement [`PyPayload`]. This marker selects a layout, not a runtime +/// type check: checked element conversion still requires +/// [`crate::convert::TransmuteFromObject`]. +pub trait PyTypedRefTarget: PyRefTarget {} + +impl>> PyTypedRefTarget for T {} +impl PyTypedRefTarget for crate::builtins::set::PyAnySet {} + +impl PyRefTarget for crate::builtins::set::PyAnySet { + type Target = Self; +} + +impl PyRefTarget for PyTuple> { + type Target = Py; +} + /// A reference to a Python object. /// /// Note that a `PyRef` can only deref to a shared / immutable reference. @@ -2732,7 +2729,9 @@ impl fmt::Debug for Py { /// where a reference to the same object must be returned. #[repr(transparent)] pub struct PyRef { - ptr: NonNull>, + ptr: NonNull, + // Preserve the variance and auto traits of the former typed pointer. + marker: PhantomData>>, } cfg_select! { @@ -2743,17 +2742,17 @@ cfg_select! { _ => {} } -impl fmt::Debug for PyRef { +impl fmt::Debug for PyRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - (**self).fmt(f) + self.as_ref().fmt(f) } } impl Drop for PyRef { #[inline] fn drop(&mut self) { - if self.ref_count.dec() { - unsafe { PyObject::drop_slow(self.ptr.cast::()) } + if unsafe { self.ptr.as_ref() }.0.ref_count.dec() { + unsafe { PyObject::drop_slow(self.ptr) } } } } @@ -2761,14 +2760,48 @@ impl Drop for PyRef { impl Clone for PyRef { #[inline(always)] fn clone(&self) -> Self { - (**self).to_owned() + unsafe { self.ptr.as_ref() }.0.ref_count.inc(); + Self { + ptr: self.ptr, + marker: PhantomData, + } + } +} + +impl PyRef { + /// Transfers ownership from an erased reference to a registered view. + /// + /// # Safety + /// The object must satisfy all invariants of `T::Target`, including its + /// payload restrictions. Registering a target establishes its layout only; + /// the caller must validate this particular object before the transfer. + #[inline] + pub(crate) unsafe fn from_object_unchecked(object: PyObjectRef) -> Self { + let object = ManuallyDrop::new(object); + Self { + ptr: object.ptr, + marker: PhantomData, + } + } +} + +impl PyRef { + /// Erases the borrowed view, transferring the existing strong reference. + #[inline] + #[must_use] + pub fn into_object(self) -> PyObjectRef { + let object = ManuallyDrop::new(self); + PyObjectRef { + ptr: object.ptr, + marker: PhantomData, + } } } impl PyRef { #[inline(always)] pub(super) const fn into_non_null(self) -> NonNull> { - let ptr = self.ptr; + let ptr = self.ptr.cast(); core::mem::forget(self); ptr } @@ -2778,7 +2811,10 @@ impl PyRef { #[must_use] #[inline(always)] pub const unsafe fn from_non_null(ptr: NonNull>) -> Self { - Self { ptr } + Self { + ptr: ptr.cast(), + marker: PhantomData, + } } /// # Safety @@ -2792,15 +2828,13 @@ impl PyRef { #[inline(always)] unsafe fn from_obj_unchecked(obj: PyObjectRef) -> Self { debug_assert!(obj.downcast_ref::().is_some()); - let obj = ManuallyDrop::new(obj); - Self { - ptr: obj.ptr.cast(), - } + // SAFETY: the caller guarantees the payload type. + unsafe { Self::from_object_unchecked(obj) } } #[must_use] pub const fn leak(pyref: Self) -> &'static Py { - let ptr = pyref.ptr; + let ptr = pyref.ptr.cast::>(); core::mem::forget(pyref); unsafe { ptr.as_ref() } } @@ -2858,7 +2892,10 @@ impl PyRef { } } - Self { ptr } + Self { + ptr: ptr.cast(), + marker: PhantomData, + } } } @@ -2915,54 +2952,47 @@ impl Py { } } -impl Borrow for PyRef -where - T: PyPayload, -{ +impl Borrow for PyRef { #[inline(always)] fn borrow(&self) -> &PyObject { - (**self).as_object() + unsafe { self.ptr.as_ref() } } } -impl AsRef for PyRef -where - T: PyPayload, -{ +impl AsRef for PyRef { #[inline(always)] fn as_ref(&self) -> &PyObject { self.borrow() } } -impl From> for PyObjectRef { +impl>> From> for PyObjectRef { #[inline] fn from(value: PyRef) -> Self { - let me = ManuallyDrop::new(value); - Self { ptr: me.ptr.cast() } + value.into_object() } } -impl Borrow> for PyRef { +impl>> Borrow> for PyRef { #[inline(always)] fn borrow(&self) -> &Py { self } } -impl AsRef> for PyRef { +impl>> AsRef> for PyRef { #[inline(always)] fn as_ref(&self) -> &Py { self } } -impl Deref for PyRef { - type Target = Py; +impl Deref for PyRef { + type Target = T::Target; #[inline(always)] - fn deref(&self) -> &Py { - unsafe { self.ptr.as_ref() } + fn deref(&self) -> &Self::Target { + unsafe { self.ptr.cast::().as_ref() } } } @@ -3060,12 +3090,12 @@ pub(crate) fn init_type_hierarchy() -> BootstrapTypeHierarchy { } unsafe fn initial_ref(ptr: *mut Py) -> PyRef { - unsafe { PyRef::from_raw(ptr.cast()) } + unsafe { PyRef::::from_raw(ptr.cast()) } } unsafe fn clone_raw_ref(ptr: *mut Py) -> PyRef { unsafe { &*ptr::addr_of!((*ptr).ref_count) }.inc(); - unsafe { PyRef::from_raw(ptr.cast()) } + unsafe { PyRef::::from_raw(ptr.cast()) } } unsafe fn into_type_tuple(tuple: PyTupleRef) -> PyTypeTupleRef { @@ -3245,6 +3275,141 @@ pub(crate) fn init_type_hierarchy() -> BootstrapTypeHierarchy { mod tests { use super::*; + #[derive(Debug)] + struct RefProbe(alloc::sync::Arc); + + impl MaybeTraverse for RefProbe { + fn try_traverse(&self, _traverse_fn: &mut TraverseFn<'_>) {} + } + + impl PyPayload for RefProbe { + fn class(ctx: &crate::Context) -> &'static Py { + ctx.types.object_type + } + } + + impl Drop for RefProbe { + fn drop(&mut self) { + self.0.fetch_add(1, core::sync::atomic::Ordering::SeqCst); + } + } + + #[test] + fn pyref_object_layout_and_views() { + static_assertions::assert_type_eq_all!(PyObjectRef, PyRef); + static_assertions::assert_not_impl_any!(PyObject: PyPayload); + static_assertions::assert_impl_all!(PyRef: Unpin); + static_assertions::assert_eq_size!( + PyObjectRef, + NonNull, + Option, + PyRef, + Option>, + PyRef>> + ); + static_assertions::assert_eq_align!( + PyObjectRef, + NonNull, + PyRef, + PyRef>> + ); + #[cfg(feature = "threading")] + static_assertions::assert_impl_all!(PyRef: Send, Sync); + #[cfg(not(feature = "threading"))] + static_assertions::assert_not_impl_any!(PyRef: Send, Sync); + + fn erased_view(reference: &PyRef) -> &PyObject { + reference + } + fn payload_view(reference: &PyRef) -> &Py { + reference + } + fn tuple_view( + reference: &PyRef>>, + ) -> &Py>> { + reference + } + type NestedTuple = PyTuple>>>; + fn nested_view(reference: &PyRef) -> &Py { + reference + } + fn covariance<'long: 'short, 'short>(reference: PyRef<&'long ()>) -> PyRef<&'short ()> { + reference + } + fn into_object>() {} + into_object::>(); + let _ = ( + erased_view, + payload_view, + tuple_view, + nested_view, + covariance, + ); + } + + #[test] + fn pyref_object_ownership_and_raw_roundtrip() { + let drops = alloc::sync::Arc::new(core::sync::atomic::AtomicUsize::new(0)); + let ctx = crate::Context::genesis(); + let typed = PyRef::new_ref( + RefProbe(drops.clone()), + ctx.types.object_type.to_owned(), + None, + ); + let pointer = typed.as_object() as *const PyObject; + assert_eq!(typed.as_object().strong_count(), 1); + let erased: PyRef = typed.clone().into(); + assert_eq!(erased.strong_count(), 2); + assert!(core::ptr::eq(&*erased, pointer)); + let raw = erased.into_raw(); + assert_eq!(typed.as_object().strong_count(), 2); + let erased = unsafe { PyRef::::from_raw(raw) }; + let erased = erased.downcast::().unwrap_err(); + assert_eq!(erased.strong_count(), 2); + let restored = erased.downcast::().unwrap(); + assert!(core::ptr::eq(&*typed, &*restored)); + let raw = restored.into_non_null(); + let restored = unsafe { PyRef::::from_non_null(raw) }; + drop(typed); + assert_eq!(restored.as_object().strong_count(), 1); + assert_eq!(drops.load(core::sync::atomic::Ordering::SeqCst), 0); + drop(restored); + assert_eq!(drops.load(core::sync::atomic::Ordering::SeqCst), 1); + } + + #[test] + fn pyref_typed_tuple_erased_storage_drops_once() { + let ctx = crate::Context::genesis(); + // Both constructors transfer a Vec allocation into erased tuple storage. + for explicit_type in [false, true] { + let drops = alloc::sync::Arc::new(core::sync::atomic::AtomicUsize::new(0)); + let item = PyRef::new_ref( + RefProbe(drops.clone()), + ctx.types.object_type.to_owned(), + None, + ); + let mut elements = Vec::with_capacity(4); + elements.push(item.clone()); + let tuple = if explicit_type { + PyTuple::new_ref_typed_with_type(elements, ctx.types.tuple_type.to_owned()) + } else { + PyTuple::new_ref_typed(elements, ctx) + }; + assert_eq!(tuple.as_slice()[0].as_object().strong_count(), 2); + let copy = (*tuple).to_owned(); + let object: PyObjectRef = tuple.into(); + let untyped = copy.into_untyped(); + assert_eq!(object.strong_count(), 2); + assert!(core::ptr::eq(untyped.as_object(), &*object)); + assert_eq!(untyped.as_slice()[0].strong_count(), 2); + drop(object); + drop(item); + assert_eq!(drops.load(core::sync::atomic::Ordering::SeqCst), 0); + drop(untyped); + assert_eq!(drops.load(core::sync::atomic::Ordering::SeqCst), 1); + } + } + #[test] fn native_type_basicsize_includes_payload_padding() { use crate::class::PyClassDef; diff --git a/crates/vm/src/object/ext.rs b/crates/vm/src/object/ext.rs index 97309722a19..68fea76d4ff 100644 --- a/crates/vm/src/object/ext.rs +++ b/crates/vm/src/object/ext.rs @@ -360,7 +360,7 @@ impl PyAtomicRef { pub unsafe fn swap(&self, pyref: PyRef) -> PyRef { let py = PyRef::leak(pyref) as *const Py as *mut _; let old = Radium::swap(&self.inner, py, Ordering::AcqRel); - unsafe { PyRef::from_raw(old.cast()) } + unsafe { PyRef::::from_raw(old.cast()) } } pub fn swap_to_temporary_refs(&self, pyref: PyRef, vm: &VirtualMachine) { @@ -439,7 +439,11 @@ impl PyAtomicRef> { pub unsafe fn swap(&self, opt_ref: Option>) -> Option> { let val = opt_ref.map_or(null_mut(), |x| PyRef::leak(x) as *const Py as *mut _); let old = Radium::swap(&self.inner, val, Ordering::AcqRel); - unsafe { old.cast::>().as_ref().map(|x| PyRef::from_raw(x)) } + unsafe { + old.cast::>() + .as_ref() + .map(|x| PyRef::::from_raw(x)) + } } pub fn swap_to_temporary_refs(&self, opt_ref: Option>, vm: &VirtualMachine) { diff --git a/crates/vm/src/object/mod.rs b/crates/vm/src/object/mod.rs index 4f028338d4b..1921e3bec67 100644 --- a/crates/vm/src/object/mod.rs +++ b/crates/vm/src/object/mod.rs @@ -1,3 +1,78 @@ +//! Essential types for object models +//! +//! +-------------------------+--------------+-----------------------+ +//! | Management | Typed | Untyped | +//! +-------------------------+------------------+-------------------+ +//! | Interpreter-independent | [`Py`] | [`PyObject`] | +//! | Reference-counted | [`PyRef`] | [`PyObjectRef`] | +//! | Weak | [`PyWeakRef`] | [`PyRef`] | +//! +-------------------------+--------------+-----------------------+ +//! +//! [`PyRef`] may looking like to be called as PyObjectWeak by the rule, +//! but not to do to remember it is a PyRef object. +//! +//! # Reference ownership +//! +//! `PyObjectRef` is an alias for `PyRef`. Typed payload references, +//! erased references, union views, and typed tuple views share the same owning representation. +//! +//! ## Representation and invariants +//! +//! - `PyRef` stores `NonNull` and a zero-sized +//! `PhantomData>>`. The latter preserves the previous typed pointer's +//! covariance and auto-trait/`Unpin` shape. The transparent owner and its `Option` +//! remain one pointer wide with pointer alignment. +//! - The sealed `object::PyRefTarget` selects `Deref::Target`: payload `T` maps to +//! `Py`, `PyObject` maps to itself, and immutable typed tuples map recursively +//! to their existing typed `Py` view. Downstream safe code cannot register an +//! unrelated layout. `PyObject` does not implement `PyPayload`. +//! - `PyAnySet` is an internal transparent `PyObject` view. Its owner is +//! `PyRef`, whose construction validates set/frozenset (including +//! subclasses). Payload access downcasts to the actual alternative; neither +//! `Py` nor a shared set/frozenset payload layout is assumed. +//! `ArgIterable` instead stores a cached iterator function beside its owner, +//! so it remains a wrapper rather than an allocation view. +//! - [`PyTypedRefTarget`] admits non-erased element targets for typed tuple +//! views, including union views and recursively typed tuples. It does not +//! perform validation: `try_into_typed` checks every element before changing +//! the view. Typed constructors instead receive already-typed owners. +//! All tuple views retain erased element storage with transparent wrappers; +//! typed slices reinterpret only the guaranteed transparent reference +//! elements, never a generic `Vec` or `Box` representation. Private marshal +//! initialization finishes before publication; published elements are immutable. +//! - The owner itself has no new generic bound. `Clone` and `Drop` share one +//! implementation over the existing object header. Object allocation, vtable +//! destruction, weakrefs, GC, and trashcan handling are unchanged. +//! - Typed erasure uses `T: PyRefTarget>`. This excludes the erased +//! type from the conversion blanket, avoiding overlap with Rust's identity +//! `From for T`, while retaining typed and nested tuple conversions. +//! Union views provide their own checked conversions; `PyRef::into_object` +//! erases any owning view without adjusting its strong count. +//! - Ownership transfers use `ManuallyDrop` or `forget`, without an extra incref or +//! decrement. The existing raw APIs retain their ownership-transfer contracts. +//! - C-API typed output uses the same target equality. An erased owner supports +//! `*mut PyObject`, but cannot produce the fictitious `*mut Py` output. +//! Duplicate erased owner/member-layout/FFI implementations are removed. +//! - Feature-dependent `Send`/`Sync` behavior is retained. The non-threading erased +//! owner is neither; the threading owner implements both. +//! +//! ## API compatibility considerations +//! +//! Existing payload `PyRef` and `PyObjectRef` use remains supported, but aliasing +//! makes some inference less direct: +//! +//! - Distinct raw constructor signatures remain: erased +//! `PyObjectRef::from_raw(NonNull)` and typed +//! `PyRef::::from_raw(*const Py)`. Unqualified `PyRef::from_raw` can now be +//! ambiguous and require the explicit type parameter. +//! - Associated-target dereferencing can require an explicit downcast type or a +//! `PyRefTarget` equality bound in generic typed-view code. Arbitrary non-payload +//! views no longer automatically gain typed dereferencing; supported internal +//! views must be registered in the sealed selector. +//! - Rust `Debug` formatting now consistently uses the allocation's vtable rather +//! than the statically selected payload's formatter. This is a Rust diagnostic +//! difference, not a Python semantic assertion. + mod core; mod ext; mod payload;