Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pyclass: move flags to PyClassImpl #1456

Merged
merged 2 commits into from
Mar 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- `PyMethodDef_INIT` [#1426](https://github.com/PyO3/pyo3/pull/1426)
- `PyTypeObject_INIT` [#1429](https://github.com/PyO3/pyo3/pull/1429)
- `PyObject_Check`, `PySuper_Check`, and `FreeFunc` [#1438](https://github.com/PyO3/pyo3/pull/1438)
- Remove pyclass implementation details `Type`, `DESCRIPTION`, and `FLAGS` from `PyTypeInfo`. [#1456](https://github.com/PyO3/pyo3/pull/1456)

### Fixed
- Remove FFI definition `PyCFunction_ClearFreeList` for Python 3.9 and later. [#1425](https://github.com/PyO3/pyo3/pull/1425)
Expand Down
15 changes: 8 additions & 7 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ struct MyClass {
impl pyo3::pyclass::PyClassAlloc for MyClass {}

unsafe impl pyo3::PyTypeInfo for MyClass {
type Type = MyClass;
type BaseType = PyAny;
type BaseLayout = pyo3::pycell::PyCellBase<PyAny>;
type Layout = PyCell<Self>;
Expand All @@ -735,8 +734,6 @@ unsafe impl pyo3::PyTypeInfo for MyClass {

const NAME: &'static str = "MyClass";
const MODULE: Option<&'static str> = None;
const DESCRIPTION: &'static str = "Class for demonstration";
const FLAGS: usize = 0;

#[inline]
fn type_object_raw(py: pyo3::Python) -> *mut pyo3::ffi::PyTypeObject {
Expand All @@ -759,6 +756,10 @@ impl pyo3::IntoPy<PyObject> for MyClass {
}

impl pyo3::class::impl_::PyClassImpl for MyClass {
const DOC: &'static str = "Class for demonstration";
const IS_GC: bool = false;
const IS_BASETYPE: bool = false;
const IS_SUBCLASS: bool = false;
type ThreadChecker = pyo3::class::impl_::ThreadCheckerStub<MyClass>;

fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) {
Expand All @@ -776,18 +777,18 @@ impl pyo3::class::impl_::PyClassImpl for MyClass {
}
fn get_new() -> Option<pyo3::ffi::newfunc> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<MyClass>::new();
let collector = PyClassImplCollector::<Self>::new();
collector.new_impl()
}
fn get_call() -> Option<pyo3::ffi::PyCFunctionWithKeywords> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<MyClass>::new();
let collector = PyClassImplCollector::<Self>::new();
collector.call_impl()
}
fn for_each_proto_slot(visitor: impl FnMut(&pyo3::ffi::PyType_Slot)) {
// Implementation which uses dtolnay specialization to load all slots.
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<MyClass>::new();
let collector = PyClassImplCollector::<Self>::new();
collector.object_protocol_slots()
.iter()
.chain(collector.number_protocol_slots())
Expand All @@ -803,7 +804,7 @@ impl pyo3::class::impl_::PyClassImpl for MyClass {

fn get_buffer() -> Option<&'static pyo3::class::impl_::PyBufferProcs> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<MyClass>::new();
let collector = PyClassImplCollector::<Self>::new();
collector.buffer_procs()
}
}
Expand Down
70 changes: 32 additions & 38 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ use syn::{parse_quote, spanned::Spanned, Expr, Token};
pub struct PyClassArgs {
pub freelist: Option<syn::Expr>,
pub name: Option<syn::Ident>,
pub flags: Vec<syn::Expr>,
pub base: syn::TypePath,
pub has_dict: bool,
pub has_weaklist: bool,
pub is_gc: bool,
pub is_basetype: bool,
pub has_extends: bool,
pub has_unsendable: bool,
pub module: Option<syn::LitStr>,
Expand All @@ -42,10 +45,11 @@ impl Default for PyClassArgs {
freelist: None,
name: None,
module: None,
// We need the 0 as value for the constant we're later building using quote for when there
// are no other flags
flags: vec![parse_quote! { 0 }],
base: parse_quote! { pyo3::PyAny },
has_dict: false,
has_weaklist: false,
is_gc: false,
is_basetype: false,
has_extends: false,
has_unsendable: false,
}
Expand Down Expand Up @@ -136,14 +140,19 @@ impl PyClassArgs {
/// Match a single flag
fn add_path(&mut self, exp: &syn::ExprPath) -> syn::Result<()> {
let flag = exp.path.segments.first().unwrap().ident.to_string();
let mut push_flag = |flag| {
self.flags.push(syn::Expr::Path(flag));
};
match flag.as_str() {
"gc" => push_flag(parse_quote! {pyo3::type_flags::GC}),
"weakref" => push_flag(parse_quote! {pyo3::type_flags::WEAKREF}),
"subclass" => push_flag(parse_quote! {pyo3::type_flags::BASETYPE}),
"dict" => push_flag(parse_quote! {pyo3::type_flags::DICT}),
"gc" => {
self.is_gc = true;
}
"weakref" => {
self.has_weaklist = true;
}
"subclass" => {
self.is_basetype = true;
}
"dict" => {
self.has_dict = true;
}
"unsendable" => {
self.has_unsendable = true;
}
Expand Down Expand Up @@ -293,29 +302,14 @@ fn impl_class(
};

// insert space for weak ref
let mut has_weakref = false;
let mut has_dict = false;
let mut has_gc = false;
for f in attr.flags.iter() {
if let syn::Expr::Path(epath) = f {
if epath.path == parse_quote! { pyo3::type_flags::WEAKREF } {
has_weakref = true;
} else if epath.path == parse_quote! { pyo3::type_flags::DICT } {
has_dict = true;
} else if epath.path == parse_quote! { pyo3::type_flags::GC } {
has_gc = true;
}
}
}

let weakref = if has_weakref {
let weakref = if attr.has_weaklist {
quote! { pyo3::pyclass_slots::PyClassWeakRefSlot }
} else if attr.has_extends {
quote! { <Self::BaseType as pyo3::derive_utils::PyBaseTypeUtils>::WeakRef }
} else {
quote! { pyo3::pyclass_slots::PyClassDummySlot }
};
let dict = if has_dict {
let dict = if attr.has_dict {
quote! { pyo3::pyclass_slots::PyClassDictSlot }
} else if attr.has_extends {
quote! { <Self::BaseType as pyo3::derive_utils::PyBaseTypeUtils>::Dict }
Expand All @@ -329,7 +323,7 @@ fn impl_class(
};

// Enforce at compile time that PyGCProtocol is implemented
let gc_impl = if has_gc {
let gc_impl = if attr.is_gc {
let closure_name = format!("__assertion_closure_{}", cls);
let closure_token = syn::Ident::new(&closure_name, Span::call_site());
quote! {
Expand Down Expand Up @@ -357,12 +351,6 @@ fn impl_class(
};

let base = &attr.base;
let flags = &attr.flags;
let extended = if attr.has_extends {
quote! { pyo3::type_flags::EXTENDED }
} else {
quote! { 0 }
};
let base_layout = if attr.has_extends {
quote! { <Self::BaseType as pyo3::derive_utils::PyBaseTypeUtils>::LayoutAsBase }
} else {
Expand Down Expand Up @@ -397,9 +385,12 @@ fn impl_class(
quote! { pyo3::class::impl_::ThreadCheckerStub<#cls> }
};

let is_gc = attr.is_gc;
let is_basetype = attr.is_basetype;
let is_subclass = attr.has_extends;

Ok(quote! {
unsafe impl pyo3::type_object::PyTypeInfo for #cls {
type Type = #cls;
type BaseType = #base;
type Layout = pyo3::PyCell<Self>;
type BaseLayout = #base_layout;
Expand All @@ -408,8 +399,6 @@ fn impl_class(

const NAME: &'static str = #cls_name;
const MODULE: Option<&'static str> = #module;
const DESCRIPTION: &'static str = #doc;
const FLAGS: usize = #(#flags)|* | #extended;

#[inline]
fn type_object_raw(py: pyo3::Python) -> *mut pyo3::ffi::PyTypeObject {
Expand Down Expand Up @@ -440,6 +429,11 @@ fn impl_class(
#impl_inventory

impl pyo3::class::impl_::PyClassImpl for #cls {
const DOC: &'static str = #doc;
const IS_GC: bool = #is_gc;
const IS_BASETYPE: bool = #is_basetype;
const IS_SUBCLASS: bool = #is_subclass;

type ThreadChecker = #thread_checker;

fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) {
Expand Down
12 changes: 12 additions & 0 deletions src/class/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ impl<T> Copy for PyClassImplCollector<T> {}
/// 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 {
/// Class doc string
const DOC: &'static str = "\0";

/// #[pyclass(gc)]
const IS_GC: bool = false;

/// #[pyclass(subclass)]
const IS_BASETYPE: bool = false;

/// #[pyclass(extends=...)]
const IS_SUBCLASS: bool = false;

/// This handles following two situations:
/// 1. In case `T` is `Send`, stub `ThreadChecker` is used and does nothing.
/// This implementation is used by default. Compile fails if `T: !Send`.
Expand Down
3 changes: 2 additions & 1 deletion src/freelist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

//! Free allocation list

use crate::class::impl_::PyClassImpl;
use crate::pyclass::{get_type_free, tp_free_fallback, PyClassAlloc};
use crate::type_object::{PyLayout, PyTypeInfo};
use crate::{ffi, AsPyPointer, FromPyPointer, PyAny, Python};
Expand Down Expand Up @@ -69,7 +70,7 @@ impl<T> FreeList<T> {

impl<T> PyClassAlloc for T
where
T: PyTypeInfo + PyClassWithFreeList,
T: PyTypeInfo + PyClassImpl + PyClassWithFreeList,
{
unsafe fn new(py: Python, subtype: *mut ffi::PyTypeObject) -> *mut Self::Layout {
// if subtype is not equal to this type, we cannot use the freelist
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ pub use crate::pycell::{PyCell, PyRef, PyRefMut};
pub use crate::pyclass::PyClass;
pub use crate::pyclass_init::PyClassInitializer;
pub use crate::python::{Python, PythonVersionInfo};
pub use crate::type_object::{type_flags, PyTypeInfo};
pub use crate::type_object::PyTypeInfo;
// Since PyAny is as important as PyObject, we expose it to the top level.
pub use crate::types::PyAny;

Expand Down
20 changes: 10 additions & 10 deletions src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::class::impl_::PyClassImpl;
use crate::class::methods::PyMethodDefType;
use crate::pyclass_slots::{PyClassDict, PyClassWeakRef};
use crate::type_object::{type_flags, PyLayout};
use crate::type_object::PyLayout;
use crate::{ffi, PyCell, PyErr, PyNativeType, PyResult, PyTypeInfo, Python};
use std::convert::TryInto;
use std::ffi::CString;
Expand Down Expand Up @@ -40,12 +40,12 @@ pub(crate) unsafe fn bpo_35810_workaround(_py: Python, ty: *mut ffi::PyTypeObjec
}

#[inline]
pub(crate) unsafe fn default_new<T: PyTypeInfo>(
pub(crate) unsafe fn default_new<T: PyTypeInfo + PyClassImpl>(
py: Python,
subtype: *mut ffi::PyTypeObject,
) -> *mut ffi::PyObject {
// if the class derives native types(e.g., PyDict), call special new
if T::FLAGS & type_flags::EXTENDED != 0 && T::BaseLayout::IS_NATIVE_TYPE {
if T::IS_SUBCLASS && T::BaseLayout::IS_NATIVE_TYPE {
#[cfg(not(Py_LIMITED_API))]
{
let base_tp = T::BaseType::type_object_raw(py);
Expand All @@ -70,7 +70,7 @@ pub(crate) unsafe fn default_new<T: PyTypeInfo>(
}

/// This trait enables custom `tp_new`/`tp_dealloc` implementations for `T: PyClass`.
pub trait PyClassAlloc: PyTypeInfo + Sized {
pub trait PyClassAlloc: PyTypeInfo + PyClassImpl {
/// Allocate the actual field for `#[pyclass]`.
///
/// # Safety
Expand Down Expand Up @@ -159,7 +159,7 @@ impl TypeSlots {
}

fn tp_doc<T: PyClass>() -> PyResult<Option<*mut c_void>> {
Ok(match T::DESCRIPTION {
Ok(match T::DOC {
"\0" => None,
s if s.as_bytes().ends_with(b"\0") => Some(s.as_ptr() as _),
// If the description is not null-terminated, create CString and leak it
Expand Down Expand Up @@ -253,15 +253,15 @@ fn tp_init_additional<T: PyClass>(type_object: *mut ffi::PyTypeObject) {
// Running this causes PyPy to segfault.
#[cfg(all(not(PyPy), not(Py_3_10)))]
{
if T::DESCRIPTION != "\0" {
if T::DOC != "\0" {
unsafe {
// Until CPython 3.10, tp_doc was treated specially for
// heap-types, and it removed the text_signature value from it.
// We go in after the fact and replace tp_doc with something
// that _does_ include the text_signature value!
ffi::PyObject_Free((*type_object).tp_doc as _);
let data = ffi::PyObject_Malloc(T::DESCRIPTION.len());
data.copy_from(T::DESCRIPTION.as_ptr() as _, T::DESCRIPTION.len());
let data = ffi::PyObject_Malloc(T::DOC.len());
data.copy_from(T::DOC.as_ptr() as _, T::DOC.len());
(*type_object).tp_doc = data as _;
}
}
Expand Down Expand Up @@ -300,12 +300,12 @@ fn tp_init_additional<T: PyClass>(type_object: *mut ffi::PyTypeObject) {
fn tp_init_additional<T: PyClass>(_type_object: *mut ffi::PyTypeObject) {}

fn py_class_flags<T: PyClass + PyTypeInfo>(has_gc_methods: bool) -> c_uint {
let mut flags = if has_gc_methods || T::FLAGS & type_flags::GC != 0 {
let mut flags = if has_gc_methods || T::IS_GC {
ffi::Py_TPFLAGS_DEFAULT | ffi::Py_TPFLAGS_HAVE_GC
} else {
ffi::Py_TPFLAGS_DEFAULT
};
if T::FLAGS & type_flags::BASETYPE != 0 {
if T::IS_BASETYPE {
flags |= ffi::Py_TPFLAGS_BASETYPE;
}
flags.try_into().unwrap()
Expand Down
28 changes: 0 additions & 28 deletions src/type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,47 +46,19 @@ pub trait PySizedLayout<T: PyTypeInfo>: PyLayout<T> + Sized {}
/// Otherwise, implementing this trait is undefined behavior.
pub unsafe trait PyBorrowFlagLayout<T: PyTypeInfo>: PyLayout<T> + Sized {}

/// Our custom type flags
#[doc(hidden)]
pub mod type_flags {
/// Type object supports Python GC
pub const GC: usize = 1;

/// Type object supports Python weak references
pub const WEAKREF: usize = 1 << 1;

/// Type object can be used as the base type of another type
pub const BASETYPE: usize = 1 << 2;

/// The instances of this type have a dictionary containing instance variables
pub const DICT: usize = 1 << 3;

/// The class declared by #[pyclass(extends=~)]
pub const EXTENDED: usize = 1 << 4;
}

/// Python type information.
/// All Python native types(e.g., `PyDict`) and `#[pyclass]` structs implement this trait.
///
/// This trait is marked unsafe because:
/// - specifying the incorrect layout can lead to memory errors
/// - the return value of type_object must always point to the same PyTypeObject instance
pub unsafe trait PyTypeInfo: Sized {
/// Type of objects to store in PyObject struct
type Type;

/// Class name
const NAME: &'static str;

/// Module name, if any
const MODULE: Option<&'static str>;

/// Class doc string
const DESCRIPTION: &'static str = "\0";

/// Type flags (ie PY_TYPE_FLAG_GC, PY_TYPE_FLAG_WEAKREF)
const FLAGS: usize = 0;

/// Base class
type BaseType: PyTypeInfo + PyTypeObject;

Expand Down
1 change: 0 additions & 1 deletion src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ macro_rules! pyobject_native_type_info(
($name: ty, $layout: path, $typeobject: expr,
$module: expr $(, $checkfunction:path)? $(;$generics: ident)*) => {
unsafe impl<$($generics,)*> $crate::type_object::PyTypeInfo for $name {
type Type = ();
type BaseType = $crate::PyAny;
type Layout = $layout;
type BaseLayout = $crate::ffi::PyObject;
Expand Down