From 7520b49ac1517755e2f54c6fc88ad9e8fc6543cd Mon Sep 17 00:00:00 2001 From: mejrs Date: Mon, 22 Nov 2021 09:26:34 +0100 Subject: [PATCH] Implement opt-in immutable pyclasses --- guide/src/class.md | 49 ++-- pyo3-macros-backend/src/pyclass.rs | 62 ++++- src/class/basic.rs | 12 +- src/class/buffer.rs | 4 +- src/class/gc.rs | 4 +- src/class/mapping.rs | 6 +- src/class/number.rs | 30 +-- src/class/sequence.rs | 10 +- src/conversion.rs | 5 +- src/impl_/pyclass.rs | 21 +- src/instance.rs | 16 +- src/pycell.rs | 230 +++++++++++++----- src/pyclass.rs | 8 +- src/pyclass_init.rs | 17 +- src/types/mod.rs | 4 +- tests/test_compile_error.rs | 4 + tests/ui/invalid_immutable_pyclass_borrow.rs | 13 + .../invalid_immutable_pyclass_borrow.stderr | 5 + tests/ui/pyclass_send.stderr | 12 + 19 files changed, 364 insertions(+), 148 deletions(-) create mode 100644 tests/ui/invalid_immutable_pyclass_borrow.rs create mode 100644 tests/ui/invalid_immutable_pyclass_borrow.stderr diff --git a/guide/src/class.md b/guide/src/class.md index fdc5fc26..1156ae1c 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -932,36 +932,41 @@ The `#[pyclass]` macro expands to roughly the code seen below. The `PyClassImplC # #[cfg(not(feature = "multiple-pymethods"))] { # use pyo3::prelude::*; // Note: the implementation differs slightly with the `multiple-pymethods` feature enabled. - -/// Class for demonstration struct MyClass { # #[allow(dead_code)] num: i32, } - -unsafe impl pyo3::PyTypeInfo for MyClass { - type AsRefTarget = PyCell; - +unsafe impl ::pyo3::type_object::PyTypeInfo for MyClass { + type AsRefTarget = ::pyo3::PyCell; const NAME: &'static str = "MyClass"; - const MODULE: Option<&'static str> = None; - + const MODULE: ::std::option::Option<&'static str> = ::std::option::Option::None; #[inline] - fn type_object_raw(py: pyo3::Python) -> *mut pyo3::ffi::PyTypeObject { - use pyo3::type_object::LazyStaticType; + fn type_object_raw(py: ::pyo3::Python<'_>) -> *mut ::pyo3::ffi::PyTypeObject { + use ::pyo3::type_object::LazyStaticType; static TYPE_OBJECT: LazyStaticType = LazyStaticType::new(); TYPE_OBJECT.get_or_init::(py) } } -impl pyo3::pyclass::PyClass for MyClass { - type Dict = pyo3::impl_::pyclass::PyClassDummySlot; - type WeakRef = pyo3::impl_::pyclass::PyClassDummySlot; - type BaseNativeType = PyAny; +impl ::pyo3::PyClass for MyClass { + type Dict = ::pyo3::impl_::pyclass::PyClassDummySlot; + type WeakRef = ::pyo3::impl_::pyclass::PyClassDummySlot; + type BaseNativeType = ::pyo3::PyAny; } -impl pyo3::IntoPy for MyClass { - fn into_py(self, py: pyo3::Python) -> pyo3::PyObject { - pyo3::IntoPy::into_py(pyo3::Py::new(py, self).unwrap(), py) +unsafe impl ::pyo3::pyclass::MutablePyClass for MyClass {} + +impl<'a> ::pyo3::derive_utils::ExtractExt<'a> for &'a mut MyClass { + type Target = ::pyo3::PyRefMut<'a, MyClass>; +} + +impl<'a> ::pyo3::derive_utils::ExtractExt<'a> for &'a MyClass { + type Target = ::pyo3::PyRef<'a, MyClass>; +} + +impl ::pyo3::IntoPy<::pyo3::PyObject> for MyClass { + fn into_py(self, py: ::pyo3::Python) -> ::pyo3::PyObject { + ::pyo3::IntoPy::into_py(::pyo3::Py::new(py, self).unwrap(), py) } } @@ -973,6 +978,7 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { type Layout = PyCell; type BaseType = PyAny; type ThreadChecker = pyo3::impl_::pyclass::ThreadCheckerStub; + type Mutabilty = pyo3::pyclass::Mutable; fn for_all_items(visitor: &mut dyn FnMut(&pyo3::impl_::pyclass::PyClassItems)) { use pyo3::impl_::pyclass::*; @@ -997,6 +1003,15 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { collector.free_impl() } } + +impl ::pyo3::impl_::pyclass::PyClassDescriptors + for ::pyo3::impl_::pyclass::PyClassImplCollector +{ + fn py_class_descriptors(self) -> &'static [::pyo3::class::methods::PyMethodDefType] { + static METHODS: &[::pyo3::class::methods::PyMethodDefType] = &[]; + METHODS + } +} # Python::with_gil(|py| { # let cls = py.get_type::(); # pyo3::py_run!(py, cls, "assert cls.__name__ == 'MyClass'") diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 18c29957..c233900e 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -35,6 +35,7 @@ pub struct PyClassArgs { pub has_unsendable: bool, pub module: Option, pub class_kind: PyClassKind, + pub is_immutable: bool, } impl PyClassArgs { @@ -67,6 +68,7 @@ impl PyClassArgs { is_basetype: false, has_extends: false, has_unsendable: false, + is_immutable: false, class_kind, } } @@ -176,6 +178,9 @@ impl PyClassArgs { "unsendable" => { self.has_unsendable = true; } + "immutable" => { + self.is_immutable = true; + } _ => bail_spanned!( exp.path.span() => "expected one of gc/weakref/subclass/dict/unsendable" ), @@ -587,12 +592,29 @@ fn impl_enum_class( let default_items = gen_default_items(cls, vec![default_repr_impl, default_richcmp, default_int]); + let mutability = if args.is_immutable { + quote! { + unsafe impl _pyo3::pyclass::ImmutablePyClass for #cls {} + } + } else { + quote! { + unsafe impl _pyo3::pyclass::MutablePyClass for #cls {} + + impl<'a> _pyo3::derive_utils::ExtractExt<'a> for &'a mut #cls + { + type Target = _pyo3::PyRefMut<'a, #cls>; + } + } + }; + Ok(quote! { const _: () = { use #krate as _pyo3; #pytypeinfo + #mutability + #pyclass_impls #default_items @@ -788,20 +810,30 @@ impl<'a> PyClassImplsBuilder<'a> { type Dict = #dict; type WeakRef = #weakref; type BaseNativeType = #base_nativetype; + type Mutability = _pyo3::pycell::Immutable; } } } fn impl_extractext(&self) -> TokenStream { let cls = self.cls; - quote! { - impl<'a> _pyo3::derive_utils::ExtractExt<'a> for &'a #cls - { - type Target = _pyo3::PyRef<'a, #cls>; + if self.attr.is_immutable { + quote! { + impl<'a> _pyo3::derive_utils::ExtractExt<'a> for &'a #cls + { + type Target = _pyo3::PyRef<'a, #cls>; + } } + } else { + quote! { + impl<'a> _pyo3::derive_utils::ExtractExt<'a> for &'a #cls + { + type Target = _pyo3::PyRef<'a, #cls>; + } - impl<'a> _pyo3::derive_utils::ExtractExt<'a> for &'a mut #cls - { - type Target = _pyo3::PyRefMut<'a, #cls>; + impl<'a> _pyo3::derive_utils::ExtractExt<'a> for &'a mut #cls + { + type Target = _pyo3::PyRefMut<'a, #cls>; + } } } } @@ -901,8 +933,18 @@ impl<'a> PyClassImplsBuilder<'a> { let default_items = &self.default_items; + let mutability = if self.attr.is_immutable { + quote! { + _pyo3::pycell::Immutable + } + } else { + quote! { + _pyo3::pycell::Mutable + } + }; + quote! { - impl _pyo3::impl_::pyclass::PyClassImpl for #cls { + impl _pyo3::impl_::pyclass::PyClassImpl<#mutability> for #cls { const DOC: &'static str = #doc; const IS_GC: bool = #is_gc; const IS_BASETYPE: bool = #is_basetype; @@ -943,9 +985,9 @@ impl<'a> PyClassImplsBuilder<'a> { #dict_offset #weaklist_offset - } - #inventory_class + #inventory_class + } } } diff --git a/src/class/basic.rs b/src/class/basic.rs index b2d0e9ab..90f941f2 100644 --- a/src/class/basic.rs +++ b/src/class/basic.rs @@ -9,7 +9,9 @@ //! [typeobj docs](https://docs.python.org/3/c-api/typeobj.html) use crate::callback::{HashCallbackOutput, IntoPyCallbackOutput}; -use crate::{exceptions, ffi, FromPyObject, PyAny, PyCell, PyClass, PyObject}; +use crate::{ + exceptions, ffi, pyclass::MutablePyClass, FromPyObject, PyAny, PyCell, PyClass, PyObject, +}; use std::os::raw::c_int; /// Basic Python class customization @@ -24,14 +26,14 @@ pub trait PyObjectProtocol<'p>: PyClass { fn __setattr__(&'p mut self, name: Self::Name, value: Self::Value) -> Self::Result where - Self: PyObjectSetAttrProtocol<'p>, + Self: PyObjectSetAttrProtocol<'p> + MutablePyClass, { unimplemented!() } fn __delattr__(&'p mut self, name: Self::Name) -> Self::Result where - Self: PyObjectDelAttrProtocol<'p>, + Self: PyObjectDelAttrProtocol<'p> + MutablePyClass, { unimplemented!() } @@ -75,12 +77,12 @@ pub trait PyObjectGetAttrProtocol<'p>: PyObjectProtocol<'p> { type Name: FromPyObject<'p>; type Result: IntoPyCallbackOutput; } -pub trait PyObjectSetAttrProtocol<'p>: PyObjectProtocol<'p> { +pub trait PyObjectSetAttrProtocol<'p>: PyObjectProtocol<'p> + MutablePyClass { type Name: FromPyObject<'p>; type Value: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyObjectDelAttrProtocol<'p>: PyObjectProtocol<'p> { +pub trait PyObjectDelAttrProtocol<'p>: PyObjectProtocol<'p> + MutablePyClass { type Name: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } diff --git a/src/class/buffer.rs b/src/class/buffer.rs index 7b4c8acf..be47a5c2 100644 --- a/src/class/buffer.rs +++ b/src/class/buffer.rs @@ -5,7 +5,7 @@ //! For more information check [buffer protocol](https://docs.python.org/3/c-api/buffer.html) //! c-api use crate::callback::IntoPyCallbackOutput; -use crate::{ffi, PyCell, PyClass, PyRefMut}; +use crate::{ffi, pyclass::MutablePyClass, PyCell, PyRefMut}; use std::os::raw::c_int; /// Buffer protocol interface @@ -13,7 +13,7 @@ use std::os::raw::c_int; /// For more information check [buffer protocol](https://docs.python.org/3/c-api/buffer.html) /// c-api. #[allow(unused_variables)] -pub trait PyBufferProtocol<'p>: PyClass { +pub trait PyBufferProtocol<'p>: MutablePyClass { // No default implementations so that implementors of this trait provide both methods. fn bf_getbuffer(slf: PyRefMut, view: *mut ffi::Py_buffer, flags: c_int) -> Self::Result diff --git a/src/class/gc.rs b/src/class/gc.rs index 2641a2b3..2f1085d0 100644 --- a/src/class/gc.rs +++ b/src/class/gc.rs @@ -2,14 +2,14 @@ //! Python GC support -use crate::{ffi, AsPyPointer, PyCell, PyClass, Python}; +use crate::{ffi, pyclass::MutablePyClass, AsPyPointer, PyCell, Python}; use std::os::raw::{c_int, c_void}; #[repr(transparent)] pub struct PyTraverseError(c_int); /// GC support -pub trait PyGCProtocol<'p>: PyClass { +pub trait PyGCProtocol<'p>: MutablePyClass { fn __traverse__(&'p self, visit: PyVisit) -> Result<(), PyTraverseError>; fn __clear__(&'p mut self); } diff --git a/src/class/mapping.rs b/src/class/mapping.rs index b6868521..627ef997 100644 --- a/src/class/mapping.rs +++ b/src/class/mapping.rs @@ -4,7 +4,7 @@ //! Trait and support implementation for implementing mapping support use crate::callback::IntoPyCallbackOutput; -use crate::{FromPyObject, PyClass, PyObject}; +use crate::{pyclass::MutablePyClass, FromPyObject, PyClass, PyObject}; /// Mapping interface #[allow(unused_variables)] @@ -50,13 +50,13 @@ pub trait PyMappingGetItemProtocol<'p>: PyMappingProtocol<'p> { type Result: IntoPyCallbackOutput; } -pub trait PyMappingSetItemProtocol<'p>: PyMappingProtocol<'p> { +pub trait PyMappingSetItemProtocol<'p>: PyMappingProtocol<'p> + MutablePyClass { type Key: FromPyObject<'p>; type Value: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyMappingDelItemProtocol<'p>: PyMappingProtocol<'p> { +pub trait PyMappingDelItemProtocol<'p>: PyMappingProtocol<'p> + MutablePyClass { type Key: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } diff --git a/src/class/number.rs b/src/class/number.rs index 448834d2..e261db67 100644 --- a/src/class/number.rs +++ b/src/class/number.rs @@ -4,7 +4,7 @@ //! Trait and support implementation for implementing number protocol use crate::callback::IntoPyCallbackOutput; use crate::err::PyErr; -use crate::{ffi, FromPyObject, PyClass, PyObject}; +use crate::{ffi, pyclass::MutablePyClass, FromPyObject, PyClass, PyObject}; /// Number interface #[allow(unused_variables)] @@ -459,74 +459,74 @@ pub trait PyNumberROrProtocol<'p>: PyNumberProtocol<'p> { type Result: IntoPyCallbackOutput; } -pub trait PyNumberIAddProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIAddProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberISubProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberISubProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIMulProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIMulProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIMatmulProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIMatmulProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberITruedivProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberITruedivProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIFloordivProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIFloordivProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIModProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIModProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIDivmodProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIDivmodProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIPowProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIPowProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; // See https://bugs.python.org/issue36379 type Modulo: FromPyObject<'p>; } -pub trait PyNumberILShiftProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberILShiftProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIRShiftProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIRShiftProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIAndProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIAndProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIXorProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIXorProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PyNumberIOrProtocol<'p>: PyNumberProtocol<'p> { +pub trait PyNumberIOrProtocol<'p>: PyNumberProtocol<'p> + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } diff --git a/src/class/sequence.rs b/src/class/sequence.rs index 129a188a..6f453d78 100644 --- a/src/class/sequence.rs +++ b/src/class/sequence.rs @@ -6,7 +6,7 @@ use crate::callback::IntoPyCallbackOutput; use crate::conversion::{FromPyObject, IntoPy}; use crate::err::PyErr; -use crate::{exceptions, ffi, PyAny, PyCell, PyClass, PyObject}; +use crate::{exceptions, ffi, pyclass::MutablePyClass, PyAny, PyCell, PyClass, PyObject}; use std::os::raw::c_int; /// Sequence interface @@ -88,13 +88,13 @@ pub trait PySequenceGetItemProtocol<'p>: PySequenceProtocol<'p> { type Result: IntoPyCallbackOutput; } -pub trait PySequenceSetItemProtocol<'p>: PySequenceProtocol<'p> { +pub trait PySequenceSetItemProtocol<'p>: PySequenceProtocol<'p> + MutablePyClass { type Index: FromPyObject<'p> + From; type Value: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; } -pub trait PySequenceDelItemProtocol<'p>: PySequenceProtocol<'p> { +pub trait PySequenceDelItemProtocol<'p>: PySequenceProtocol<'p> + MutablePyClass { type Index: FromPyObject<'p> + From; type Result: IntoPyCallbackOutput<()>; } @@ -115,14 +115,14 @@ pub trait PySequenceRepeatProtocol<'p>: PySequenceProtocol<'p> { } pub trait PySequenceInplaceConcatProtocol<'p>: - PySequenceProtocol<'p> + IntoPy + 'p + PySequenceProtocol<'p> + IntoPy + MutablePyClass { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput; } pub trait PySequenceInplaceRepeatProtocol<'p>: - PySequenceProtocol<'p> + IntoPy + 'p + PySequenceProtocol<'p> + IntoPy + MutablePyClass + 'p { type Index: FromPyObject<'p> + From; type Result: IntoPyCallbackOutput; diff --git a/src/conversion.rs b/src/conversion.rs index ccd690be..f2da392d 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -5,7 +5,8 @@ use crate::err::{self, PyDowncastError, PyResult}; use crate::type_object::PyTypeInfo; use crate::types::PyTuple; use crate::{ - ffi, gil, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python, + ffi, gil, pyclass::MutablePyClass, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, + PyRefMut, Python, }; use std::ptr::NonNull; @@ -329,7 +330,7 @@ where impl<'a, T> FromPyObject<'a> for PyRefMut<'a, T> where - T: PyClass, + T: MutablePyClass, { fn extract(obj: &'a PyAny) -> PyResult { let cell: &PyCell = PyTryFrom::try_from(obj)?; diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 8f33b964..5ef2423d 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -2,7 +2,7 @@ use crate::{ exceptions::{PyAttributeError, PyNotImplementedError}, ffi, impl_::freelist::FreeList, - pycell::PyCellLayout, + pycell::{Mutability, PyCellLayout}, pyclass_init::PyObjectInit, type_object::{PyLayout, PyTypeObject}, PyCell, PyClass, PyMethodDefType, PyNativeType, PyResult, PyTypeInfo, Python, @@ -145,7 +145,7 @@ unsafe impl Sync for PyClassItems {} /// /// Users are discouraged from implementing this trait manually; it is a PyO3 implementation detail /// and may be changed at any time. -pub trait PyClassImpl: Sized { +pub trait PyClassImpl: Sized { /// Class doc string const DOC: &'static str = "\0"; @@ -162,7 +162,7 @@ pub trait PyClassImpl: Sized { type Layout: PyLayout; /// Base class - type BaseType: PyTypeInfo + PyTypeObject + PyClassBaseType; + type BaseType: PyTypeInfo + PyTypeObject + PyClassBaseType; /// This handles following two situations: /// 1. In case `T` is `Send`, stub `ThreadChecker` is used and does nothing. @@ -818,9 +818,14 @@ impl PyClassThreadChecker for ThreadCheckerImpl { /// Thread checker for types that have `Send` and `extends=...`. /// Ensures that `T: Send` and the parent is not accessed by another thread. #[doc(hidden)] -pub struct ThreadCheckerInherited(PhantomData, U::ThreadChecker); +pub struct ThreadCheckerInherited, M: Mutability>( + PhantomData, + U::ThreadChecker, +); -impl PyClassThreadChecker for ThreadCheckerInherited { +impl, M: Mutability> PyClassThreadChecker + for ThreadCheckerInherited +{ fn ensure(&self) { self.1.ensure(); } @@ -831,15 +836,15 @@ impl PyClassThreadChecker for ThreadCheckerInher } /// Trait denoting that this class is suitable to be used as a base type for PyClass. -pub trait PyClassBaseType: Sized { - type LayoutAsBase: PyCellLayout; +pub trait PyClassBaseType: Sized { + type LayoutAsBase: PyCellLayout; type BaseNativeType; type ThreadChecker: PyClassThreadChecker; type Initializer: PyObjectInit; } /// All PyClasses can be used as a base type. -impl PyClassBaseType for T { +impl PyClassBaseType for T { type LayoutAsBase = crate::pycell::PyCell; type BaseNativeType = T::BaseNativeType; type ThreadChecker = T::ThreadChecker; diff --git a/src/instance.rs b/src/instance.rs index 780d35fa..694e85f5 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -5,8 +5,8 @@ use crate::gil; use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell}; use crate::types::{PyDict, PyTuple}; use crate::{ - ffi, AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyAny, PyClass, PyClassInitializer, - PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject, + ffi, pyclass::MutablePyClass, AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyAny, PyClass, + PyClassInitializer, PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject, }; use std::marker::PhantomData; use std::mem; @@ -427,7 +427,10 @@ where /// # Panics /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use /// [`try_borrow_mut`](#method.try_borrow_mut). - pub fn borrow_mut<'py>(&'py self, py: Python<'py>) -> PyRefMut<'py, T> { + pub fn borrow_mut<'py>(&'py self, py: Python<'py>) -> PyRefMut<'py, T> + where + T: MutablePyClass, + { self.as_ref(py).borrow_mut() } @@ -454,7 +457,10 @@ where pub fn try_borrow_mut<'py>( &'py self, py: Python<'py>, - ) -> Result, PyBorrowMutError> { + ) -> Result, PyBorrowMutError> + where + T: MutablePyClass, + { self.as_ref(py).try_borrow_mut() } } @@ -819,7 +825,7 @@ where impl<'a, T> std::convert::From> for Py where - T: PyClass, + T: MutablePyClass, { fn from(pyref: PyRefMut<'a, T>) -> Self { unsafe { Py::from_borrowed_ptr(pyref.py(), pyref.as_ptr()) } diff --git a/src/pycell.rs b/src/pycell.rs index b8b2e395..4d116d45 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -176,7 +176,7 @@ use crate::exceptions::PyRuntimeError; use crate::impl_::pyclass::{PyClassBaseType, PyClassDict, PyClassThreadChecker, PyClassWeakRef}; -use crate::pyclass::PyClass; +use crate::pyclass::{MutablePyClass, PyClass}; use crate::pyclass_init::PyClassInitializer; use crate::type_object::{PyLayout, PySizedLayout}; use crate::types::PyAny; @@ -189,19 +189,117 @@ use crate::{ use crate::{ffi, IntoPy, PyErr, PyNativeType, PyObject, PyResult, Python}; use std::cell::{Cell, UnsafeCell}; use std::fmt; +use std::marker::PhantomData; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; +pub trait Mutability { + /// Creates a new borrow checker + fn new() -> Self; + /// Increments immutable borrow count, if possible + fn try_borrow(&self) -> Result<(), PyBorrowError>; + + fn try_borrow_unguarded(&self) -> Result<(), PyBorrowError>; + + /// Decrements immutable borrow count + fn release_borrow(&self); + /// Increments mutable borrow count, if possible + fn try_borrow_mut(&self) -> Result<(), PyBorrowMutError>; + /// Decremements mutable borrow count + fn release_borrow_mut(&self); +} + +pub struct Mutable { + flag: Cell, +} +impl Mutability for Mutable { + fn new() -> Self { + Self { + flag: Cell::new(BorrowFlag::UNUSED), + } + } + + fn try_borrow(&self) -> Result<(), PyBorrowError> { + let flag = self.flag.get(); + if flag != BorrowFlag::HAS_MUTABLE_BORROW { + self.flag.set(flag.increment()); + Ok(()) + } else { + Err(PyBorrowError { _private: () }) + } + } + + fn try_borrow_unguarded(&self) -> Result<(), PyBorrowError> { + let flag = self.flag.get(); + if flag != BorrowFlag::HAS_MUTABLE_BORROW { + Ok(()) + } else { + Err(PyBorrowError { _private: () }) + } + } + + fn release_borrow(&self) { + let flag = self.flag.get(); + self.flag.set(flag.decrement()) + } + + fn try_borrow_mut(&self) -> Result<(), PyBorrowMutError> { + let flag = self.flag.get(); + if flag == BorrowFlag::UNUSED { + self.flag.set(BorrowFlag::HAS_MUTABLE_BORROW); + Ok(()) + } else { + Err(PyBorrowMutError { _private: () }) + } + } + + fn release_borrow_mut(&self) { + self.flag.set(BorrowFlag::UNUSED) + } +} + +pub struct Immutable { + flag: PhantomData>, +} +impl Mutability for Immutable { + fn new() -> Self { + Self { flag: PhantomData } + } + + fn try_borrow(&self) -> Result<(), PyBorrowError> { + Ok(()) + } + + fn try_borrow_unguarded(&self) -> Result<(), PyBorrowError> { + Ok(()) + } + + fn release_borrow(&self) {} + + fn try_borrow_mut(&self) -> Result<(), PyBorrowMutError> { + unreachable!() + } + + fn release_borrow_mut(&self) { + unreachable!() + } +} + /// Base layout of PyCell. /// This is necessary for sharing BorrowFlag between parents and children. #[doc(hidden)] #[repr(C)] -pub struct PyCellBase { +pub struct PyCellBase { ob_base: T, - borrow_flag: Cell, + borrow_impl: M, } -unsafe impl PyLayout for PyCellBase where U: PySizedLayout {} +unsafe impl PyLayout for PyCellBase +where + U: PySizedLayout, + M: Mutability, +{ +} /// A container type for (mutably) accessing [`PyClass`] values /// @@ -240,7 +338,7 @@ unsafe impl PyLayout for PyCellBase where U: PySizedLayout {} /// [module-level documentation](self). #[repr(C)] pub struct PyCell { - ob_base: ::LayoutAsBase, + ob_base: >::LayoutAsBase, contents: PyCellContents, } @@ -283,7 +381,10 @@ impl PyCell { /// /// Panics if the value is currently borrowed. For a non-panicking variant, use /// [`try_borrow_mut`](#method.try_borrow_mut). - pub fn borrow_mut(&self) -> PyRefMut<'_, T> { + pub fn borrow_mut(&self) -> PyRefMut<'_, T> + where + T: MutablePyClass, + { self.try_borrow_mut().expect("Already borrowed") } @@ -313,13 +414,9 @@ impl PyCell { /// }); /// ``` pub fn try_borrow(&self) -> Result, PyBorrowError> { - let flag = self.get_borrow_flag(); - if flag == BorrowFlag::HAS_MUTABLE_BORROW { - Err(PyBorrowError { _private: () }) - } else { - self.set_borrow_flag(flag.increment()); - Ok(PyRef { inner: self }) - } + self.borrow_checker() + .try_borrow() + .map(|_| PyRef { inner: self }) } /// Mutably borrows the value `T`, returning an error if the value is currently borrowed. @@ -343,13 +440,13 @@ impl PyCell { /// assert!(c.try_borrow_mut().is_ok()); /// }); /// ``` - pub fn try_borrow_mut(&self) -> Result, PyBorrowMutError> { - if self.get_borrow_flag() != BorrowFlag::UNUSED { - Err(PyBorrowMutError { _private: () }) - } else { - self.set_borrow_flag(BorrowFlag::HAS_MUTABLE_BORROW); - Ok(PyRefMut { inner: self }) - } + pub fn try_borrow_mut(&self) -> Result, PyBorrowMutError> + where + T: MutablePyClass, + { + self.borrow_checker() + .try_borrow_mut() + .map(|_| PyRefMut { inner: self }) } /// Immutably borrows the value `T`, returning an error if the value is @@ -382,11 +479,9 @@ impl PyCell { /// }); /// ``` pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, PyBorrowError> { - if self.get_borrow_flag() == BorrowFlag::HAS_MUTABLE_BORROW { - Err(PyBorrowError { _private: () }) - } else { - Ok(&*self.contents.value.get()) - } + self.borrow_checker() + .try_borrow_unguarded() + .map(|_: ()| &*self.contents.value.get()) } /// Replaces the wrapped value with a new one, returning the old value. @@ -395,7 +490,10 @@ impl PyCell { /// /// Panics if the value is currently borrowed. #[inline] - pub fn replace(&self, t: T) -> T { + pub fn replace(&self, t: T) -> T + where + T: MutablePyClass, + { std::mem::replace(&mut *self.borrow_mut(), t) } @@ -404,7 +502,10 @@ impl PyCell { /// # Panics /// /// Panics if the value is currently borrowed. - pub fn replace_with T>(&self, f: F) -> T { + pub fn replace_with T>(&self, f: F) -> T + where + T: MutablePyClass, + { let mut_borrow = &mut *self.borrow_mut(); let replacement = f(mut_borrow); std::mem::replace(mut_borrow, replacement) @@ -416,7 +517,10 @@ impl PyCell { /// /// Panics if the value in either `PyCell` is currently borrowed. #[inline] - pub fn swap(&self, other: &Self) { + pub fn swap(&self, other: &Self) + where + T: MutablePyClass, + { std::mem::swap(&mut *self.borrow_mut(), &mut *other.borrow_mut()) } @@ -674,8 +778,7 @@ impl<'p, T: PyClass> Deref for PyRef<'p, T> { impl<'p, T: PyClass> Drop for PyRef<'p, T> { fn drop(&mut self) { - let flag = self.inner.get_borrow_flag(); - self.inner.set_borrow_flag(flag.decrement()) + self.inner.borrow_checker().release_borrow() } } @@ -707,11 +810,11 @@ impl fmt::Debug for PyRef<'_, T> { /// A wrapper type for a mutably borrowed value from a[`PyCell`]``. /// /// See the [module-level documentation](self) for more information. -pub struct PyRefMut<'p, T: PyClass> { +pub struct PyRefMut<'p, T: MutablePyClass> { inner: &'p PyCell, } -impl<'p, T: PyClass> PyRefMut<'p, T> { +impl<'p, T: MutablePyClass> PyRefMut<'p, T> { /// Returns a `Python` token that is bound to the lifetime of the `PyRefMut`. pub fn py(&self) -> Python { unsafe { Python::assume_gil_acquired() } @@ -720,8 +823,8 @@ impl<'p, T: PyClass> PyRefMut<'p, T> { impl<'p, T, U> AsRef for PyRefMut<'p, T> where - T: PyClass, - U: PyClass, + T: PyClass + MutablePyClass, + U: MutablePyClass, { fn as_ref(&self) -> &T::BaseType { unsafe { &*self.inner.ob_base.get_ptr() } @@ -730,8 +833,8 @@ where impl<'p, T, U> AsMut for PyRefMut<'p, T> where - T: PyClass, - U: PyClass, + T: PyClass + MutablePyClass, + U: MutablePyClass, { fn as_mut(&mut self) -> &mut T::BaseType { unsafe { &mut *self.inner.ob_base.get_ptr() } @@ -740,8 +843,8 @@ where impl<'p, T, U> PyRefMut<'p, T> where - T: PyClass, - U: PyClass, + T: PyClass + MutablePyClass, + U: MutablePyClass, { /// Gets a `PyRef`. /// @@ -755,7 +858,7 @@ where } } -impl<'p, T: PyClass> Deref for PyRefMut<'p, T> { +impl<'p, T: MutablePyClass> Deref for PyRefMut<'p, T> { type Target = T; #[inline] @@ -764,46 +867,46 @@ impl<'p, T: PyClass> Deref for PyRefMut<'p, T> { } } -impl<'p, T: PyClass> DerefMut for PyRefMut<'p, T> { +impl<'p, T: MutablePyClass> DerefMut for PyRefMut<'p, T> { #[inline] fn deref_mut(&mut self) -> &mut T { unsafe { &mut *self.inner.get_ptr() } } } -impl<'p, T: PyClass> Drop for PyRefMut<'p, T> { +impl<'p, T: MutablePyClass> Drop for PyRefMut<'p, T> { fn drop(&mut self) { - self.inner.set_borrow_flag(BorrowFlag::UNUSED) + self.inner.borrow_checker().release_borrow_mut() } } -impl IntoPy for PyRefMut<'_, T> { +impl IntoPy for PyRefMut<'_, T> { fn into_py(self, py: Python) -> PyObject { unsafe { PyObject::from_borrowed_ptr(py, self.inner.as_ptr()) } } } -impl<'a, T: PyClass> AsPyPointer for PyRefMut<'a, T> { +impl<'a, T: MutablePyClass> AsPyPointer for PyRefMut<'a, T> { fn as_ptr(&self) -> *mut ffi::PyObject { self.inner.as_ptr() } } -impl<'a, T: PyClass> std::convert::TryFrom<&'a PyCell> for crate::PyRefMut<'a, T> { +impl<'a, T: MutablePyClass> std::convert::TryFrom<&'a PyCell> for crate::PyRefMut<'a, T> { type Error = PyBorrowMutError; fn try_from(cell: &'a crate::PyCell) -> Result { cell.try_borrow_mut() } } -impl fmt::Debug for PyRefMut<'_, T> { +impl fmt::Debug for PyRefMut<'_, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&*(self.deref()), f) } } #[doc(hidden)] -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Debug, Copy, Clone, Eq, PartialEq)] pub struct BorrowFlag(usize); impl BorrowFlag { @@ -868,9 +971,11 @@ impl From for PyErr { } #[doc(hidden)] -pub trait PyCellLayout: PyLayout { - fn get_borrow_flag(&self) -> BorrowFlag; - fn set_borrow_flag(&self, flag: BorrowFlag); +pub trait PyCellLayout: PyLayout +where + M: Mutability, +{ + fn borrow_checker(&self) -> &M; /// Implementation of tp_dealloc. /// # Safety /// - slf must be a valid pointer to an instance of a T or a subclass. @@ -878,17 +983,16 @@ pub trait PyCellLayout: PyLayout { unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python); } -impl PyCellLayout for PyCellBase +impl PyCellLayout for PyCellBase where U: PySizedLayout, T: PyTypeInfo, + M: Mutability, { - fn get_borrow_flag(&self) -> BorrowFlag { - self.borrow_flag.get() - } - fn set_borrow_flag(&self, flag: BorrowFlag) { - self.borrow_flag.set(flag) + fn borrow_checker(&self) -> &M { + &self.borrow_impl } + unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python) { // For `#[pyclass]` types which inherit from PyAny, we can just call tp_free if T::type_object_raw(py) == &mut PyBaseObject_Type { @@ -910,16 +1014,14 @@ where } } -impl PyCellLayout for PyCell +impl PyCellLayout for PyCell where - ::LayoutAsBase: PyCellLayout, + >::LayoutAsBase: + PyCellLayout, { - fn get_borrow_flag(&self) -> BorrowFlag { + fn borrow_checker(&self) -> &T::Mutability { self.contents.thread_checker.ensure(); - self.ob_base.get_borrow_flag() - } - fn set_borrow_flag(&self, flag: BorrowFlag) { - self.ob_base.set_borrow_flag(flag) + self.ob_base.borrow_checker() } unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python) { // Safety: Python only calls tp_dealloc when no references to the object remain. @@ -927,6 +1029,6 @@ where ManuallyDrop::drop(&mut cell.contents.value); cell.contents.dict.clear_dict(py); cell.contents.weakref.clear_weakrefs(slf, py); - ::LayoutAsBase::tp_dealloc(slf, py) + >::LayoutAsBase::tp_dealloc(slf, py) } } diff --git a/src/pyclass.rs b/src/pyclass.rs index f08669ad..ee448b32 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -1,4 +1,5 @@ //! `PyClass` and related traits. +use crate::pycell::Mutability; use crate::{ callback::IntoPyCallbackOutput, ffi, @@ -22,7 +23,7 @@ use std::{ /// The `#[pyclass]` attribute automatically implements this trait for your Rust struct, /// so you normally don't have to use this trait directly. pub trait PyClass: - PyTypeInfo> + PyClassImpl> + PyTypeInfo> + PyClassImpl> { /// Specify this class has `#[pyclass(dict)]` or not. type Dict: PyClassDict; @@ -31,8 +32,13 @@ pub trait PyClass: /// The closest native ancestor. This is `PyAny` by default, and when you declare /// `#[pyclass(extends=PyDict)]`, it's `PyDict`. type BaseNativeType: PyTypeInfo + PyNativeType; + + type Mutability: Mutability; } +pub unsafe trait MutablePyClass: PyClass {} +pub unsafe trait ImmutablePyClass: PyClass {} + fn into_raw(vec: Vec) -> *mut c_void { Box::into_raw(vec.into_boxed_slice()) as _ } diff --git a/src/pyclass_init.rs b/src/pyclass_init.rs index b8b2025b..e478472b 100644 --- a/src/pyclass_init.rs +++ b/src/pyclass_init.rs @@ -130,14 +130,17 @@ impl PyObjectInit for PyNativeTypeInitializer { /// ``` pub struct PyClassInitializer { init: T, - super_init: ::Initializer, + super_init: >::Initializer, } impl PyClassInitializer { /// Constructs a new initializer from value `T` and base class' initializer. /// /// It is recommended to use `add_subclass` instead of this method for most usage. - pub fn new(init: T, super_init: ::Initializer) -> Self { + pub fn new( + init: T, + super_init: >::Initializer, + ) -> Self { Self { init, super_init } } @@ -190,7 +193,7 @@ impl PyClassInitializer { pub fn add_subclass(self, subclass_value: S) -> PyClassInitializer where S: PyClass, - S::BaseType: PyClassBaseType, + S::BaseType: PyClassBaseType, { PyClassInitializer::new(subclass_value, self) } @@ -231,16 +234,16 @@ impl PyObjectInit for PyClassInitializer { /// Layout of a PyCellBase after base new has been called, but the borrow flag has not /// yet been initialized. #[repr(C)] - struct PartiallyInitializedPyCellBase { + struct PartiallyInitializedPyCellBase { _ob_base: T, - borrow_flag: MaybeUninit>, + borrow_flag: MaybeUninit, } /// Layout of a PyCell after base new has been called, but the contents have not yet been /// written. #[repr(C)] struct PartiallyInitializedPyCell { - _ob_base: ::LayoutAsBase, + _ob_base: >::LayoutAsBase, contents: MaybeUninit>, } @@ -274,7 +277,7 @@ impl PyObjectInit for PyClassInitializer { impl From for PyClassInitializer where T: PyClass, - T::BaseType: PyClassBaseType>, + T::BaseType: PyClassBaseType>, { #[inline] fn from(value: T) -> PyClassInitializer { diff --git a/src/types/mod.rs b/src/types/mod.rs index 1058a16a..2aa889ff 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -196,8 +196,8 @@ macro_rules! pyobject_native_type_sized { ($name:ty, $layout:path $(;$generics:ident)*) => { unsafe impl $crate::type_object::PyLayout<$name> for $layout {} impl $crate::type_object::PySizedLayout<$name> for $layout {} - impl<'a, $($generics,)*> $crate::impl_::pyclass::PyClassBaseType for $name { - type LayoutAsBase = $crate::pycell::PyCellBase<$layout>; + impl<'a, $($generics,)*> $crate::impl_::pyclass::PyClassBaseType<$crate::pycell::Mutable> for $name { + type LayoutAsBase = $crate::pycell::PyCellBase<$layout, $crate::pycell::Mutable>; type BaseNativeType = $name; type ThreadChecker = $crate::impl_::pyclass::ThreadCheckerStub<$crate::PyObject>; type Initializer = $crate::pyclass_init::PyNativeTypeInitializer; diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 2fef7685..2af1c9ff 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -28,8 +28,12 @@ fn _test_compile_errors() { t.compile_fail("tests/ui/invalid_pymethod_names.rs"); t.compile_fail("tests/ui/invalid_pymodule_args.rs"); t.compile_fail("tests/ui/reject_generics.rs"); +<<<<<<< HEAD t.compile_fail("tests/ui/invalid_pymethod_proto_args.rs"); t.compile_fail("tests/ui/invalid_pymethod_proto_args_py.rs"); +======= + t.compile_fail("tests/ui/invalid_immutable_pyclass_borrow.rs"); +>>>>>>> 0fa03a67cd (Implement opt-in immutable pyclasses) tests_rust_1_49(&t); tests_rust_1_56(&t); diff --git a/tests/ui/invalid_immutable_pyclass_borrow.rs b/tests/ui/invalid_immutable_pyclass_borrow.rs new file mode 100644 index 00000000..d020fbec --- /dev/null +++ b/tests/ui/invalid_immutable_pyclass_borrow.rs @@ -0,0 +1,13 @@ +use pyo3::prelude::*; + +#[pyclass(immutable)] +pub struct Foo { + #[pyo3(get)] + field: u32, +} + +fn borrow_mut_fails(foo: Py, py: Python){ + let borrow = foo.as_ref(py).borrow_mut(); +} + +fn main(){} \ No newline at end of file diff --git a/tests/ui/invalid_immutable_pyclass_borrow.stderr b/tests/ui/invalid_immutable_pyclass_borrow.stderr new file mode 100644 index 00000000..3574139a --- /dev/null +++ b/tests/ui/invalid_immutable_pyclass_borrow.stderr @@ -0,0 +1,5 @@ +error[E0277]: the trait bound `Foo: MutablePyClass` is not satisfied + --> tests/ui/invalid_immutable_pyclass_borrow.rs:10:33 + | +10 | let borrow = foo.as_ref(py).borrow_mut(); + | ^^^^^^^^^^ the trait `MutablePyClass` is not implemented for `Foo` diff --git a/tests/ui/pyclass_send.stderr b/tests/ui/pyclass_send.stderr index 287430ac..7e5addac 100644 --- a/tests/ui/pyclass_send.stderr +++ b/tests/ui/pyclass_send.stderr @@ -11,8 +11,20 @@ note: required because it appears within the type `NotThreadSafe` 5 | struct NotThreadSafe { | ^^^^^^^^^^^^^ note: required by a bound in `ThreadCheckerStub` +<<<<<<< HEAD +<<<<<<< HEAD --> src/impl_/pyclass.rs | | pub struct ThreadCheckerStub(PhantomData); +======= + --> src/class/impl_.rs:731:33 + | +731 | pub struct ThreadCheckerStub(PhantomData); +>>>>>>> 0fa03a67cd (Implement opt-in immutable pyclasses) +======= + --> src/class/impl_.rs:728:33 + | +728 | pub struct ThreadCheckerStub(PhantomData); +>>>>>>> 7cded1178d (Fix formatting.) | ^^^^ required by this bound in `ThreadCheckerStub` = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)