Use &'static CStr for representing method names and docs

guide/src/class.md

@@ -754,14 +754,14 @@ impl pyo3::IntoPy<PyObject> for MyClass {
 }
 
 pub struct Pyo3MethodsInventoryForMyClass {
-    methods: &'static [pyo3::class::PyMethodDefType],
+    methods: Vec<pyo3::class::PyMethodDefType>,
 }
 impl pyo3::class::methods::PyMethodsInventory for Pyo3MethodsInventoryForMyClass {
-    fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
+    fn new(methods: Vec<pyo3::class::PyMethodDefType>) -> Self {
         Self { methods }
     }
-    fn get(&self) -> &'static [pyo3::class::PyMethodDefType] {
+    fn get(&'static self) -> &'static [pyo3::class::PyMethodDefType] {
-        self.methods
+        &self.methods
     }
 }
 impl pyo3::class::methods::HasMethodsInventory for MyClass {

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<pyo3::class::PyMethodDefType>,
         }
         impl pyo3::class::methods::PyMethodsInventory for #inventory_cls {
-            fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
+            fn new(methods: Vec<pyo3::class::PyMethodDefType>) -> Self {
                 Self { methods }
             }
-            fn get(&self) -> &'static [pyo3::class::PyMethodDefType] {
+            fn get(&'static self) -> &'static [pyo3::class::PyMethodDefType] {
-                self.methods
+                &self.methods
             }
         }
 
@@ -483,7 +483,7 @@ fn impl_descriptors(
         pyo3::inventory::submit! {
             #![crate = pyo3] {
                 type Inventory = <#cls as pyo3::class::methods::HasMethodsInventory>::Methods;
-                <Inventory as pyo3::class::methods::PyMethodsInventory>::new(&[#(#py_methods),*])
+                <Inventory as pyo3::class::methods::PyMethodsInventory>::new(vec![#(#py_methods),*])
             }
         }
     })

pyo3-derive-backend/src/pyimpl.rs

@@ -43,7 +43,7 @@ pub fn impl_methods(ty: &syn::Type, impls: &mut Vec<syn::ImplItem>) -> syn::Resu
        pyo3::inventory::submit! {
             #![crate = pyo3] {
                 type Inventory = <#ty as pyo3::class::methods::HasMethodsInventory>::Methods;
-                <Inventory as pyo3::class::methods::PyMethodsInventory>::new(&[#(
+                <Inventory as pyo3::class::methods::PyMethodsInventory>::new(vec![#(
                     #(#cfg_attributes)*
                     #methods
                 ),*])

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 {
+                pyo3::class::PyMethodDef::cfunction(
-                    ml_name: stringify!(#python_name),
+                    concat!(stringify!(#python_name), "\0"),
-                    ml_meth: pyo3::class::PyMethodType::PyCFunction(__wrap),
+                    __wrap,
-                    ml_flags: pyo3::ffi::METH_NOARGS,
+                    #doc
-                    ml_doc: #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 {
+                pyo3::class::PyMethodDef::cfunction_with_keywords(
-                    ml_name: stringify!(#python_name),
+                    concat!(stringify!(#python_name), "\0"),
-                    ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
+                    __wrap,
-                    ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS,
+                    0,
-                    ml_doc: #doc,
+                    #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 {
+            pyo3::class::PyMethodDef::new_func(concat!(stringify!(#python_name), "\0"), __wrap, #doc)
-                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,
-            }
         })
     }
 }
@@ -618,13 +612,12 @@ pub fn impl_py_method_def_class(spec: &FnSpec, wrapper: &TokenStream) -> TokenSt
         pyo3::class::PyMethodDefType::Class({
             #wrapper
 
-            pyo3::class::PyMethodDef {
+            pyo3::class::PyMethodDef::cfunction_with_keywords(
-                ml_name: stringify!(#python_name),
+                concat!(stringify!(#python_name), "\0"),
-                ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
+                __wrap,
-                ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS |
                 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 {
+            pyo3::class::PyMethodDef::cfunction_with_keywords(
-                ml_name: stringify!(#python_name),
+                concat!(stringify!(#python_name), "\0"),
-                ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
+                __wrap,
-                ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | pyo3::ffi::METH_STATIC,
+                pyo3::ffi::METH_STATIC,
-                ml_doc: #doc,
+                #doc
-            }
+            )
         })
     }
 }
@@ -652,10 +645,7 @@ pub fn impl_py_method_class_attribute(spec: &FnSpec<'_>, wrapper: &TokenStream)
         pyo3::class::PyMethodDefType::ClassAttribute({
             #wrapper
 
-            pyo3::class::PyClassAttributeDef {
+            pyo3::class::PyClassAttributeDef::new(concat!(stringify!(#python_name), "\0"), __wrap)
-                name: stringify!(#python_name),
-                meth: __wrap,
-            }
         })
     }
 }
@@ -666,10 +656,7 @@ pub fn impl_py_const_class_attribute(spec: &ConstSpec, wrapper: &TokenStream) ->
         pyo3::class::PyMethodDefType::ClassAttribute({
             #wrapper
 
-            pyo3::class::PyClassAttributeDef {
+            pyo3::class::PyClassAttributeDef::new(concat!(stringify!(#python_name), "\0"), __wrap)
-                name: stringify!(#python_name),
-                meth: __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 {
+            pyo3::class::PyMethodDef::cfunction_with_keywords(
-                ml_name: stringify!(#python_name),
+                concat!(stringify!(#python_name), "\0"),
-                ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
+                __wrap,
-                ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS,
+                pyo3::ffi::METH_STATIC,
-                ml_doc: #doc,
+                #doc
-            }
+            )
         })
     }
 }
@@ -700,11 +687,7 @@ pub(crate) fn impl_py_setter_def(
         pyo3::class::PyMethodDefType::Setter({
             #wrapper
 
-            pyo3::class::PySetterDef {
+            pyo3::class::PySetterDef::new(concat!(stringify!(#python_name), "\0"), __wrap, #doc)
-                name: stringify!(#python_name),
-                meth: __wrap,
-                doc: #doc,
-            }
         })
     }
 }
@@ -718,11 +701,7 @@ pub(crate) fn impl_py_getter_def(
         pyo3::class::PyMethodDefType::Getter({
             #wrapper
 
-            pyo3::class::PyGetterDef {
+            pyo3::class::PyGetterDef::new(concat!(stringify!(#python_name), "\0"), __wrap, #doc)
-                name: stringify!(#python_name),
-                meth: __wrap,
-                doc: #doc,
-            }
         })
     }
 }

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 {
+                        pyo3::class::PyMethodDef::cfunction_with_keywords(
-                            ml_name: stringify!(#name),
+                            concat!(stringify!(#name), "\0"),
-                            ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
+                            __wrap,
-                            ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | #coexist,
+                            #coexist,
-                            ml_doc: "\0"
+                            "\0"
-                        }
+                        )
                     })
                 });
             }
@@ -123,7 +123,7 @@ fn inventory_submission(py_methods: Vec<TokenStream>, ty: &syn::Type) -> TokenSt
         pyo3::inventory::submit! {
             #![crate = pyo3] {
                 type Inventory = <#ty as pyo3::class::methods::HasMethodsInventory>::Methods;
-                <Inventory as pyo3::class::methods::PyMethodsInventory>::new(&[#(#py_methods),*])
+                <Inventory as pyo3::class::methods::PyMethodsInventory>::new(vec![#(#py_methods),*])
             }
         }
     }

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,
+    ml_name: &'static CStr,
-    pub ml_meth: PyMethodType,
+    ml_meth: PyMethodType,
-    pub ml_flags: c_int,
+    ml_flags: c_int,
-    pub ml_doc: &'static str,
+    ml_doc: &'static CStr,
 }
 
 #[derive(Copy, Clone)]
 pub struct PyClassAttributeDef {
-    pub name: &'static str,
+    pub(crate) name: &'static CStr,
-    pub meth: for<'p> fn(Python<'p>) -> PyObject,
+    pub(crate) meth: for<'p> fn(Python<'p>) -> PyObject,
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct PyGetterDef {
-    pub name: &'static str,
+    pub(crate) name: &'static CStr,
-    pub meth: ffi::getter,
+    pub(crate) meth: ffi::getter,
-    pub doc: &'static str,
+    doc: &'static CStr,
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct PySetterDef {
-    pub name: &'static str,
+    pub(crate) name: &'static CStr,
-    pub meth: ffi::setter,
+    pub(crate) meth: ffi::setter,
-    pub doc: &'static str,
+    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 {
+fn get_name(name: &'static str) -> &'static CStr {
-    CString::new(name)
+    CStr::from_bytes_with_nul(name.as_bytes())
-        .expect("Method name must not contain NULL byte")
+        .expect("Method name must be terminated with NULL byte")
-        .into_raw() as _
 }
 
-fn get_doc(doc: &'static str) -> *const std::os::raw::c_char {
+fn get_doc(doc: &'static str) -> &'static CStr {
-    CStr::from_bytes_with_nul(doc.as_bytes())
+    CStr::from_bytes_with_nul(doc.as_bytes()).expect("Document must be terminated with NULL byte")
-        .expect("Document must be terminated with NULL byte")
-        .as_ptr()
 }
 
 impl PyMethodDef {
+    pub(crate) fn get_new_func(&self) -> Option<ffi::newfunc> {
+        if let PyMethodType::PyNewFunc(new_func) = self.ml_meth {
+            Some(new_func)
+        } else {
+            None
+        }
+    }
+
+    pub(crate) fn get_cfunction_with_keywords(&self) -> Option<ffi::PyCFunctionWithKeywords> {
+        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<PyMethodDefType>) -> 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.

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<T: PyTypeInfo>(type_object: &mut ffi::PyTypeObject) {
 
 pub(crate) fn py_class_attributes<T: PyMethods>() -> impl Iterator<Item = PyClassAttributeDef> {
     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<T: PyMethods>() -> (
     for def in T::py_methods() {
         match *def {
             PyMethodDefType::New(ref def) => {
-                if let class::methods::PyMethodType::PyNewFunc(meth) = def.ml_meth {
+                new = def.get_new_func();
-                    new = Some(meth)
+                debug_assert!(new.is_some());
-                }
             }
             PyMethodDefType::Call(ref def) => {
-                if let class::methods::PyMethodType::PyCFunctionWithKeywords(meth) = def.ml_meth {
+                call = def.get_cfunction_with_keywords();
-                    call = Some(meth)
+                debug_assert!(call.is_some());
-                } else {
-                    panic!("Method type is not supoorted by tp_call slot")
-                }
             }
             PyMethodDefType::Method(ref def)
             | PyMethodDefType::Class(ref def)
@@ -285,19 +281,17 @@ fn py_class_properties<T: PyMethods>() -> Vec<ffi::PyGetSetDef> {
     for def in T::py_methods() {
         match *def {
             PyMethodDefType::Getter(ref getter) => {
-                let name = getter.name.to_string();
+                if !defs.contains_key(getter.name) {
-                if !defs.contains_key(&name) {
+                    let _ = defs.insert(getter.name.to_owned(), ffi::PyGetSetDef_INIT);
-                    let _ = defs.insert(name.clone(), 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(setter.name) {
-                if !defs.contains_key(&name) {
+                    let _ = defs.insert(setter.name.to_owned(), ffi::PyGetSetDef_INIT);
-                    let _ = defs.insert(name.clone(), 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);
             }
             _ => (),

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_SetItemString(tp_dict, key.as_ptr(), val.into_ptr()) };
-            let ret = unsafe { ffi::PyDict_SetItem(tp_dict, key, val.into_ptr()) };
+        if ret < 0 {
-            if ret < 0 {
+            return Err(PyErr::fetch(py));
-                return Err(PyErr::fetch(py));
+        }
-            }
-            Ok(())
-        })?;
     }
     Ok(())
 }