From 2a427728a68e06d376e77e4e7b35442f891b2baa Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 11 Nov 2020 00:11:59 -0600 Subject: [PATCH 1/6] Use a custom vtable for PyObjectRef --- vm/src/builtins/code.rs | 2 +- vm/src/builtins/object.rs | 4 +- vm/src/builtins/pystr.rs | 5 +- vm/src/builtins/weakref.rs | 6 +- vm/src/bytesinner.rs | 12 +- vm/src/pyobject.rs | 325 ++++--------------- vm/src/pyobjectrc.rs | 618 ++++++++++++++++++++++++------------- vm/src/stdlib/io.rs | 2 +- vm/src/stdlib/itertools.rs | 8 +- vm/src/stdlib/pystruct.rs | 2 +- vm/src/stdlib/weakref.rs | 4 +- vm/src/sysmodule.rs | 5 +- vm/src/types.rs | 110 +------ 13 files changed, 492 insertions(+), 611 deletions(-) diff --git a/vm/src/builtins/code.rs b/vm/src/builtins/code.rs index 3b3d25ae3..c8af7f389 100644 --- a/vm/src/builtins/code.rs +++ b/vm/src/builtins/code.rs @@ -198,7 +198,7 @@ impl PyCode {} #[pyimpl] impl PyCodeRef { #[pyslot] - fn tp_new(_cls: PyTypeRef, vm: &VirtualMachine) -> PyResult> { + fn tp_new(_cls: PyTypeRef, vm: &VirtualMachine) -> PyResult { Err(vm.new_type_error("Cannot directly create code object".to_owned())) } diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index c27369cf4..decb916dd 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -217,10 +217,10 @@ impl PyBaseObject { #[pyproperty(name = "__class__", setter)] fn set_class(instance: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { if instance.payload_is::() { - match value.downcast_generic::() { + match value.downcast::() { Ok(cls) => { // FIXME(#1979) cls instances might have a payload - *instance.typ.write() = cls; + *instance.class_lock().write() = cls; Ok(()) } Err(value) => { diff --git a/vm/src/builtins/pystr.rs b/vm/src/builtins/pystr.rs index 45ebe0100..7f3c2aaf5 100644 --- a/vm/src/builtins/pystr.rs +++ b/vm/src/builtins/pystr.rs @@ -240,7 +240,10 @@ impl PyStr { if other.isinstance(&vm.ctx.types.str_type) { Ok(self.value.py_add(borrow_value(&other))) } else { - Err(vm.new_type_error(format!("Cannot add {} and {}", self, other))) + Err(vm.new_type_error(format!( + "can only concatenate str (not \"{}\") to str", + other.class().name + ))) } } diff --git a/vm/src/builtins/weakref.rs b/vm/src/builtins/weakref.rs index f98045b74..7046a55d5 100644 --- a/vm/src/builtins/weakref.rs +++ b/vm/src/builtins/weakref.rs @@ -2,9 +2,9 @@ use super::pytype::PyTypeRef; use crate::common::hash::PyHash; use crate::function::{FuncArgs, OptionalArg}; use crate::pyobject::{ - IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, + IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyObjectWeak, PyRef, PyResult, PyValue, + TypeProtocol, }; -use crate::pyobjectrc::{PyObjectRc, PyObjectWeak}; use crate::slots::{Callable, Comparable, Hashable, PyComparisonOp}; use crate::vm::VirtualMachine; @@ -20,7 +20,7 @@ pub struct PyWeak { impl PyWeak { pub fn downgrade(obj: &PyObjectRef) -> PyWeak { PyWeak { - referent: PyObjectRc::downgrade(obj), + referent: PyObjectRef::downgrade(obj), hash: AtomicCell::new(None), } } diff --git a/vm/src/bytesinner.rs b/vm/src/bytesinner.rs index 14dfee74e..90668eb99 100644 --- a/vm/src/bytesinner.rs +++ b/vm/src/bytesinner.rs @@ -14,8 +14,8 @@ use crate::byteslike::try_bytes_like; use crate::cformat::CFormatBytes; use crate::function::{OptionalArg, OptionalOption}; use crate::pyobject::{ - BorrowValue, Either, IdProtocol, PyComparisonValue, PyIterable, PyObjectRef, PyRef, PyResult, - PyValue, TryFromObject, TypeProtocol, + BorrowValue, Either, IdProtocol, PyComparisonValue, PyIterable, PyObjectRef, PyResult, PyValue, + TryFromObject, TypeProtocol, }; use crate::sliceable::PySliceableSequence; use crate::slots::PyComparisonOp; @@ -91,10 +91,10 @@ impl ByteInnerNewOptions { pub fn get_bytes(mut self, cls: PyTypeRef, vm: &VirtualMachine) -> PyResult { let inner = if let OptionalArg::Present(source) = self.source.take() { - if source.class().is(&PyBytes::class(vm)) && cls.is(&PyBytes::class(vm)) { - return self - .check_args(vm) - .map(|_| unsafe { PyRef::from_obj_unchecked(source) }); + if cls.is(&PyBytes::class(vm)) { + if let Ok(source) = source.clone().downcast_exact(vm) { + return self.check_args(vm).map(|()| source); + } } match_class!(match source { diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 716bcd6bd..a4277c0bb 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -37,7 +37,7 @@ use crate::dictdatatype::Dict; use crate::exceptions::{self, PyBaseExceptionRef}; use crate::function::{IntoFuncArgs, IntoPyNativeFunc}; use crate::iterator; -pub use crate::pyobjectrc::{PyObjectRc, PyObjectWeak}; +pub use crate::pyobjectrc::{PyObjectRef, PyObjectWeak, PyRef, PyWeakRef}; use crate::scope::Scope; use crate::slots::{PyTpFlags, PyTypeSlots}; use crate::types::{create_type_with_slots, TypeZoo}; @@ -57,13 +57,6 @@ Basically reference counting, but then done by rust. * Good reference: https://github.com/ProgVal/pythonvm-rust/blob/master/src/objects/mod.rs */ -/// The `PyObjectRef` is one of the most used types. It is a reference to a -/// python object. A single python object can have multiple references, and -/// 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. -pub type PyObjectRef = PyObjectRc; - /// Use this type for functions which return a python object or an exception. /// Both the python object and the python exception are `PyObjectRef` types /// since exceptions are also python objects. @@ -74,7 +67,8 @@ pub type PyResult = Result; // A valid v /// TODO: class attributes should maintain insertion order (use IndexMap here) pub type PyAttributes = HashMap; -impl fmt::Display for PyObject { +// TODO: remove this impl +impl fmt::Display for PyObjectRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if let Some(PyType { ref name, .. }) = self.payload::() { let type_name = self.class().name.clone(); @@ -378,8 +372,8 @@ impl PyContext { pub fn new_base_object(&self, class: PyTypeRef, dict: Option) -> PyObjectRef { PyObject { - typ: PyRwLock::new(class.into_typed_pyobj()), - dict: dict.map(|d| PyRwLock::new(d.into_typed_pyobj())), + typ: PyRwLock::new(class), + dict: dict.map(PyRwLock::new), payload: object::PyBaseObject, } .into_ref() @@ -408,156 +402,16 @@ impl Default for PyContext { /// This is an actual python object. It consists of a `typ` which is the /// python class, and carries some rust payload optionally. This rust /// payload can be a rust float or rust int in case of float and int objects. +#[repr(C)] pub struct PyObject where - T: ?Sized + PyObjectPayload, + T: ?Sized, { - pub(crate) typ: PyRwLock>, // __class__ member - pub(crate) dict: Option>>, // __dict__ member + pub(crate) typ: PyRwLock, // __class__ member + pub(crate) dict: Option>, // __dict__ member pub payload: T, } -impl PyObjectRef { - /// Attempt to downcast this reference to a subclass. - /// - /// If the downcast fails, the original ref is returned in as `Err` so - /// another downcast can be attempted without unnecessary cloning. - pub fn downcast(self) -> Result, Self> { - if self.payload_is::() { - Ok(unsafe { PyRef::from_obj_unchecked(self) }) - } else { - Err(self) - } - } - - /// Attempt to downcast this reference to the specific class that is associated `T`. - /// - /// If the downcast fails, the original ref is returned in as `Err` so - /// another downcast can be attempted without unnecessary cloning. - pub fn downcast_exact( - self, - vm: &VirtualMachine, - ) -> Result, Self> { - if self.class().is(T::class(vm)) { - // TODO: is this always true? - assert!( - self.payload_is::(), - "obj.__class__ is T::class() but payload is not T" - ); - Ok(unsafe { PyRef::from_obj_unchecked(self) }) - } else { - Err(self) - } - } - - /// Downcast this PyObjectRef to an `PyRc>`. The [`downcast`](#method.downcast) method - /// is generally preferred, as the `PyRef` it returns implements `Deref`, and - /// therefore can be used similarly to an `&T`. - pub fn downcast_generic(self) -> Result, Self> { - if self.payload_is::() { - let ptr = PyObjectRc::into_raw(self) as *const PyObject; - let ret = unsafe { PyObjectRc::from_raw(ptr) }; - Ok(ret) - } else { - Err(self) - } - } - - pub fn downcast_ref(&self) -> Option<&PyRef> { - if self.payload_is::() { - // when payload exacts, PyObjectRef == PyRef { PyObject } - Some(unsafe { &*(self as *const PyObjectRef as *const PyRef) }) - } else { - None - } - } -} - -/// A reference to a Python object. -/// -/// Note that a `PyRef` can only deref to a shared / immutable reference. -/// It is the payload type's responsibility to handle (possibly concurrent) -/// mutability with locks or concurrent data structures if required. -/// -/// A `PyRef` can be directly returned from a built-in function to handle -/// situations (such as when implementing in-place methods such as `__iadd__`) -/// where a reference to the same object must be returned. -#[derive(Debug)] -#[repr(transparent)] -pub struct PyRef { - // invariant: this obj must always have payload of type T - obj: PyObjectRef, - _payload: PhantomData, -} - -impl Clone for PyRef { - fn clone(&self) -> Self { - Self { - obj: self.obj.clone(), - _payload: PhantomData, - } - } -} - -impl PyRef { - #[allow(clippy::new_ret_no_self)] - pub fn new_ref(payload: T, typ: PyTypeRef, dict: Option) -> Self { - let obj = PyObject::new(payload, typ, dict); - // SAFETY: we just created the object from a payload of type T - unsafe { Self::from_obj_unchecked(obj) } - } - - fn from_obj(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if obj.payload_is::() { - // SAFETY: we just checked the payload - Ok(unsafe { Self::from_obj_unchecked(obj) }) - } else { - Err(vm.new_runtime_error(format!( - "Unexpected payload '{}' for type '{}'", - T::class(vm).name, - obj.class().name, - ))) - } - } - - /// Safety: payload type of `obj` must be `T` - pub(crate) unsafe fn from_obj_unchecked(obj: PyObjectRef) -> Self { - PyRef { - obj, - _payload: PhantomData, - } - } - - pub fn as_object(&self) -> &PyObjectRef { - &self.obj - } - - pub fn into_object(self) -> PyObjectRef { - self.obj - } - - pub fn into_typed_pyobj(self) -> PyObjectRc { - self.into_object().downcast_generic().unwrap() - } -} - -impl Deref for PyRef -where - T: PyValue, -{ - type Target = T; - - fn deref(&self) -> &T { - self.obj.payload().unwrap_or_else(|| { - if cfg!(debug_assertions) { - panic!("PyRef.obj did not have a payload of T") - } - // SAFETY: self.obj has invariant that payload is `T` - unsafe { std::hint::unreachable_unchecked() } - }) - } -} - impl TryFromObject for PyRef where T: PyValue, @@ -565,7 +419,13 @@ where fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { let class = T::class(vm); if obj.isinstance(class) { - PyRef::from_obj(obj, vm) + obj.downcast().map_err(|obj| { + vm.new_runtime_error(format!( + "Unexpected payload '{}' for type '{}'", + class.name, + obj.class().name, + )) + }) } else { let expected_type = &class.name; let actual_type = &obj.class().name; @@ -591,14 +451,14 @@ impl From> for PyObjectRef { impl fmt::Display for PyRef where - T: PyValue + fmt::Display, + T: PyObjectPayload + fmt::Display, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&**self, f) } } -pub struct PyRefExact { +pub struct PyRefExact { obj: PyRef, } impl TryFromObject for PyRefExact { @@ -607,9 +467,14 @@ impl TryFromObject for PyRefExact { let cls = obj.class(); if cls.is(target_cls) { drop(cls); - Ok(Self { - obj: PyRef::from_obj(obj, vm)?, - }) + let obj = obj.downcast().map_err(|obj| { + vm.new_runtime_error(format!( + "Unexpected payload '{}' for type '{}'", + target_cls.name, + obj.class().name, + )) + })?; + Ok(Self { obj }) } else if cls.issubclass(target_cls) { Err(vm.new_type_error(format!( "Expected an exact instance of '{}', not a subclass '{}'", @@ -685,21 +550,15 @@ pub trait IdProtocol { } } -impl IdProtocol for PyObject { +impl IdProtocol for PyRc { fn get_id(&self) -> usize { - self as *const _ as *const PyObject as usize - } -} - -impl IdProtocol for PyRc { - fn get_id(&self) -> usize { - (**self).get_id() + &**self as *const T as *const Never as usize } } impl IdProtocol for PyRef { fn get_id(&self) -> usize { - self.obj.get_id() + self.as_object().get_id() } } @@ -718,14 +577,14 @@ impl IdProtocol for &'_ T { /// A borrow of a reference to a Python object. This avoids having clone the `PyRef`/ /// `PyObjectRef`, which isn't that cheap as that increments the atomic reference counter. pub struct PyLease<'a, T: PyObjectPayload> { - inner: PyRwLockReadGuard<'a, PyObjectRc>, + inner: PyRwLockReadGuard<'a, PyRef>, } impl<'a, T: PyObjectPayload + PyValue> PyLease<'a, T> { // Associated function on purpose, because of deref #[allow(clippy::wrong_self_convention)] pub fn into_pyref(zelf: Self) -> PyRef { - zelf.inner.clone().into_pyref() + zelf.inner.clone() } } @@ -733,7 +592,7 @@ impl<'a, T: PyObjectPayload + PyValue> Deref for PyLease<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { - &self.inner.payload + &self.inner } } @@ -742,7 +601,7 @@ where T: PyValue + fmt::Display, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Display::fmt(&self.inner.payload, f) + fmt::Display::fmt(&**self, f) } } @@ -769,16 +628,7 @@ pub trait TypeProtocol { } } -impl TypeProtocol for PyObjectRef { - fn class(&self) -> PyLease<'_, PyType> { - (**self).class() - } -} - -impl TypeProtocol for PyObject -where - T: ?Sized + PyObjectPayload, -{ +impl TypeProtocol for PyObject { fn class(&self) -> PyLease<'_, PyType> { PyLease { inner: self.typ.read(), @@ -786,9 +636,9 @@ where } } -impl TypeProtocol for PyRef { +impl TypeProtocol for PyRef { fn class(&self) -> PyLease<'_, PyType> { - self.obj.class() + self.as_object().class() } } @@ -826,7 +676,7 @@ where } } -impl fmt::Debug for PyObject { +impl fmt::Debug for PyObject { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "[PyObj {:?}]", &self.payload) } @@ -931,11 +781,11 @@ impl TryFromObject for Option { /// Allows coercion of a types into PyRefs, so that we can write functions that can take /// refs, pyobject refs or basic types. -pub trait TryIntoRef { +pub trait TryIntoRef { fn try_into_ref(self, vm: &VirtualMachine) -> PyResult>; } -impl TryIntoRef for PyRef { +impl TryIntoRef for PyRef { fn try_into_ref(self, _vm: &VirtualMachine) -> PyResult> { Ok(self) } @@ -959,7 +809,7 @@ pub trait TryFromObject: Sized { fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult; } -pub trait IntoPyRef { +pub trait IntoPyRef { fn into_pyref(self, vm: &VirtualMachine) -> PyRef; } @@ -981,9 +831,9 @@ pub trait IntoPyObject { fn into_pyobject(self, vm: &VirtualMachine) -> PyObjectRef; } -impl IntoPyObject for PyRef { +impl IntoPyObject for PyRef { fn into_pyobject(self, _vm: &VirtualMachine) -> PyObjectRef { - self.obj + self.into_object() } } @@ -1039,8 +889,8 @@ where #[allow(clippy::new_ret_no_self)] pub fn new(payload: T, typ: PyTypeRef, dict: Option) -> PyObjectRef { PyObject { - typ: PyRwLock::new(typ.into_typed_pyobj()), - dict: dict.map(|d| PyRwLock::new(d.into_typed_pyobj())), + typ: PyRwLock::new(typ), + dict: dict.map(PyRwLock::new), payload, } .into_ref() @@ -1048,28 +898,20 @@ where // Move this object into a reference object, transferring ownership. pub fn into_ref(self) -> PyObjectRef { - let raw = PyObjectRc::into_raw(PyObjectRc::new(self)); - unsafe { PyObjectRef::from_raw(raw) } + PyObjectRef::new(self) } } -impl PyObject -where - T: ?Sized + PyObjectPayload, -{ - pub fn raw_payload(&self) -> &T { - &self.payload - } - +impl PyObject { pub fn dict(&self) -> Option { - self.dict.as_ref().map(|mu| mu.read().clone().into_pyref()) + self.dict.as_ref().map(|mu| mu.read().clone()) } /// Set the dict field. Returns `Err(dict)` if this object does not have a dict field /// in the first place. pub fn set_dict(&self, dict: PyDictRef) -> Result<(), PyDictRef> { match self.dict { Some(ref mu) => { - *mu.write() = dict.into_typed_pyobj(); + *mu.write() = dict; Ok(()) } None => Err(dict), @@ -1077,60 +919,6 @@ where } } -impl PyObject { - #[inline] - pub fn payload(&self) -> Option<&T> { - self.payload.as_any().downcast_ref() - } - - #[inline] - pub fn payload_is(&self) -> bool { - self.payload.as_any().is::() - } - - #[inline] - pub fn payload_if_exact( - &self, - vm: &VirtualMachine, - ) -> Option<&T> { - if self.class().is(T::class(vm)) { - self.payload() - } else { - None - } - } - - #[inline] - pub fn payload_if_subclass( - &self, - vm: &VirtualMachine, - ) -> Option<&T> { - if self.class().issubclass(T::class(vm)) { - self.payload() - } else { - None - } - } -} - -impl PyObjectRc -where - T: Sized + PyObjectPayload, -{ - pub fn into_pyref(self) -> PyRef - where - T: PyValue, - { - // SAFETY: we know just casted from PyRc> to PyObjectRef, so we know the - // payload is `T` - let raw = PyObjectRc::into_raw(self); - unsafe { - let rc = PyObjectRc::::from_raw(raw); - PyRef::from_obj_unchecked(rc) - } - } -} - cfg_if::cfg_if! { if #[cfg(feature = "threading")] { pub trait PyThreadingConstraint: Send + Sync {} @@ -1157,8 +945,8 @@ pub trait PyValue: fmt::Debug + PyThreadingConstraint + Sized + 'static { if cls.issubclass(class) { Ok(self.into_ref_with_type_unchecked(cls, vm)) } else { - let subtype = vm.to_str(&cls.obj)?; - let basetype = vm.to_str(&class.obj)?; + let subtype = vm.to_str(cls.as_object())?; + let basetype = vm.to_str(cls.as_object())?; Err(vm.new_type_error(format!("{} is not a subtype of {}", subtype, basetype))) } } @@ -1173,16 +961,9 @@ pub trait PyValue: fmt::Debug + PyThreadingConstraint + Sized + 'static { } } -pub trait PyObjectPayload: Any + fmt::Debug + PyThreadingConstraint + 'static { - fn as_any(&self) -> &dyn Any; -} +pub trait PyObjectPayload: Any + fmt::Debug + PyThreadingConstraint + 'static {} -impl PyObjectPayload for T { - #[inline] - fn as_any(&self) -> &dyn Any { - self - } -} +impl PyObjectPayload for T {} pub enum Either { A(A), @@ -1300,7 +1081,7 @@ pub trait StaticType { impl PyClassDef for PyRef where - T: PyClassDef, + T: PyObjectPayload + PyClassDef, { const NAME: &'static str = T::NAME; const MODULE_NAME: Option<&'static str> = T::MODULE_NAME; diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index 724172dbd..b81871233 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -1,80 +1,99 @@ use crate::common::rc::{PyRc, PyWeak}; -use crate::pyobject::{IdProtocol, PyObject, PyObjectPayload, TypeProtocol}; -use std::borrow; +use crate::pyobject::{self, IdProtocol, PyObject, PyObjectPayload, TypeProtocol}; +use crate::VirtualMachine; +use std::any::TypeId; use std::fmt; +use std::marker::PhantomData; +use std::mem::ManuallyDrop; use std::ops::Deref; -pub struct PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ - inner: PyRc>, +struct Erased; + +struct PyObjVTable { + drop: unsafe fn(*mut PyObject), + debug: unsafe fn(*const PyObject, &mut fmt::Formatter) -> fmt::Result, +} +unsafe fn drop_obj(x: *mut PyObject) { + std::ptr::drop_in_place(x as *mut PyObject) +} +unsafe fn debug_obj( + x: *const PyObject, + f: &mut fmt::Formatter, +) -> fmt::Result { + let x = &*x.cast::>(); + fmt::Debug::fmt(x, f) } -pub struct PyObjectWeak -where - T: ?Sized + PyObjectPayload, -{ - inner: PyWeak>, +macro_rules! make_vtable { + ($t:ty) => { + &PyObjVTable { + drop: drop_obj::<$t>, + debug: debug_obj::<$t>, + } + }; } -pub trait AsPyObjectRef { - fn _as_ref(self) -> PyRc>; +#[repr(C)] +struct PyInner { + // TODO: move typeid into vtable once TypeId::of is const + typeid: TypeId, + vtable: &'static PyObjVTable, + value: ManuallyDrop>, } -impl AsPyObjectRef for PyRc> -where - T: PyObjectPayload, -{ - fn _as_ref(self) -> PyRc> { - self +impl PyInner { + fn new(value: PyObject) -> Self { + PyInner { + typeid: TypeId::of::(), + vtable: make_vtable!(T), + value: ManuallyDrop::new(value), + } } } -impl AsPyObjectRef for PyRc> { - fn _as_ref(self) -> PyRc> { - self +impl Drop for PyInner { + fn drop(&mut self) { + let erased = &mut *self.value as *mut _ as *mut PyObject; + unsafe { (self.vtable.drop)(erased) } } } -impl PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ - pub fn into_raw(this: Self) -> *const PyObject { +/// The `PyObjectRef` is one of the most used types. It is a reference to a +/// python object. A single python object can have multiple references, and +/// 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. +#[derive(Clone)] +pub struct PyObjectRef { + inner: PyRc>, +} + +#[derive(Clone)] +pub struct PyObjectWeak { + inner: PyWeak>, +} + +pub enum RawPyObject {} + +impl PyObjectRef { + pub fn into_raw(this: Self) -> *const RawPyObject { let ptr = PyRc::as_ptr(&this.inner); std::mem::forget(this); - ptr - } - - unsafe fn into_rc(this: Self) -> PyRc> { - let raw = Self::into_raw(this); - PyRc::from_raw(raw) - } - - pub fn into_ref(this: Self) -> PyObjectRc { - PyObjectRc:: { - inner: unsafe { Self::into_rc(this) }._as_ref(), - } + ptr.cast() } /// # Safety /// See PyRc::from_raw - pub unsafe fn from_raw(ptr: *const PyObject) -> Self { + pub unsafe fn from_raw(ptr: *const RawPyObject) -> Self { Self { - inner: PyRc::from_raw(ptr), + inner: PyRc::from_raw(ptr.cast()), } } - pub fn new(value: PyObject) -> Self - where - T: Sized, - { - Self { - inner: PyRc::new(value), - } + pub fn new(value: PyObject) -> Self { + let inner = PyRc::into_raw(PyRc::new(PyInner::::new(value))); + let inner = unsafe { PyRc::from_raw(inner as *const PyInner) }; + Self { inner } } pub fn strong_count(this: &Self) -> usize { @@ -85,68 +104,136 @@ where PyRc::weak_count(&this.inner) } - pub fn downgrade(this: &Self) -> PyObjectWeak { + pub fn downgrade(this: &Self) -> PyObjectWeak { PyObjectWeak { inner: PyRc::downgrade(&this.inner), } } + + pub fn payload_is(&self) -> bool { + self.inner.typeid == TypeId::of::() + } + + pub fn payload(&self) -> Option<&T> { + if self.payload_is::() { + // we cast to a PyObject first because we don't know T's exact offset because of varying alignment, + // but we *do* know that PyObject is always + let pyobj = + unsafe { &*(&*self.inner.value as *const PyObject as *const PyObject) }; + Some(&pyobj.payload) + } else { + None + } + } + + /// Attempt to downcast this reference to a subclass. + /// + /// If the downcast fails, the original ref is returned in as `Err` so + /// another downcast can be attempted without unnecessary cloning. + pub fn downcast(self) -> Result, Self> { + if self.payload_is::() { + Ok(unsafe { PyRef::from_obj_unchecked(self) }) + } else { + Err(self) + } + } + + pub fn downcast_ref(&self) -> Option<&PyRef> { + if self.payload_is::() { + // when payload exacts, PyObjectRef == PyRef { PyObject } + Some(unsafe { &*(self as *const PyObjectRef as *const PyRef) }) + } else { + None + } + } + + pub(crate) fn class_lock(&self) -> &crate::common::lock::PyRwLock { + &self.inner.value.typ + } + + // ideally we'd be able to define these in pyobject.rs, but method visibility rules are weird + + /// Attempt to downcast this reference to the specific class that is associated `T`. + /// + /// If the downcast fails, the original ref is returned in as `Err` so + /// another downcast can be attempted without unnecessary cloning. + pub fn downcast_exact( + self, + vm: &VirtualMachine, + ) -> Result, Self> { + if self.class().is(T::class(vm)) { + // TODO: is this always true? + assert!( + self.payload_is::(), + "obj.__class__ is T::class() but payload is not T" + ); + Ok(unsafe { PyRef::from_obj_unchecked(self) }) + } else { + Err(self) + } + } + + #[inline] + pub fn payload_if_exact( + &self, + vm: &VirtualMachine, + ) -> Option<&T> { + if self.class().is(T::class(vm)) { + self.payload() + } else { + None + } + } + + pub fn dict(&self) -> Option { + self.inner.value.dict() + } + /// Set the dict field. Returns `Err(dict)` if this object does not have a dict field + /// in the first place. + pub fn set_dict( + &self, + dict: crate::builtins::PyDictRef, + ) -> Result<(), crate::builtins::PyDictRef> { + self.inner.value.set_dict(dict) + } + + #[inline] + pub fn payload_if_subclass( + &self, + vm: &crate::VirtualMachine, + ) -> Option<&T> { + if self.class().issubclass(T::class(vm)) { + self.payload() + } else { + None + } + } } -impl IdProtocol for PyObjectRc -where - PyRc>: IdProtocol + AsPyObjectRef, -{ +impl IdProtocol for PyObjectRef { fn get_id(&self) -> usize { self.inner.get_id() } } -impl PyObjectWeak -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ - pub fn upgrade(&self) -> Option> { - self.inner.upgrade().map(|inner| PyObjectRc { inner }) +impl PyObjectWeak { + pub fn upgrade(&self) -> Option { + self.inner.upgrade().map(|inner| PyObjectRef { inner }) } } -#[cfg(feature = "threading")] -unsafe impl Send for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ -} -#[cfg(feature = "threading")] -unsafe impl Sync for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ -} - -#[cfg(feature = "threading")] -unsafe impl Send for PyObjectWeak where T: ?Sized + PyObjectPayload {} -#[cfg(feature = "threading")] -unsafe impl Sync for PyObjectWeak where T: ?Sized + PyObjectPayload {} - -impl Drop for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ +impl Drop for PyObjectRef { fn drop(&mut self) { use crate::pyobject::BorrowValue; - // PyObjectRc will drop the value when its count goes to 0 + // PyObjectRef will drop the value when its count goes to 0 if PyRc::strong_count(&self.inner) != 1 { return; } // CPython-compatible drop implementation - let zelf = Self::into_ref(self.clone()); - if let Some(del_slot) = zelf.class().mro_find_map(|cls| cls.slots.del.load()) { + let zelf = self.clone(); + if let Some(del_slot) = self.class().mro_find_map(|cls| cls.slots.del.load()) { crate::vm::thread::with_vm(&zelf, |vm| { if let Err(e) = del_slot(&zelf, vm) { // exception in del will be ignored but printed @@ -169,146 +256,263 @@ where }); } - let _ = unsafe { PyObjectRc::::into_rc(zelf) }; - debug_assert!(PyRc::strong_count(&self.inner) == 1); // make sure to keep same state + // __del__ might have resurrected the object, but that's fine, strong_count would be >1 now } } -impl Deref for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ - type Target = PyObject; - - #[inline] - fn deref(&self) -> &PyObject { - self.inner.deref() +impl fmt::Debug for PyObjectRef { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + unsafe { (self.inner.vtable.debug)(&*self.inner.value, f) } } } -impl Clone for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, -{ +impl fmt::Debug for PyObjectWeak { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "(PyWeak)") + } +} + +/// A reference to a Python object. +/// +/// Note that a `PyRef` can only deref to a shared / immutable reference. +/// It is the payload type's responsibility to handle (possibly concurrent) +/// mutability with locks or concurrent data structures if required. +/// +/// A `PyRef` can be directly returned from a built-in function to handle +/// situations (such as when implementing in-place methods such as `__iadd__`) +/// where a reference to the same object must be returned. +#[derive(Debug)] +#[repr(transparent)] +pub struct PyRef { + // invariant: this obj must always have payload of type T + obj: PyObjectRef, + _payload: PhantomData>, +} + +impl Clone for PyRef { fn clone(&self) -> Self { - PyObjectRc { - inner: self.inner.clone(), + Self { + obj: self.obj.clone(), + _payload: PhantomData, } } } -impl fmt::Display for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, - PyObject: fmt::Display, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) - } -} - -impl fmt::Debug for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, - PyObject: fmt::Debug, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) - } -} - -impl fmt::Pointer for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef, - PyObject: fmt::Pointer, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) - } -} - -impl borrow::Borrow for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef + borrow::Borrow, -{ - fn borrow(&self) -> &T { - self.inner.borrow() - } -} - -impl borrow::BorrowMut for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef + borrow::BorrowMut, -{ - fn borrow_mut(&mut self) -> &mut T { - self.inner.borrow_mut() - } -} - -impl AsRef for PyObjectRc -where - T: ?Sized + PyObjectPayload, - PyRc>: AsPyObjectRef + AsRef, -{ - fn as_ref(&self) -> &T { - self.inner.as_ref() - } -} - -impl Clone for PyObjectWeak -where - T: ?Sized + PyObjectPayload, -{ - fn clone(&self) -> Self { - PyObjectWeak { - inner: self.inner.clone(), +impl PyRef { + /// Safety: payload type of `obj` must be `T` + unsafe fn from_obj_unchecked(obj: PyObjectRef) -> Self { + PyRef { + obj, + _payload: PhantomData, } } -} -impl fmt::Debug for PyObjectWeak -where - T: ?Sized + PyObjectPayload, - PyObject: fmt::Debug, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) + #[inline(always)] + pub fn as_object(&self) -> &PyObjectRef { + &self.obj + } + + #[inline(always)] + pub fn into_object(self) -> PyObjectRef { + self.obj + } + + pub fn downgrade(this: &Self) -> PyWeakRef { + PyWeakRef { + weak: PyObjectRef::downgrade(&this.obj), + _payload: PhantomData, + } + } + + // ideally we'd be able to define this in pyobject.rs, but method visibility rules are weird + #[allow(clippy::new_ret_no_self)] + pub fn new_ref( + payload: T, + typ: crate::builtins::PyTypeRef, + dict: Option, + ) -> Self { + let obj = PyObject::new(payload, typ, dict); + // SAFETY: we just created the object from a payload of type T + unsafe { Self::from_obj_unchecked(obj) } } } -impl borrow::Borrow for PyObjectWeak +impl Deref for PyRef where - T: ?Sized + PyObjectPayload, - PyWeak>: borrow::Borrow, + T: PyObjectPayload, { - fn borrow(&self) -> &T { - self.inner.borrow() + type Target = T; + + fn deref(&self) -> &T { + let obj = + unsafe { &*(&*self.obj.inner.value as *const PyObject as *const PyObject) }; + &obj.payload } } -impl borrow::BorrowMut for PyObjectWeak -where - T: ?Sized + PyObjectPayload, - PyWeak>: borrow::BorrowMut, -{ - fn borrow_mut(&mut self) -> &mut T { - self.inner.borrow_mut() +pub struct PyWeakRef { + weak: PyObjectWeak, + _payload: PhantomData>, +} + +impl PyWeakRef { + pub fn upgrade(&self) -> Option> { + self.weak.upgrade().map(|obj| unsafe { + // SAFETY: PyWeakRef is only ever created from a PyRef + PyRef::from_obj_unchecked(obj) + }) } } -impl AsRef for PyObjectWeak -where - T: ?Sized + PyObjectPayload, - PyWeak>: AsRef, -{ - fn as_ref(&self) -> &T { - self.inner.as_ref() +impl TypeProtocol for PyObjectRef { + fn class(&self) -> pyobject::PyLease<'_, crate::builtins::PyType> { + self.inner.value.class() + } +} + +/// Paritally initialize a struct, ensuring that all fields are +/// either given values or explicitly left uninitialized +macro_rules! partially_init { + ( + $ty:path {$($init_field:ident: $init_value:expr),*$(,)?}, + Uninit { $($uninit_field:ident),*$(,)? }$(,)? + ) => {{ + // check all the fields are there but *don't* actually run it + if false { + #[allow(invalid_value, dead_code, unreachable_code)] + let _ = {$ty { + $($init_field: $init_value,)* + $($uninit_field: unreachable!(),)* + }}; + } + let mut m = ::std::mem::MaybeUninit::<$ty>::uninit(); + #[allow(unused_unsafe)] + unsafe { + $(::std::ptr::write(&mut (*m.as_mut_ptr()).$init_field, $init_value);)* + } + m + }}; +} + +use crate::builtins::PyTypeRef; +pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { + use crate::builtins::{object, PyType, PyWeak}; + use crate::common::lock::PyRwLock; + use crate::pyobject::{PyAttributes, PyClassDef, PyClassImpl}; + use std::mem::MaybeUninit; + use std::ptr; + + // `type` inherits from `object` + // and both `type` and `object are instances of `type`. + // to produce this circular dependency, we need an unsafe block. + // (and yes, this will never get dropped. TODO?) + let (type_type, object_type) = { + type PyTypeObj = PyObject; + type UninitRef = PyRwLock>>>; + + let type_payload = PyType { + name: PyTypeRef::NAME.to_owned(), + base: None, + bases: vec![], + mro: vec![], + subclasses: PyRwLock::default(), + attributes: PyRwLock::new(PyAttributes::new()), + slots: PyType::make_slots(), + }; + let object_payload = PyType { + name: object::PyBaseObject::NAME.to_owned(), + base: None, + bases: vec![], + mro: vec![], + subclasses: PyRwLock::default(), + attributes: PyRwLock::new(PyAttributes::new()), + slots: object::PyBaseObject::make_slots(), + }; + let type_type = PyRc::new(partially_init!( + PyInner:: { + typeid: TypeId::of::(), + vtable: make_vtable!(PyType), + }, + Uninit { value } + )); + let object_type = PyRc::new(partially_init!( + PyInner:: { + typeid: TypeId::of::(), + vtable: make_vtable!(PyType), + // dict: None, + // payload: object_payload, + }, + Uninit { value }, + )); + + let object_type_ptr = PyRc::into_raw(object_type) as *mut MaybeUninit> + as *mut PyInner; + let type_type_ptr = PyRc::into_raw(type_type.clone()) as *mut MaybeUninit> + as *mut PyInner; + + unsafe { + // TODO: make this part of the partially_init!() method + // partially initialize the inner PyObject of PyInner + std::ptr::write( + &mut (*type_type_ptr).value as *mut ManuallyDrop + as *mut MaybeUninit, + partially_init!( + PyTypeObj { + dict: None, + payload: type_payload, + }, + Uninit { typ } + ), + ); + std::ptr::write( + &mut (*object_type_ptr).value as *mut ManuallyDrop + as *mut MaybeUninit, + partially_init!( + PyTypeObj { + dict: None, + payload: object_payload, + }, + Uninit { typ } + ), + ); + + ptr::write( + &mut (*object_type_ptr).value.typ as *mut PyRwLock + as *mut UninitRef, + PyRwLock::new(type_type.clone()), + ); + ptr::write( + &mut (*type_type_ptr).value.typ as *mut PyRwLock + as *mut UninitRef, + PyRwLock::new(type_type), + ); + + let type_type = + PyTypeRef::from_obj_unchecked(PyObjectRef::from_raw(type_type_ptr.cast())); + let object_type = + PyTypeRef::from_obj_unchecked(PyObjectRef::from_raw(object_type_ptr.cast())); + + (*type_type_ptr).value.payload.mro = vec![object_type.clone()]; + (*type_type_ptr).value.payload.bases = vec![object_type.clone()]; + (*type_type_ptr).value.payload.base = Some(object_type.clone()); + + (type_type, object_type) + } + }; + + object_type + .subclasses + .write() + .push(PyWeak::downgrade(&type_type.as_object())); + + (type_type, object_type) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn test_type_initialization() { + let _ = init_type_hierarchy(); } } diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index 0595ed027..b972d82f8 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -809,7 +809,7 @@ mod _io { .into_pyobject(vm); // TODO: loop if write() raises an interrupt - call_method(vm, self.raw.as_ref().unwrap(), "write", (memobj.clone(),))? + call_method(vm, self.raw.as_ref().unwrap(), "write", (memobj,))? } else { let options = BufferOptions { len, diff --git a/vm/src/stdlib/itertools.rs b/vm/src/stdlib/itertools.rs index 817fb124d..0014c82d0 100644 --- a/vm/src/stdlib/itertools.rs +++ b/vm/src/stdlib/itertools.rs @@ -16,8 +16,8 @@ mod decl { use crate::function::{Args, FuncArgs, OptionalArg, OptionalOption}; use crate::iterator::{call_next, get_all, get_iter, get_next_object}; use crate::pyobject::{ - BorrowValue, IdProtocol, IntoPyObject, PyCallable, PyObjectRc, PyObjectRef, PyObjectWeak, - PyRef, PyResult, PyValue, StaticType, TypeProtocol, + BorrowValue, IdProtocol, IntoPyObject, PyCallable, PyObjectRef, PyRef, PyResult, PyValue, + PyWeakRef, StaticType, TypeProtocol, }; use crate::slots::PyIter; use crate::vm::VirtualMachine; @@ -458,7 +458,7 @@ mod decl { current_value: Option, current_key: Option, next_group: bool, - grouper: Option>, + grouper: Option>, } impl fmt::Debug for GroupByState { @@ -573,7 +573,7 @@ mod decl { } .into_ref(vm); - state.grouper = Some(PyObjectRc::downgrade(&grouper.clone().into_typed_pyobj())); + state.grouper = Some(PyRef::downgrade(&grouper)); Ok((state.current_key.as_ref().unwrap().clone(), grouper).into_pyobject(vm)) } } diff --git a/vm/src/stdlib/pystruct.rs b/vm/src/stdlib/pystruct.rs index 343dd469c..beaaca6b3 100644 --- a/vm/src/stdlib/pystruct.rs +++ b/vm/src/stdlib/pystruct.rs @@ -445,7 +445,7 @@ pub(crate) mod _struct { where T: num_traits::PrimInt + for<'a> std::convert::TryFrom<&'a BigInt>, { - match vm.to_index_opt(arg.clone()) { + match vm.to_index_opt(arg) { Some(index) => try_to_primitive(index?.borrow_value(), vm), None => Err(new_struct_error( vm, diff --git a/vm/src/stdlib/weakref.rs b/vm/src/stdlib/weakref.rs index 4622a72b1..e256e7c0a 100644 --- a/vm/src/stdlib/weakref.rs +++ b/vm/src/stdlib/weakref.rs @@ -5,11 +5,11 @@ //! - [rust weak struct](https://doc.rust-lang.org/std/rc/struct.Weak.html) //! -use crate::pyobject::{PyObjectRc, PyObjectRef}; +use crate::pyobject::PyObjectRef; use crate::vm::VirtualMachine; fn weakref_getweakrefcount(obj: PyObjectRef) -> usize { - PyObjectRc::weak_count(&obj) + PyObjectRef::weak_count(&obj) } fn weakref_getweakrefs(_obj: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef { diff --git a/vm/src/sysmodule.rs b/vm/src/sysmodule.rs index 3513f7fd9..59b028d47 100644 --- a/vm/src/sysmodule.rs +++ b/vm/src/sysmodule.rs @@ -6,8 +6,7 @@ use crate::common::hash::{PyHash, PyUHash}; use crate::frame::FrameRef; use crate::function::{Args, FuncArgs, OptionalArg}; use crate::pyobject::{ - ItemProtocol, PyClassImpl, PyContext, PyObjectRc, PyObjectRef, PyRefExact, PyResult, - PyStructSequence, + ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyRefExact, PyResult, PyStructSequence, }; use crate::vm::{PySettings, VirtualMachine}; use crate::{builtins, exceptions, py_io, version}; @@ -137,7 +136,7 @@ impl SysFlags { } fn sys_getrefcount(obj: PyObjectRef) -> usize { - PyObjectRc::strong_count(&obj) + PyObjectRef::strong_count(&obj) } fn sys_getsizeof(obj: PyObjectRef) -> usize { diff --git a/vm/src/types.rs b/vm/src/types.rs index 67e97e251..834a9e01d 100644 --- a/vm/src/types.rs +++ b/vm/src/types.rs @@ -38,13 +38,8 @@ use crate::builtins::tuple; use crate::builtins::weakproxy; use crate::builtins::weakref; use crate::builtins::zip; -use crate::pyobject::{ - PyAttributes, PyClassDef, PyClassImpl, PyContext, PyObject, PyObjectRc, PyObjectRef, StaticType, -}; +use crate::pyobject::{PyAttributes, PyContext, StaticType}; use crate::slots::PyTypeSlots; -use rustpython_common::{lock::PyRwLock, rc::PyRc}; -use std::mem::MaybeUninit; -use std::ptr; /// Holder of references to builtin types. #[derive(Debug, Clone)] @@ -121,7 +116,7 @@ pub struct TypeZoo { impl TypeZoo { pub(crate) fn init() -> Self { - let (type_type, object_type) = init_type_hierarchy(); + let (type_type, object_type) = crate::pyobjectrc::init_type_hierarchy(); Self { // the order matters for type, object and int type_type: pytype::PyType::init_manually(type_type).clone(), @@ -269,104 +264,3 @@ pub fn create_type_with_slots( ) .expect("Failed to create a new type in internal code.") } - -/// Paritally initialize a struct, ensuring that all fields are -/// either given values or explicitly left uninitialized -macro_rules! partially_init { - ( - $ty:path {$($init_field:ident: $init_value:expr),*$(,)?}, - Uninit { $($uninit_field:ident),*$(,)? }$(,)? - ) => {{ - // check all the fields are there but *don't* actually run it - if false { - #[allow(invalid_value, dead_code, unreachable_code)] - let _ = {$ty { - $($init_field: $init_value,)* - $($uninit_field: unreachable!(),)* - }}; - } - let mut m = ::std::mem::MaybeUninit::<$ty>::uninit(); - unsafe { - $(::std::ptr::write(&mut (*m.as_mut_ptr()).$init_field, $init_value);)* - } - m - }}; -} - -fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { - // `type` inherits from `object` - // and both `type` and `object are instances of `type`. - // to produce this circular dependency, we need an unsafe block. - // (and yes, this will never get dropped. TODO?) - let (type_type, object_type) = { - type PyTypeObj = PyObject; - type UninitRef = PyRwLock>>>; - - let type_payload = PyType { - name: PyTypeRef::NAME.to_owned(), - base: None, - bases: vec![], - mro: vec![], - subclasses: PyRwLock::default(), - attributes: PyRwLock::new(PyAttributes::new()), - slots: PyType::make_slots(), - }; - let object_payload = PyType { - name: object::PyBaseObject::NAME.to_owned(), - base: None, - bases: vec![], - mro: vec![], - subclasses: PyRwLock::default(), - attributes: PyRwLock::new(PyAttributes::new()), - slots: object::PyBaseObject::make_slots(), - }; - let type_type: PyRc> = PyRc::new(partially_init!( - PyObject:: { - dict: None, - payload: type_payload, - }, - Uninit { typ } - )); - let object_type: PyRc> = PyRc::new(partially_init!( - PyObject:: { - dict: None, - payload: object_payload, - }, - Uninit { typ }, - )); - - let object_type_ptr = - PyRc::into_raw(object_type) as *mut MaybeUninit as *mut PyTypeObj; - let type_type_ptr = - PyRc::into_raw(type_type.clone()) as *mut MaybeUninit as *mut PyTypeObj; - - unsafe { - ptr::write( - &mut (*object_type_ptr).typ as *mut PyRwLock> - as *mut UninitRef, - PyRwLock::new(type_type.clone()), - ); - ptr::write( - &mut (*type_type_ptr).typ as *mut PyRwLock> - as *mut UninitRef, - PyRwLock::new(type_type), - ); - - let type_type = PyTypeRef::from_obj_unchecked(PyObjectRef::from_raw(type_type_ptr)); - let object_type = PyTypeRef::from_obj_unchecked(PyObjectRef::from_raw(object_type_ptr)); - - (*type_type_ptr).payload.mro = vec![object_type.clone()]; - (*type_type_ptr).payload.bases = vec![object_type.clone()]; - (*type_type_ptr).payload.base = Some(object_type.clone()); - - (type_type, object_type) - } - }; - - object_type - .subclasses - .write() - .push(weakref::PyWeak::downgrade(&type_type.as_object())); - - (type_type, object_type) -} From fafee7fe0dd1e81e9d88c215ce1bdae661c3f62c Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Tue, 17 Nov 2020 09:47:33 -0600 Subject: [PATCH 2/6] Use static_assertions to verify the safety of init_type_hierarchy --- Cargo.lock | 1 + vm/Cargo.toml | 1 + vm/src/pyobjectrc.rs | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index fbd14826f..079c2c3e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1894,6 +1894,7 @@ dependencies = [ "sha2", "sha3", "socket2", + "static_assertions", "statrs", "thiserror", "thread_local", diff --git a/vm/Cargo.toml b/vm/Cargo.toml index 50b64eef8..a5946e835 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -79,6 +79,7 @@ cfg-if = "0.1.10" timsort = "0.1" thiserror = "1.0" atty = "0.2" +static_assertions = "1.1" ## unicode stuff unicode_names2 = "0.4" diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index b81871233..80115a0af 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -410,6 +410,11 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { type PyTypeObj = PyObject; type UninitRef = PyRwLock>>>; + // We cast between these 2 types, so make sure (at compile time) that there's no change in + // layout when we wrap PyInner in MaybeUninit<> + static_assertions::assert_eq_size!(MaybeUninit>, PyInner); + static_assertions::assert_eq_align!(MaybeUninit>, PyInner); + let type_payload = PyType { name: PyTypeRef::NAME.to_owned(), base: None, From 0d50118e44f09144d45ef8cc257de3f2fb9c1821 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Tue, 17 Nov 2020 13:47:40 -0600 Subject: [PATCH 3/6] Run tests under miri on CI --- .github/workflows/ci.yaml | 15 +++++++++++++++ vm/src/builtins/dict.rs | 6 +++--- vm/src/pyobjectrc.rs | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f60631075..a29e303e6 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -161,6 +161,21 @@ jobs: run: python -m pip install flake8 - name: run lint run: flake8 . --count --exclude=./.*,./Lib,./vm/Lib --select=E9,F63,F7,F82 --show-source --statistics + miri: + name: Run tests under miri + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + components: miri + override: true + - name: Run tests under miri + # miri-ignore-leaks because the type-object circular reference means that there will always be + # a memory leak, at least until we have proper cyclic gc + run: MIRIFLAGS='-Zmiri-ignore-leaks' cargo +nightly miri test -p rustpython-vm -- miri_test wasm: name: Check the WASM package and demo diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index d716a2fd0..9d45bfaff 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -597,7 +597,7 @@ macro_rules! dict_iterator { let s = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { let mut str_parts = vec![]; for (key, value) in zelf.dict.clone() { - let s = vm.to_repr(&$result_fn(vm, key, value))?; + let s = vm.to_repr(&($result_fn)(vm, key, value))?; str_parts.push(s.borrow_value().to_owned()); } format!("{}([{}])", $class_name, str_parts.join(", ")) @@ -694,7 +694,7 @@ macro_rules! dict_iterator { match zelf.dict.entries.next_entry(&mut position) { Some((key, value)) => { zelf.position.store(position); - Ok($result_fn(vm, key, value)) + Ok(($result_fn)(vm, key, value)) } None => Err(vm.new_stop_iteration()), } @@ -743,7 +743,7 @@ macro_rules! dict_iterator { match zelf.dict.len().checked_sub(count) { Some(mut pos) => { let (key, value) = zelf.dict.entries.next_entry(&mut pos).unwrap(); - Ok($result_fn(vm, key, value)) + Ok(($result_fn)(vm, key, value)) } None => { zelf.position.store(std::isize::MAX as usize); diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index 80115a0af..f60718929 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -517,7 +517,7 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { mod tests { use super::*; #[test] - fn test_type_initialization() { + fn miri_test_type_initialization() { let _ = init_type_hierarchy(); } } From 8cb1cfa56d940cc1fa20da2c3166e9b13d478a15 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Tue, 17 Nov 2020 14:22:26 -0600 Subject: [PATCH 4/6] Add a bunch of comments --- vm/src/pyobjectrc.rs | 62 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index f60718929..ca4eac429 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -7,6 +7,40 @@ use std::marker::PhantomData; use std::mem::ManuallyDrop; use std::ops::Deref; +// so, PyObjectRef is basically equivalent to `PyRc>`, except it's +// only one pointer in width rather than 2. We do that by manually creating a vtable, and putting +// a &'static reference to it inside the `PyRc` rather than adjacent to it, like trait objects do. +// This can lead to faster code since there's just less data to pass around, as well as because of +// some weird stuff with trait objects, alignment, and padding. +// +// So, every type has an alignment, which means that if you create a value of it it's location in +// memory has to be a multiple of it's alignment. e.g., a type with alignment 4 (like i32) could be +// at 0xb7befbc0, 0xb7befbc4, or 0xb7befbc8, but not 0xb7befbc2. If you have a struct and there are +// 2 fields whose sizes/alignments don't perfectly fit in with each other, e.g.: +// +-------------+-------------+---------------------------+ +// | u16 | ? | i32 | +// | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 | +// +-------------+-------------+---------------------------+ +// There has to be padding in the space between the 2 fields. But, if that field is a trait object +// (like `dyn PyObjectPayload`) we don't *know* how much padding there is between the `payload` +// field and the previous field. So, Rust has to consult the vtable to know the exact offset of +// `payload` in `PyObject`, which has a huge performance impact when *every +// single payload access* requires a vtable lookup. Thankfully, we're able to avoid that because of +// the way we use PyObjectRef, in that whenever we want to access the payload we (almost) always +// access it from a generic function. So, rather than doing +// +// - check vtable for payload offset +// - get offset in PyObject struct +// - call as_any() method of PyObjectPayload +// - call downcast_ref() method of Any +// we can just do +// - check vtable that typeid matches +// - pointer cast directly to *const PyObject +// +// and at that point the compiler can know the offset of `payload` for us because **we've given it a +// concrete type to work with before we ever access the `payload` field** + +/// A type to just represent "we've erased the type of this object, cast it before you use it" struct Erased; struct PyObjVTable { @@ -54,6 +88,8 @@ impl PyInner { impl Drop for PyInner { fn drop(&mut self) { let erased = &mut *self.value as *mut _ as *mut PyObject; + // SAFETY: the vtable contains functions that accept payload types that always match up + // with the payload of the object unsafe { (self.vtable.drop)(erased) } } } @@ -64,15 +100,19 @@ impl Drop for PyInner { /// method to create a new reference and increment the amount of references /// to the python object by 1. #[derive(Clone)] +#[repr(transparent)] pub struct PyObjectRef { inner: PyRc>, } #[derive(Clone)] +#[repr(transparent)] pub struct PyObjectWeak { inner: PyWeak>, } +/// A marker type that just references a raw python object. Don't use directly, pass as a pointer +/// back to [`PyObjectRef::from_raw`] pub enum RawPyObject {} impl PyObjectRef { @@ -83,7 +123,10 @@ impl PyObjectRef { } /// # Safety - /// See PyRc::from_raw + /// The raw pointer must have been previously returned from a call to + /// [`PyObjectRef::into_raw`]. The user is responsible for ensuring that the inner data is not + /// dropped more than once due to mishandling the reference count by calling this function + /// too many times. pub unsafe fn from_raw(ptr: *const RawPyObject) -> Self { Self { inner: PyRc::from_raw(ptr.cast()), @@ -116,8 +159,8 @@ impl PyObjectRef { pub fn payload(&self) -> Option<&T> { if self.payload_is::() { - // we cast to a PyObject first because we don't know T's exact offset because of varying alignment, - // but we *do* know that PyObject is always + // we cast to a PyObject first because we don't know T's exact offset because of + // varying alignment, but once we get a PyObject the compiler can get it for us let pyobj = unsafe { &*(&*self.inner.value as *const PyObject as *const PyObject) }; Some(&pyobj.payload) @@ -140,7 +183,8 @@ impl PyObjectRef { pub fn downcast_ref(&self) -> Option<&PyRef> { if self.payload_is::() { - // when payload exacts, PyObjectRef == PyRef { PyObject } + // SAFETY: just checked that the payload is T, and PyRef is repr(transparent) over + // PyObjectRef Some(unsafe { &*(self as *const PyObjectRef as *const PyRef) }) } else { None @@ -167,6 +211,7 @@ impl PyObjectRef { self.payload_is::(), "obj.__class__ is T::class() but payload is not T" ); + // SAFETY: just asserted that payload_is::() Ok(unsafe { PyRef::from_obj_unchecked(self) }) } else { Err(self) @@ -256,12 +301,15 @@ impl Drop for PyObjectRef { }); } - // __del__ might have resurrected the object, but that's fine, strong_count would be >1 now + // __del__ might have resurrected the object at this point, but that's fine, + // inner.strong_count would be >1 now and it'll maybe get dropped the next time } } impl fmt::Debug for PyObjectRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // SAFETY: the vtable contains functions that accept payload types that always match up + // with the payload of the object unsafe { (self.inner.vtable.debug)(&*self.inner.value, f) } } } @@ -325,7 +373,6 @@ impl PyRef { } // ideally we'd be able to define this in pyobject.rs, but method visibility rules are weird - #[allow(clippy::new_ret_no_self)] pub fn new_ref( payload: T, typ: crate::builtins::PyTypeRef, @@ -344,12 +391,15 @@ where type Target = T; fn deref(&self) -> &T { + // SAFETY: per the invariant on `self.obj`, the payload of the pyobject is always T, so it + // can always be cast to a PyObject let obj = unsafe { &*(&*self.obj.inner.value as *const PyObject as *const PyObject) }; &obj.payload } } +#[repr(transparent)] pub struct PyWeakRef { weak: PyObjectWeak, _payload: PhantomData>, From 7a5ba0efcc43f35034950786f07fb425ca255376 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Thu, 19 Nov 2020 12:46:49 -0600 Subject: [PATCH 5/6] Update pytype to be a bit more idiomatic --- vm/src/builtins/pytype.rs | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/vm/src/builtins/pytype.rs b/vm/src/builtins/pytype.rs index cdbe2aec5..26262a0e6 100644 --- a/vm/src/builtins/pytype.rs +++ b/vm/src/builtins/pytype.rs @@ -727,24 +727,22 @@ pub fn tp_new_wrapper( call_tp_new(zelf, cls, args, vm) } -fn take_next_base(mut bases: Vec>) -> (Option, Vec>) { - bases = bases.into_iter().filter(|x| !x.is_empty()).collect(); - - for base in &bases { +fn take_next_base(bases: &mut Vec>) -> Option { + for base in bases.iter() { let head = base[0].clone(); - if !(&bases).iter().any(|x| x[1..].iter().any(|x| x.is(&head))) { + if !bases.iter().any(|x| x[1..].iter().any(|x| x.is(&head))) { // Remove from other heads. - for item in &mut bases { + for item in bases.iter_mut() { if item[0].is(&head) { item.remove(0); } } - return (Some(head), bases); + return Some(head); } } - (None, bases) + None } fn linearise_mro(mut bases: Vec>) -> Result, String> { @@ -768,22 +766,19 @@ fn linearise_mro(mut bases: Vec>) -> Result, Strin } let mut result = vec![]; - loop { - if (&bases).iter().all(Vec::is_empty) { - break; - } - let (head, new_bases) = take_next_base(bases); - let head = head.ok_or_else(|| { + while !bases.is_empty() { + let head = take_next_base(&mut bases).ok_or_else(|| { // Take the head class of each class here. Now that we have reached the problematic bases. // Because this failed, we assume the lists cannot be empty. format!( "Cannot create a consistent method resolution order (MRO) for bases {}", - new_bases.iter().map(|x| x.first().unwrap()).join(", ") + bases.iter().map(|x| x.first().unwrap()).format(", ") ) - }); + })?; - result.push(head.unwrap()); - bases = new_bases; + result.push(head); + + bases.retain(|x| !x.is_empty()); } Ok(result) } From 31f4999dfd4f0bcbe7ace0dd4232608b25300615 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Thu, 19 Nov 2020 12:47:47 -0600 Subject: [PATCH 6/6] Flatten PyObject/PyInner into one struct --- vm/src/pyobject.rs | 69 +-------------- vm/src/pyobjectrc.rs | 205 +++++++++++++++++++++---------------------- 2 files changed, 103 insertions(+), 171 deletions(-) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index a4277c0bb..ca5dc940a 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -37,7 +37,7 @@ use crate::dictdatatype::Dict; use crate::exceptions::{self, PyBaseExceptionRef}; use crate::function::{IntoFuncArgs, IntoPyNativeFunc}; use crate::iterator; -pub use crate::pyobjectrc::{PyObjectRef, PyObjectWeak, PyRef, PyWeakRef}; +pub use crate::pyobjectrc::{PyObject, PyObjectRef, PyObjectWeak, PyRef, PyWeakRef}; use crate::scope::Scope; use crate::slots::{PyTpFlags, PyTypeSlots}; use crate::types::{create_type_with_slots, TypeZoo}; @@ -371,12 +371,7 @@ impl PyContext { } pub fn new_base_object(&self, class: PyTypeRef, dict: Option) -> PyObjectRef { - PyObject { - typ: PyRwLock::new(class), - dict: dict.map(PyRwLock::new), - payload: object::PyBaseObject, - } - .into_ref() + PyObject::new(object::PyBaseObject, class, dict) } pub fn add_tp_new_wrapper(&self, ty: &PyTypeRef) { @@ -399,19 +394,6 @@ impl Default for PyContext { } } -/// This is an actual python object. It consists of a `typ` which is the -/// python class, and carries some rust payload optionally. This rust -/// payload can be a rust float or rust int in case of float and int objects. -#[repr(C)] -pub struct PyObject -where - T: ?Sized, -{ - pub(crate) typ: PyRwLock, // __class__ member - pub(crate) dict: Option>, // __dict__ member - pub payload: T, -} - impl TryFromObject for PyRef where T: PyValue, @@ -628,10 +610,10 @@ pub trait TypeProtocol { } } -impl TypeProtocol for PyObject { +impl TypeProtocol for PyObjectRef { fn class(&self) -> PyLease<'_, PyType> { PyLease { - inner: self.typ.read(), + inner: self.class_lock().read(), } } } @@ -676,12 +658,6 @@ where } } -impl fmt::Debug for PyObject { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "[PyObj {:?}]", &self.payload) - } -} - /// An iterable Python object. /// /// `PyIterable` implements `FromArgs` so that a built-in function can accept @@ -882,43 +858,6 @@ where } } -impl PyObject -where - T: Sized + PyObjectPayload, -{ - #[allow(clippy::new_ret_no_self)] - pub fn new(payload: T, typ: PyTypeRef, dict: Option) -> PyObjectRef { - PyObject { - typ: PyRwLock::new(typ), - dict: dict.map(PyRwLock::new), - payload, - } - .into_ref() - } - - // Move this object into a reference object, transferring ownership. - pub fn into_ref(self) -> PyObjectRef { - PyObjectRef::new(self) - } -} - -impl PyObject { - pub fn dict(&self) -> Option { - self.dict.as_ref().map(|mu| mu.read().clone()) - } - /// Set the dict field. Returns `Err(dict)` if this object does not have a dict field - /// in the first place. - pub fn set_dict(&self, dict: PyDictRef) -> Result<(), PyDictRef> { - match self.dict { - Some(ref mu) => { - *mu.write() = dict; - Ok(()) - } - None => Err(dict), - } - } -} - cfg_if::cfg_if! { if #[cfg(feature = "threading")] { pub trait PyThreadingConstraint: Send + Sync {} diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index ca4eac429..3467a5840 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -1,5 +1,7 @@ +use crate::builtins::{PyDictRef, PyTypeRef}; +use crate::common::lock::PyRwLock; use crate::common::rc::{PyRc, PyWeak}; -use crate::pyobject::{self, IdProtocol, PyObject, PyObjectPayload, TypeProtocol}; +use crate::pyobject::{self, IdProtocol, PyObjectPayload, TypeProtocol}; use crate::VirtualMachine; use std::any::TypeId; use std::fmt; @@ -44,27 +46,26 @@ use std::ops::Deref; struct Erased; struct PyObjVTable { - drop: unsafe fn(*mut PyObject), - debug: unsafe fn(*const PyObject, &mut fmt::Formatter) -> fmt::Result, + drop: unsafe fn(*mut PyInner), + debug: unsafe fn(*const PyInner, &mut fmt::Formatter) -> fmt::Result, } -unsafe fn drop_obj(x: *mut PyObject) { - std::ptr::drop_in_place(x as *mut PyObject) +unsafe fn drop_obj(x: *mut PyInner) { + std::ptr::drop_in_place(x as *mut PyInner) } unsafe fn debug_obj( - x: *const PyObject, + x: *const PyInner, f: &mut fmt::Formatter, ) -> fmt::Result { - let x = &*x.cast::>(); + let x = &*x.cast::>(); fmt::Debug::fmt(x, f) } - -macro_rules! make_vtable { - ($t:ty) => { +impl PyObjVTable { + pub fn of() -> &'static Self { &PyObjVTable { - drop: drop_obj::<$t>, - debug: debug_obj::<$t>, + drop: drop_obj::, + debug: debug_obj::, } - }; + } } #[repr(C)] @@ -72,25 +73,49 @@ struct PyInner { // TODO: move typeid into vtable once TypeId::of is const typeid: TypeId, vtable: &'static PyObjVTable, - value: ManuallyDrop>, + + typ: PyRwLock, // __class__ member + dict: Option>, // __dict__ member + + payload: T, } -impl PyInner { - fn new(value: PyObject) -> Self { - PyInner { - typeid: TypeId::of::(), - vtable: make_vtable!(T), - value: ManuallyDrop::new(value), - } +impl fmt::Debug for PyInner { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "[PyObj {:?}]", &self.payload) } } -impl Drop for PyInner { +/// This is an actual python object. It consists of a `typ` which is the +/// python class, and carries some rust payload optionally. This rust +/// payload can be a rust float or rust int in case of float and int objects. +#[repr(transparent)] +pub struct PyObject { + inner: ManuallyDrop>, +} + +impl PyObject { + #[allow(clippy::new_ret_no_self)] + pub fn new(payload: T, typ: PyTypeRef, dict: Option) -> PyObjectRef { + let inner = PyInner { + typeid: TypeId::of::(), + vtable: PyObjVTable::of::(), + typ: PyRwLock::new(typ), + dict: dict.map(PyRwLock::new), + payload, + }; + PyObjectRef::new(PyObject { + inner: ManuallyDrop::new(inner), + }) + } +} + +impl Drop for PyObject { fn drop(&mut self) { - let erased = &mut *self.value as *mut _ as *mut PyObject; + let erased = &mut *self.inner as *mut _ as *mut PyInner; // SAFETY: the vtable contains functions that accept payload types that always match up // with the payload of the object - unsafe { (self.vtable.drop)(erased) } + unsafe { (self.inner.vtable.drop)(erased) } } } @@ -102,13 +127,13 @@ impl Drop for PyInner { #[derive(Clone)] #[repr(transparent)] pub struct PyObjectRef { - inner: PyRc>, + rc: PyRc>, } #[derive(Clone)] #[repr(transparent)] pub struct PyObjectWeak { - inner: PyWeak>, + weak: PyWeak>, } /// A marker type that just references a raw python object. Don't use directly, pass as a pointer @@ -117,7 +142,7 @@ pub enum RawPyObject {} impl PyObjectRef { pub fn into_raw(this: Self) -> *const RawPyObject { - let ptr = PyRc::as_ptr(&this.inner); + let ptr = PyRc::as_ptr(&this.rc); std::mem::forget(this); ptr.cast() } @@ -129,41 +154,41 @@ impl PyObjectRef { /// too many times. pub unsafe fn from_raw(ptr: *const RawPyObject) -> Self { Self { - inner: PyRc::from_raw(ptr.cast()), + rc: PyRc::from_raw(ptr.cast()), } } - pub fn new(value: PyObject) -> Self { - let inner = PyRc::into_raw(PyRc::new(PyInner::::new(value))); - let inner = unsafe { PyRc::from_raw(inner as *const PyInner) }; - Self { inner } + fn new(value: PyObject) -> Self { + let inner = PyRc::into_raw(PyRc::new(value)); + let rc = unsafe { PyRc::from_raw(inner as *const PyObject) }; + Self { rc } } pub fn strong_count(this: &Self) -> usize { - PyRc::strong_count(&this.inner) + PyRc::strong_count(&this.rc) } pub fn weak_count(this: &Self) -> usize { - PyRc::weak_count(&this.inner) + PyRc::weak_count(&this.rc) } pub fn downgrade(this: &Self) -> PyObjectWeak { PyObjectWeak { - inner: PyRc::downgrade(&this.inner), + weak: PyRc::downgrade(&this.rc), } } pub fn payload_is(&self) -> bool { - self.inner.typeid == TypeId::of::() + self.rc.inner.typeid == TypeId::of::() } pub fn payload(&self) -> Option<&T> { if self.payload_is::() { - // we cast to a PyObject first because we don't know T's exact offset because of - // varying alignment, but once we get a PyObject the compiler can get it for us - let pyobj = - unsafe { &*(&*self.inner.value as *const PyObject as *const PyObject) }; - Some(&pyobj.payload) + // we cast to a PyInner first because we don't know T's exact offset because of + // varying alignment, but once we get a PyInner the compiler can get it for us + let inner = + unsafe { &*(&*self.rc.inner as *const PyInner as *const PyInner) }; + Some(&inner.payload) } else { None } @@ -191,8 +216,8 @@ impl PyObjectRef { } } - pub(crate) fn class_lock(&self) -> &crate::common::lock::PyRwLock { - &self.inner.value.typ + pub(crate) fn class_lock(&self) -> &PyRwLock { + &self.rc.inner.typ } // ideally we'd be able to define these in pyobject.rs, but method visibility rules are weird @@ -230,16 +255,19 @@ impl PyObjectRef { } } - pub fn dict(&self) -> Option { - self.inner.value.dict() + pub fn dict(&self) -> Option { + self.rc.inner.dict.as_ref().map(|mu| mu.read().clone()) } /// Set the dict field. Returns `Err(dict)` if this object does not have a dict field /// in the first place. - pub fn set_dict( - &self, - dict: crate::builtins::PyDictRef, - ) -> Result<(), crate::builtins::PyDictRef> { - self.inner.value.set_dict(dict) + pub fn set_dict(&self, dict: PyDictRef) -> Result<(), PyDictRef> { + match self.rc.inner.dict { + Some(ref mu) => { + *mu.write() = dict; + Ok(()) + } + None => Err(dict), + } } #[inline] @@ -257,13 +285,13 @@ impl PyObjectRef { impl IdProtocol for PyObjectRef { fn get_id(&self) -> usize { - self.inner.get_id() + self.rc.get_id() } } impl PyObjectWeak { pub fn upgrade(&self) -> Option { - self.inner.upgrade().map(|inner| PyObjectRef { inner }) + self.weak.upgrade().map(|rc| PyObjectRef { rc }) } } @@ -272,7 +300,7 @@ impl Drop for PyObjectRef { use crate::pyobject::BorrowValue; // PyObjectRef will drop the value when its count goes to 0 - if PyRc::strong_count(&self.inner) != 1 { + if PyRc::strong_count(&self.rc) != 1 { return; } @@ -310,7 +338,7 @@ impl fmt::Debug for PyObjectRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // SAFETY: the vtable contains functions that accept payload types that always match up // with the payload of the object - unsafe { (self.inner.vtable.debug)(&*self.inner.value, f) } + unsafe { (self.rc.inner.vtable.debug)(&*self.rc.inner, f) } } } @@ -392,9 +420,8 @@ where fn deref(&self) -> &T { // SAFETY: per the invariant on `self.obj`, the payload of the pyobject is always T, so it - // can always be cast to a PyObject - let obj = - unsafe { &*(&*self.obj.inner.value as *const PyObject as *const PyObject) }; + // can always be cast to a PyInner + let obj = unsafe { &*(&*self.obj.rc.inner as *const PyInner as *const PyInner) }; &obj.payload } } @@ -414,12 +441,6 @@ impl PyWeakRef { } } -impl TypeProtocol for PyObjectRef { - fn class(&self) -> pyobject::PyLease<'_, crate::builtins::PyType> { - self.inner.value.class() - } -} - /// Paritally initialize a struct, ensuring that all fields are /// either given values or explicitly left uninitialized macro_rules! partially_init { @@ -444,10 +465,8 @@ macro_rules! partially_init { }}; } -use crate::builtins::PyTypeRef; pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { use crate::builtins::{object, PyType, PyWeak}; - use crate::common::lock::PyRwLock; use crate::pyobject::{PyAttributes, PyClassDef, PyClassImpl}; use std::mem::MaybeUninit; use std::ptr; @@ -457,13 +476,12 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { // to produce this circular dependency, we need an unsafe block. // (and yes, this will never get dropped. TODO?) let (type_type, object_type) = { - type PyTypeObj = PyObject; type UninitRef = PyRwLock>>>; // We cast between these 2 types, so make sure (at compile time) that there's no change in // layout when we wrap PyInner in MaybeUninit<> - static_assertions::assert_eq_size!(MaybeUninit>, PyInner); - static_assertions::assert_eq_align!(MaybeUninit>, PyInner); + static_assertions::assert_eq_size!(MaybeUninit>, PyInner); + static_assertions::assert_eq_align!(MaybeUninit>, PyInner); let type_payload = PyType { name: PyTypeRef::NAME.to_owned(), @@ -486,18 +504,20 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { let type_type = PyRc::new(partially_init!( PyInner:: { typeid: TypeId::of::(), - vtable: make_vtable!(PyType), + vtable: PyObjVTable::of::(), + dict: None, + payload: type_payload, }, - Uninit { value } + Uninit { typ } )); let object_type = PyRc::new(partially_init!( PyInner:: { typeid: TypeId::of::(), - vtable: make_vtable!(PyType), - // dict: None, - // payload: object_payload, + vtable: PyObjVTable::of::(), + dict: None, + payload: object_payload, }, - Uninit { value }, + Uninit { typ }, )); let object_type_ptr = PyRc::into_raw(object_type) as *mut MaybeUninit> @@ -506,39 +526,12 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { as *mut PyInner; unsafe { - // TODO: make this part of the partially_init!() method - // partially initialize the inner PyObject of PyInner - std::ptr::write( - &mut (*type_type_ptr).value as *mut ManuallyDrop - as *mut MaybeUninit, - partially_init!( - PyTypeObj { - dict: None, - payload: type_payload, - }, - Uninit { typ } - ), - ); - std::ptr::write( - &mut (*object_type_ptr).value as *mut ManuallyDrop - as *mut MaybeUninit, - partially_init!( - PyTypeObj { - dict: None, - payload: object_payload, - }, - Uninit { typ } - ), - ); - ptr::write( - &mut (*object_type_ptr).value.typ as *mut PyRwLock - as *mut UninitRef, + &mut (*object_type_ptr).typ as *mut PyRwLock as *mut UninitRef, PyRwLock::new(type_type.clone()), ); ptr::write( - &mut (*type_type_ptr).value.typ as *mut PyRwLock - as *mut UninitRef, + &mut (*type_type_ptr).typ as *mut PyRwLock as *mut UninitRef, PyRwLock::new(type_type), ); @@ -547,9 +540,9 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef) { let object_type = PyTypeRef::from_obj_unchecked(PyObjectRef::from_raw(object_type_ptr.cast())); - (*type_type_ptr).value.payload.mro = vec![object_type.clone()]; - (*type_type_ptr).value.payload.bases = vec![object_type.clone()]; - (*type_type_ptr).value.payload.base = Some(object_type.clone()); + (*type_type_ptr).payload.mro = vec![object_type.clone()]; + (*type_type_ptr).payload.bases = vec![object_type.clone()]; + (*type_type_ptr).payload.base = Some(object_type.clone()); (type_type, object_type) }