diff --git a/guide/src/class.md b/guide/src/class.md index 3e8d8c82..e284efc4 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -754,14 +754,14 @@ impl pyo3::IntoPy for MyClass { } pub struct Pyo3MethodsInventoryForMyClass { - methods: &'static [pyo3::class::PyMethodDefType], + methods: Vec, } impl pyo3::class::methods::PyMethodsInventory for Pyo3MethodsInventoryForMyClass { - fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self { + fn new(methods: Vec) -> Self { Self { methods } } - fn get(&self) -> &'static [pyo3::class::PyMethodDefType] { - self.methods + fn get(&'static self) -> &'static [pyo3::class::PyMethodDefType] { + &self.methods } } impl pyo3::class::methods::HasMethodsInventory for MyClass { diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 693d5004..bc2fd66e 100644 --- a/pyo3-derive-backend/src/pyclass.rs +++ b/pyo3-derive-backend/src/pyclass.rs @@ -215,14 +215,14 @@ fn impl_methods_inventory(cls: &syn::Ident) -> TokenStream { quote! { #[doc(hidden)] pub struct #inventory_cls { - methods: &'static [pyo3::class::PyMethodDefType], + methods: Vec, } impl pyo3::class::methods::PyMethodsInventory for #inventory_cls { - fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self { + fn new(methods: Vec) -> Self { Self { methods } } - fn get(&self) -> &'static [pyo3::class::PyMethodDefType] { - self.methods + fn get(&'static self) -> &'static [pyo3::class::PyMethodDefType] { + &self.methods } } @@ -483,7 +483,7 @@ fn impl_descriptors( pyo3::inventory::submit! { #![crate = pyo3] { type Inventory = <#cls as pyo3::class::methods::HasMethodsInventory>::Methods; - ::new(&[#(#py_methods),*]) + ::new(vec![#(#py_methods),*]) } } }) diff --git a/pyo3-derive-backend/src/pyimpl.rs b/pyo3-derive-backend/src/pyimpl.rs index 00999af5..e59cb43f 100644 --- a/pyo3-derive-backend/src/pyimpl.rs +++ b/pyo3-derive-backend/src/pyimpl.rs @@ -43,7 +43,7 @@ pub fn impl_methods(ty: &syn::Type, impls: &mut Vec) -> syn::Resu pyo3::inventory::submit! { #![crate = pyo3] { type Inventory = <#ty as pyo3::class::methods::HasMethodsInventory>::Methods; - ::new(&[#( + ::new(vec![#( #(#cfg_attributes)* #methods ),*]) diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 62ced447..6afb66ef 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -570,12 +570,11 @@ pub fn impl_py_method_def(spec: &FnSpec, wrapper: &TokenStream) -> TokenStream { pyo3::class::PyMethodDefType::Method({ #wrapper - pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyCFunction(__wrap), - ml_flags: pyo3::ffi::METH_NOARGS, - ml_doc: #doc, - } + pyo3::class::PyMethodDef::cfunction( + concat!(stringify!(#python_name), "\0"), + __wrap, + #doc + ) }) } } else { @@ -583,12 +582,12 @@ pub fn impl_py_method_def(spec: &FnSpec, wrapper: &TokenStream) -> TokenStream { pyo3::class::PyMethodDefType::Method({ #wrapper - pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS, - ml_doc: #doc, - } + pyo3::class::PyMethodDef::cfunction_with_keywords( + concat!(stringify!(#python_name), "\0"), + __wrap, + 0, + #doc + ) }) } } @@ -601,12 +600,7 @@ pub fn impl_py_method_def_new(spec: &FnSpec, wrapper: &TokenStream) -> TokenStre pyo3::class::PyMethodDefType::New({ #wrapper - pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyNewFunc(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS, - ml_doc: #doc, - } + pyo3::class::PyMethodDef::new_func(concat!(stringify!(#python_name), "\0"), __wrap, #doc) }) } } @@ -618,13 +612,12 @@ pub fn impl_py_method_def_class(spec: &FnSpec, wrapper: &TokenStream) -> TokenSt pyo3::class::PyMethodDefType::Class({ #wrapper - pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | + pyo3::class::PyMethodDef::cfunction_with_keywords( + concat!(stringify!(#python_name), "\0"), + __wrap, pyo3::ffi::METH_CLASS, - ml_doc: #doc, - } + #doc + ) }) } } @@ -636,12 +629,12 @@ pub fn impl_py_method_def_static(spec: &FnSpec, wrapper: &TokenStream) -> TokenS pyo3::class::PyMethodDefType::Static({ #wrapper - pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | pyo3::ffi::METH_STATIC, - ml_doc: #doc, - } + pyo3::class::PyMethodDef::cfunction_with_keywords( + concat!(stringify!(#python_name), "\0"), + __wrap, + pyo3::ffi::METH_STATIC, + #doc + ) }) } } @@ -652,10 +645,7 @@ pub fn impl_py_method_class_attribute(spec: &FnSpec<'_>, wrapper: &TokenStream) pyo3::class::PyMethodDefType::ClassAttribute({ #wrapper - pyo3::class::PyClassAttributeDef { - name: stringify!(#python_name), - meth: __wrap, - } + pyo3::class::PyClassAttributeDef::new(concat!(stringify!(#python_name), "\0"), __wrap) }) } } @@ -666,10 +656,7 @@ pub fn impl_py_const_class_attribute(spec: &ConstSpec, wrapper: &TokenStream) -> pyo3::class::PyMethodDefType::ClassAttribute({ #wrapper - pyo3::class::PyClassAttributeDef { - name: stringify!(#python_name), - meth: __wrap, - } + pyo3::class::PyClassAttributeDef::new(concat!(stringify!(#python_name), "\0"), __wrap) }) } } @@ -681,12 +668,12 @@ pub fn impl_py_method_def_call(spec: &FnSpec, wrapper: &TokenStream) -> TokenStr pyo3::class::PyMethodDefType::Call({ #wrapper - pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS, - ml_doc: #doc, - } + pyo3::class::PyMethodDef::cfunction_with_keywords( + concat!(stringify!(#python_name), "\0"), + __wrap, + pyo3::ffi::METH_STATIC, + #doc + ) }) } } @@ -700,11 +687,7 @@ pub(crate) fn impl_py_setter_def( pyo3::class::PyMethodDefType::Setter({ #wrapper - pyo3::class::PySetterDef { - name: stringify!(#python_name), - meth: __wrap, - doc: #doc, - } + pyo3::class::PySetterDef::new(concat!(stringify!(#python_name), "\0"), __wrap, #doc) }) } } @@ -718,11 +701,7 @@ pub(crate) fn impl_py_getter_def( pyo3::class::PyMethodDefType::Getter({ #wrapper - pyo3::class::PyGetterDef { - name: stringify!(#python_name), - meth: __wrap, - doc: #doc, - } + pyo3::class::PyGetterDef::new(concat!(stringify!(#python_name), "\0"), __wrap, #doc) }) } } diff --git a/pyo3-derive-backend/src/pyproto.rs b/pyo3-derive-backend/src/pyproto.rs index 67976f59..7c6cbf1a 100644 --- a/pyo3-derive-backend/src/pyproto.rs +++ b/pyo3-derive-backend/src/pyproto.rs @@ -95,12 +95,12 @@ fn impl_proto_impl( py_methods.push(quote! { pyo3::class::PyMethodDefType::Method({ #method - pyo3::class::PyMethodDef { - ml_name: stringify!(#name), - ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | #coexist, - ml_doc: "\0" - } + pyo3::class::PyMethodDef::cfunction_with_keywords( + concat!(stringify!(#name), "\0"), + __wrap, + #coexist, + "\0" + ) }) }); } @@ -123,7 +123,7 @@ fn inventory_submission(py_methods: Vec, ty: &syn::Type) -> TokenSt pyo3::inventory::submit! { #![crate = pyo3] { type Inventory = <#ty as pyo3::class::methods::HasMethodsInventory>::Methods; - ::new(&[#(#py_methods),*]) + ::new(vec![#(#py_methods),*]) } } } diff --git a/src/class/methods.rs b/src/class/methods.rs index 173dfe02..4713d2ac 100644 --- a/src/class/methods.rs +++ b/src/class/methods.rs @@ -2,7 +2,7 @@ use crate::{ffi, PyObject, Python}; use libc::c_int; -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use std::fmt; /// `PyMethodDefType` represents different types of Python callable objects. @@ -35,32 +35,33 @@ pub enum PyMethodType { PyInitFunc(ffi::initproc), } -#[derive(Copy, Clone, Debug)] +// TODO(kngwyu): We should also use &'static CStr for this? I'm not sure. +#[derive(Clone, Debug)] pub struct PyMethodDef { - pub ml_name: &'static str, - pub ml_meth: PyMethodType, - pub ml_flags: c_int, - pub ml_doc: &'static str, + ml_name: &'static CStr, + ml_meth: PyMethodType, + ml_flags: c_int, + ml_doc: &'static CStr, } #[derive(Copy, Clone)] pub struct PyClassAttributeDef { - pub name: &'static str, - pub meth: for<'p> fn(Python<'p>) -> PyObject, + pub(crate) name: &'static CStr, + pub(crate) meth: for<'p> fn(Python<'p>) -> PyObject, } -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub struct PyGetterDef { - pub name: &'static str, - pub meth: ffi::getter, - pub doc: &'static str, + pub(crate) name: &'static CStr, + pub(crate) meth: ffi::getter, + doc: &'static CStr, } -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub struct PySetterDef { - pub name: &'static str, - pub meth: ffi::setter, - pub doc: &'static str, + pub(crate) name: &'static CStr, + pub(crate) meth: ffi::setter, + doc: &'static CStr, } unsafe impl Sync for PyMethodDef {} @@ -73,19 +74,80 @@ unsafe impl Sync for PySetterDef {} unsafe impl Sync for ffi::PyGetSetDef {} -fn get_name(name: &str) -> *const std::os::raw::c_char { - CString::new(name) - .expect("Method name must not contain NULL byte") - .into_raw() as _ +fn get_name(name: &'static str) -> &'static CStr { + CStr::from_bytes_with_nul(name.as_bytes()) + .expect("Method name must be terminated with NULL byte") } -fn get_doc(doc: &'static str) -> *const std::os::raw::c_char { - CStr::from_bytes_with_nul(doc.as_bytes()) - .expect("Document must be terminated with NULL byte") - .as_ptr() +fn get_doc(doc: &'static str) -> &'static CStr { + CStr::from_bytes_with_nul(doc.as_bytes()).expect("Document must be terminated with NULL byte") } impl PyMethodDef { + pub(crate) fn get_new_func(&self) -> Option { + if let PyMethodType::PyNewFunc(new_func) = self.ml_meth { + Some(new_func) + } else { + None + } + } + + pub(crate) fn get_cfunction_with_keywords(&self) -> Option { + if let PyMethodType::PyCFunctionWithKeywords(func) = self.ml_meth { + Some(func) + } else { + None + } + } + + pub fn cfunction(name: &'static str, cfunction: ffi::PyCFunction, doc: &'static str) -> Self { + Self::new( + name, + PyMethodType::PyCFunction(cfunction), + ffi::METH_NOARGS, + doc, + ) + } + + pub fn new_func(name: &'static str, newfunc: ffi::newfunc, doc: &'static str) -> Self { + Self::new( + name, + PyMethodType::PyNewFunc(newfunc), + ffi::METH_VARARGS | ffi::METH_KEYWORDS, + doc, + ) + } + + /// Define a function that can take `**kwargs`. + pub fn cfunction_with_keywords( + name: &'static str, + cfunction: ffi::PyCFunctionWithKeywords, + flags: c_int, + doc: &'static str, + ) -> Self { + let flags = flags | ffi::METH_VARARGS | ffi::METH_KEYWORDS; + Self::new( + name, + PyMethodType::PyCFunctionWithKeywords(cfunction), + flags, + doc, + ) + } + + pub(crate) fn new( + name: &'static str, + methodtype: PyMethodType, + flags: c_int, + doc: &'static str, + ) -> Self { + Self { + ml_name: get_name(name), + ml_meth: methodtype, + ml_flags: flags, + ml_doc: get_doc(doc), + } + } + /// Convert `PyMethodDef` to Python method definition struct `ffi::PyMethodDef` pub fn as_method_def(&self) -> ffi::PyMethodDef { let meth = match self.ml_meth { @@ -96,10 +158,19 @@ impl PyMethodDef { }; ffi::PyMethodDef { - ml_name: get_name(self.ml_name), + ml_name: self.ml_name.as_ptr(), ml_meth: Some(meth), ml_flags: self.ml_flags, - ml_doc: get_doc(self.ml_doc), + ml_doc: self.ml_doc.as_ptr(), + } + } +} + +impl PyClassAttributeDef { + pub fn new(name: &'static str, meth: for<'p> fn(Python<'p>) -> PyObject) -> Self { + Self { + name: get_name(name), + meth, } } } @@ -115,26 +186,44 @@ impl fmt::Debug for PyClassAttributeDef { } impl PyGetterDef { + /// Define a getter. + pub fn new(name: &'static str, getter: ffi::getter, doc: &'static str) -> Self { + Self { + name: get_name(name), + meth: getter, + doc: get_doc(doc), + } + } + /// Copy descriptor information to `ffi::PyGetSetDef` pub fn copy_to(&self, dst: &mut ffi::PyGetSetDef) { if dst.name.is_null() { - dst.name = get_name(self.name) as _; + dst.name = self.name.as_ptr() as _; } if dst.doc.is_null() { - dst.doc = get_doc(self.doc) as _; + dst.doc = self.doc.as_ptr() as _; } dst.get = Some(self.meth); } } impl PySetterDef { + /// Define a setter. + pub fn new(name: &'static str, setter: ffi::setter, doc: &'static str) -> Self { + Self { + name: get_name(name), + meth: setter, + doc: get_doc(doc), + } + } + /// Copy descriptor information to `ffi::PyGetSetDef` pub fn copy_to(&self, dst: &mut ffi::PyGetSetDef) { if dst.name.is_null() { - dst.name = get_name(self.name) as _; + dst.name = self.name.as_ptr() as _; } if dst.doc.is_null() { - dst.doc = get_doc(self.doc) as _; + dst.doc = self.doc.as_ptr() as _; } dst.set = Some(self.meth); } @@ -156,10 +245,10 @@ pub trait PyMethods { #[cfg(feature = "macros")] pub trait PyMethodsInventory: inventory::Collect { /// Create a new instance - fn new(methods: &'static [PyMethodDefType]) -> Self; + fn new(methods: Vec) -> Self; /// Returns the methods for a single `#[pymethods] impl` block - fn get(&self) -> &'static [PyMethodDefType]; + fn get(&'static self) -> &'static [PyMethodDefType]; } /// Implemented for `#[pyclass]` in our proc macro code. diff --git a/src/pyclass.rs b/src/pyclass.rs index 52cc3910..11e4060d 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -6,7 +6,7 @@ use crate::derive_utils::PyBaseTypeUtils; use crate::pyclass_slots::{PyClassDict, PyClassWeakRef}; use crate::type_object::{type_flags, PyLayout}; use crate::types::PyAny; -use crate::{class, ffi, PyCell, PyErr, PyNativeType, PyResult, PyTypeInfo, Python}; +use crate::{ffi, PyCell, PyErr, PyNativeType, PyResult, PyTypeInfo, Python}; use std::ffi::CString; use std::marker::PhantomData; use std::os::raw::c_void; @@ -239,7 +239,7 @@ fn py_class_flags(type_object: &mut ffi::PyTypeObject) { pub(crate) fn py_class_attributes() -> impl Iterator { T::py_methods().into_iter().filter_map(|def| match def { - PyMethodDefType::ClassAttribute(attr) => Some(*attr), + PyMethodDefType::ClassAttribute(attr) => Some(attr.to_owned()), _ => None, }) } @@ -256,16 +256,12 @@ fn py_class_method_defs() -> ( for def in T::py_methods() { match *def { PyMethodDefType::New(ref def) => { - if let class::methods::PyMethodType::PyNewFunc(meth) = def.ml_meth { - new = Some(meth) - } + new = def.get_new_func(); + debug_assert!(new.is_some()); } PyMethodDefType::Call(ref def) => { - if let class::methods::PyMethodType::PyCFunctionWithKeywords(meth) = def.ml_meth { - call = Some(meth) - } else { - panic!("Method type is not supoorted by tp_call slot") - } + call = def.get_cfunction_with_keywords(); + debug_assert!(call.is_some()); } PyMethodDefType::Method(ref def) | PyMethodDefType::Class(ref def) @@ -285,19 +281,17 @@ fn py_class_properties() -> Vec { for def in T::py_methods() { match *def { PyMethodDefType::Getter(ref getter) => { - let name = getter.name.to_string(); - if !defs.contains_key(&name) { - let _ = defs.insert(name.clone(), ffi::PyGetSetDef_INIT); + if !defs.contains_key(getter.name) { + let _ = defs.insert(getter.name.to_owned(), ffi::PyGetSetDef_INIT); } - let def = defs.get_mut(&name).expect("Failed to call get_mut"); + let def = defs.get_mut(getter.name).expect("Failed to call get_mut"); getter.copy_to(def); } PyMethodDefType::Setter(ref setter) => { - let name = setter.name.to_string(); - if !defs.contains_key(&name) { - let _ = defs.insert(name.clone(), ffi::PyGetSetDef_INIT); + if !defs.contains_key(setter.name) { + let _ = defs.insert(setter.name.to_owned(), ffi::PyGetSetDef_INIT); } - let def = defs.get_mut(&name).expect("Failed to call get_mut"); + let def = defs.get_mut(setter.name).expect("Failed to call get_mut"); setter.copy_to(def); } _ => (), diff --git a/src/type_object.rs b/src/type_object.rs index 456835d8..0a4a787b 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -227,18 +227,15 @@ impl LazyStaticType { fn initialize_tp_dict( py: Python, tp_dict: *mut ffi::PyObject, - items: Vec<(&'static str, PyObject)>, + items: Vec<(&'static std::ffi::CStr, PyObject)>, ) -> PyResult<()> { // We hold the GIL: the dictionary update can be considered atomic from // the POV of other threads. for (key, val) in items { - crate::types::with_tmp_string(py, key, |key| { - let ret = unsafe { ffi::PyDict_SetItem(tp_dict, key, val.into_ptr()) }; - if ret < 0 { - return Err(PyErr::fetch(py)); - } - Ok(()) - })?; + let ret = unsafe { ffi::PyDict_SetItemString(tp_dict, key.as_ptr(), val.into_ptr()) }; + if ret < 0 { + return Err(PyErr::fetch(py)); + } } Ok(()) }