rust 1.50: clippy and lint fixes

examples/rustapi_module/src/datetime.rs

@@ -192,12 +192,12 @@ impl TzClass {
         PyDelta::new(py, 0, 3600, 0, true)
     }
 
-    fn tzname(&self, _py: Python<'_>, _dt: &PyDateTime) -> PyResult<String> {
-        Ok(String::from("+01:00"))
+    fn tzname(&self, _py: Python<'_>, _dt: &PyDateTime) -> String {
+        String::from("+01:00")
     }
 
-    fn dst(&self, _py: Python<'_>, _dt: &PyDateTime) -> PyResult<Option<&PyDelta>> {
-        Ok(None)
+    fn dst(&self, _py: Python<'_>, _dt: &PyDateTime) -> Option<&PyDelta> {
+        None
     }
 }
 

examples/rustapi_module/src/misc.rs

@@ -2,11 +2,10 @@ use pyo3::prelude::*;
 use pyo3::wrap_pyfunction;
 
 #[pyfunction]
-fn issue_219() -> PyResult<()> {
+fn issue_219() {
     // issue 219: acquiring GIL inside #[pyfunction] deadlocks.
     let gil = Python::acquire_gil();
     let _py = gil.python();
-    Ok(())
 }
 
 #[pymodule]

pyo3-macros-backend/src/pymethod.rs

@@ -663,7 +663,7 @@ pub fn impl_py_method_def_call(spec: &FnSpec, wrapper: &TokenStream) -> TokenStr
         pyo3::class::PyMethodDefType::Call({
             #wrapper
 
-            pyo3::class::PyMethodDef::cfunction_with_keywords(
+            pyo3::class::PyMethodDef::call_func(
                 concat!(stringify!(#python_name), "\0"),
                 __wrap,
                 pyo3::ffi::METH_STATIC,

src/class/methods.rs

@@ -10,15 +10,15 @@ use std::os::raw::c_int;
 #[derive(Debug)]
 pub enum PyMethodDefType {
     /// Represents class `__new__` method
-    New(PyMethodDef),
+    New(PyMethodDef<ffi::newfunc>),
     /// Represents class `__call__` method
-    Call(PyMethodDef),
+    Call(PyMethodDef<ffi::PyCFunctionWithKeywords>),
     /// Represents class method
-    Class(PyMethodDef),
+    Class(PyMethodDef<PyMethodType>),
     /// Represents static method
-    Static(PyMethodDef),
+    Static(PyMethodDef<PyMethodType>),
     /// Represents normal method
-    Method(PyMethodDef),
+    Method(PyMethodDef<PyMethodType>),
     /// Represents class attribute, used by `#[attribute]`
     ClassAttribute(PyClassAttributeDef),
     /// Represents getter descriptor, used by `#[getter]`
@@ -31,14 +31,12 @@ pub enum PyMethodDefType {
 pub enum PyMethodType {
     PyCFunction(ffi::PyCFunction),
     PyCFunctionWithKeywords(ffi::PyCFunctionWithKeywords),
-    PyNewFunc(ffi::newfunc),
-    PyInitFunc(ffi::initproc),
 }
 
 #[derive(Clone, Debug)]
-pub struct PyMethodDef {
+pub struct PyMethodDef<MethodT> {
     pub(crate) ml_name: &'static CStr,
-    pub(crate) ml_meth: PyMethodType,
+    pub(crate) ml_meth: MethodT,
     pub(crate) ml_flags: c_int,
     pub(crate) ml_doc: &'static CStr,
 }
@@ -63,7 +61,9 @@ pub struct PySetterDef {
     doc: &'static CStr,
 }
 
-unsafe impl Sync for PyMethodDef {}
+// Safe because ml_meth (the T) cannot be accessed outside of the crate, so only safe-to-sync values
+// are stored in this structure.
+unsafe impl<T> Sync for PyMethodDef<T> {}
 
 unsafe impl Sync for ffi::PyMethodDef {}
 
@@ -82,23 +82,36 @@ fn get_doc(doc: &str) -> &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<ffi::newfunc> {
-        if let PyMethodType::PyNewFunc(new_func) = self.ml_meth {
-            Some(new_func)
-        } else {
-            None
+impl PyMethodDef<ffi::newfunc> {
+    /// Define a `__new__` function.
+    pub fn new_func(name: &'static str, newfunc: ffi::newfunc, doc: &'static str) -> Self {
+        Self {
+            ml_name: get_name(name),
+            ml_meth: newfunc,
+            ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS,
+            ml_doc: get_doc(doc),
         }
     }
+}
 
-    pub(crate) fn get_cfunction_with_keywords(&self) -> Option<ffi::PyCFunctionWithKeywords> {
-        if let PyMethodType::PyCFunctionWithKeywords(func) = self.ml_meth {
-            Some(func)
-        } else {
-            None
+impl PyMethodDef<ffi::PyCFunctionWithKeywords> {
+    /// Define a `__call__` function.
+    pub fn call_func(
+        name: &'static str,
+        callfunc: ffi::PyCFunctionWithKeywords,
+        flags: c_int,
+        doc: &'static str,
+    ) -> Self {
+        Self {
+            ml_name: get_name(name),
+            ml_meth: callfunc,
+            ml_flags: flags | ffi::METH_VARARGS | ffi::METH_KEYWORDS,
+            ml_doc: get_doc(doc),
         }
     }
+}
 
+impl PyMethodDef<PyMethodType> {
     /// Define a function with no `*args` and `**kwargs`.
     pub fn cfunction(name: &'static str, cfunction: ffi::PyCFunction, doc: &'static str) -> Self {
         Self {
@@ -109,16 +122,6 @@ impl PyMethodDef {
         }
     }
 
-    /// Define a `__new__` function.
-    pub fn new_func(name: &'static str, newfunc: ffi::newfunc, doc: &'static str) -> Self {
-        Self {
-            ml_name: get_name(name),
-            ml_meth: PyMethodType::PyNewFunc(newfunc),
-            ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS,
-            ml_doc: get_doc(doc),
-        }
-    }
-
     /// Define a function that can take `*args` and `**kwargs`.
     pub fn cfunction_with_keywords(
         name: &'static str,
@@ -139,8 +142,6 @@ impl PyMethodDef {
         let meth = match self.ml_meth {
             PyMethodType::PyCFunction(meth) => meth,
             PyMethodType::PyCFunctionWithKeywords(meth) => unsafe { std::mem::transmute(meth) },
-            PyMethodType::PyNewFunc(meth) => unsafe { std::mem::transmute(meth) },
-            PyMethodType::PyInitFunc(meth) => unsafe { std::mem::transmute(meth) },
         };
 
         ffi::PyMethodDef {

src/ffi/codecs.rs

@@ -51,6 +51,6 @@ extern "C" {
     pub fn PyCodec_ReplaceErrors(exc: *mut PyObject) -> *mut PyObject;
     pub fn PyCodec_XMLCharRefReplaceErrors(exc: *mut PyObject) -> *mut PyObject;
     pub fn PyCodec_BackslashReplaceErrors(exc: *mut PyObject) -> *mut PyObject;
-// skipped non-limited PyCodec_NameReplaceErrors from Include/codecs.h
-// skipped non-limited Py_hexdigits from Include/codecs.h
+    // skipped non-limited PyCodec_NameReplaceErrors from Include/codecs.h
+    // skipped non-limited Py_hexdigits from Include/codecs.h
 }

src/ffi/compile.rs

@@ -80,8 +80,8 @@ extern "C" {
     #[cfg(Py_3_8)]
     pub fn PyCompile_OpcodeStackEffectWithJump(opcode: c_int, oparg: c_int, jump: c_int) -> c_int;
 
-// skipped non-limited _PyASTOptimizeState
-// skipped non-limited _PyAST_Optimize
+    // skipped non-limited _PyASTOptimizeState
+    // skipped non-limited _PyAST_Optimize
 }
 
 pub const Py_single_input: c_int = 256;

src/ffi/complexobject.rs

@@ -56,6 +56,6 @@ extern "C" {
     pub fn PyComplex_RealAsDouble(op: *mut PyObject) -> c_double;
     #[cfg_attr(PyPy, link_name = "PyPyComplex_ImagAsDouble")]
     pub fn PyComplex_ImagAsDouble(op: *mut PyObject) -> c_double;
-// skipped non-limited PyComplex_AsCComplex
-// skipped non-limited _PyComplex_FormatAdvancedWriter
+    // skipped non-limited PyComplex_AsCComplex
+    // skipped non-limited _PyComplex_FormatAdvancedWriter
 }

src/ffi/context.rs

@@ -7,7 +7,7 @@ extern "C" {
     pub static mut PyContextVar_Type: PyTypeObject;
     // skipped non-limited opaque PyContextVar
     pub static mut PyContextToken_Type: PyTypeObject;
-// skipped non-limited opaque PyContextToken
+    // skipped non-limited opaque PyContextToken
 }
 
 #[inline]
@@ -41,5 +41,5 @@ extern "C" {
     ) -> c_int;
     pub fn PyContextVar_Set(var: *mut PyObject, value: *mut PyObject) -> *mut PyObject;
     pub fn PyContextVar_Reset(var: *mut PyObject, token: *mut PyObject) -> c_int;
-// skipped non-limited _PyContext_NewHamtForTests
+    // skipped non-limited _PyContext_NewHamtForTests
 }

src/ffi/descrobject.rs

@@ -48,7 +48,7 @@ extern "C" {
     pub static mut PyWrapperDescr_Type: PyTypeObject;
     #[cfg_attr(PyPy, link_name = "PyPyDictProxy_Type")]
     pub static mut PyDictProxy_Type: PyTypeObject;
-// skipped non-limited _PyMethodWrapper_Type
+    // skipped non-limited _PyMethodWrapper_Type
 }
 
 extern "C" {

src/ffi/dictobject.rs

@@ -64,7 +64,7 @@ extern "C" {
     ) -> c_int;
     #[cfg_attr(PyPy, link_name = "PyPyDict_DelItemString")]
     pub fn PyDict_DelItemString(dp: *mut PyObject, key: *const c_char) -> c_int;
-// skipped 3.10 / ex-non-limited PyObject_GenericGetDict
+    // skipped 3.10 / ex-non-limited PyObject_GenericGetDict
 }
 
 #[cfg_attr(windows, link(name = "pythonXY"))]

src/ffi/eval.rs

@@ -22,6 +22,6 @@ extern "C" {
         closure: *mut PyObject,
     ) -> *mut PyObject;
 
-// skipped non-limited _PyEval_EvalCodeWithName
-// skipped non-limited _PyEval_CallTracing
+    // skipped non-limited _PyEval_EvalCodeWithName
+    // skipped non-limited _PyEval_CallTracing
 }

src/ffi/fileobject.rs

@@ -29,7 +29,7 @@ extern "C" {
     pub static mut Py_FileSystemDefaultEncoding: *const c_char;
     pub static mut Py_FileSystemDefaultEncodeErrors: *const c_char;
     pub static mut Py_HasFileSystemDefaultEncoding: c_int;
-// skipped Python 3.7 / ex-non-limited Py_UTF8Mode
+    // skipped Python 3.7 / ex-non-limited Py_UTF8Mode
 }
 
 // skipped _PyIsSelectable_fd

src/ffi/genobject.rs

@@ -79,9 +79,9 @@ pub unsafe fn PyCoroWrapper_Check(op: *mut PyObject) -> c_int {
 #[cfg_attr(windows, link(name = "pythonXY"))]
 extern "C" {
     pub static mut PyAsyncGen_Type: PyTypeObject;
-// skipped _PyAsyncGenASend_Type
-// skipped _PyAsyncGenWrappedValue_Type
-// skipped _PyAsyncGenAThrow_Type
+    // skipped _PyAsyncGenASend_Type
+    // skipped _PyAsyncGenWrappedValue_Type
+    // skipped _PyAsyncGenAThrow_Type
 }
 
 // skipped PyAsyncGen_New

src/ffi/intrcheck.rs

@@ -20,6 +20,6 @@ extern "C" {
     #[cfg_attr(PyPy, link_name = "PyPyOS_AfterFork")]
     pub fn PyOS_AfterFork();
 
-// skipped non-limited _PyOS_IsMainThread
-// skipped non-limited Windows _PyOS_SigintEvent
+    // skipped non-limited _PyOS_IsMainThread
+    // skipped non-limited Windows _PyOS_SigintEvent
 }

src/pyclass.rs

@@ -102,16 +102,13 @@ pub trait PyClassAlloc: PyTypeInfo + Sized {
     }
 }
 
-fn tp_dealloc<T: PyClassAlloc>() -> Option<ffi::destructor> {
-    unsafe extern "C" fn dealloc<T>(obj: *mut ffi::PyObject)
-    where
-        T: PyClassAlloc,
-    {
-        let pool = crate::GILPool::new();
-        let py = pool.python();
-        <T as PyClassAlloc>::dealloc(py, (obj as *mut T::Layout) as _)
-    }
-    Some(dealloc::<T>)
+unsafe extern "C" fn tp_dealloc<T>(obj: *mut ffi::PyObject)
+where
+    T: PyClassAlloc,
+{
+    let pool = crate::GILPool::new();
+    let py = pool.python();
+    <T as PyClassAlloc>::dealloc(py, (obj as *mut T::Layout) as _)
 }
 
 pub(crate) unsafe fn tp_free_fallback(ty: *mut ffi::PyTypeObject) -> ffi::freefunc {
@@ -152,11 +149,6 @@ impl TypeSlots {
     fn push(&mut self, slot: c_int, pfunc: *mut c_void) {
         self.0.push(ffi::PyType_Slot { slot, pfunc });
     }
-    pub(crate) fn maybe_push(&mut self, slot: c_int, value: Option<*mut c_void>) {
-        if let Some(pfunc) = value {
-            self.push(slot, pfunc);
-        }
-    }
 }
 
 fn tp_doc<T: PyClass>() -> PyResult<Option<*mut c_void>> {
@@ -189,12 +181,16 @@ where
     let mut slots = TypeSlots::default();
 
     slots.push(ffi::Py_tp_base, T::BaseType::type_object_raw(py) as _);
-    slots.maybe_push(ffi::Py_tp_doc, tp_doc::<T>()?);
-    slots.maybe_push(ffi::Py_tp_dealloc, tp_dealloc::<T>().map(|v| v as _));
+    slots.push(ffi::Py_tp_dealloc, tp_dealloc::<T> as _);
+    if let Some(doc) = tp_doc::<T>()? {
+        slots.push(ffi::Py_tp_doc, doc);
+    }
 
     let (new, call, methods) = py_class_method_defs::<T>();
-    slots.maybe_push(ffi::Py_tp_new, new.map(|v| v as _));
-    slots.maybe_push(ffi::Py_tp_call, call.map(|v| v as _));
+    slots.push(ffi::Py_tp_new, new as _);
+    if let Some(call_meth) = call {
+        slots.push(ffi::Py_tp_call, call_meth as _);
+    }
 
     if cfg!(Py_3_9) {
         let members = py_class_members::<T>();
@@ -315,39 +311,34 @@ pub(crate) fn py_class_attributes<T: PyMethods>() -> impl Iterator<Item = PyClas
     })
 }
 
-fn fallback_new() -> Option<ffi::newfunc> {
-    unsafe extern "C" fn fallback_new(
-        _subtype: *mut ffi::PyTypeObject,
-        _args: *mut ffi::PyObject,
-        _kwds: *mut ffi::PyObject,
-    ) -> *mut ffi::PyObject {
-        crate::callback_body!(py, {
-            Err::<(), _>(crate::exceptions::PyTypeError::new_err(
-                "No constructor defined",
-            ))
-        })
-    }
-    Some(fallback_new)
+unsafe extern "C" fn fallback_new(
+    _subtype: *mut ffi::PyTypeObject,
+    _args: *mut ffi::PyObject,
+    _kwds: *mut ffi::PyObject,
+) -> *mut ffi::PyObject {
+    crate::callback_body!(py, {
+        Err::<(), _>(crate::exceptions::PyTypeError::new_err(
+            "No constructor defined",
+        ))
+    })
 }
 
 fn py_class_method_defs<T: PyMethods>() -> (
-    Option<ffi::newfunc>,
+    ffi::newfunc,
     Option<ffi::PyCFunctionWithKeywords>,
     Vec<ffi::PyMethodDef>,
 ) {
     let mut defs = Vec::new();
     let mut call = None;
-    let mut new = fallback_new();
+    let mut new = fallback_new as ffi::newfunc;
 
     for def in T::py_methods() {
         match def {
             PyMethodDefType::New(def) => {
-                new = def.get_new_func();
-                debug_assert!(new.is_some());
+                new = def.ml_meth;
             }
             PyMethodDefType::Call(def) => {
-                call = def.get_cfunction_with_keywords();
-                debug_assert!(call.is_some());
+                call = Some(def.ml_meth);
             }
             PyMethodDefType::Method(def)
             | PyMethodDefType::Class(def)

tests/test_getter_setter.rs

@@ -11,19 +11,18 @@ struct ClassWithProperties {
 
 #[pymethods]
 impl ClassWithProperties {
-    fn get_num(&self) -> PyResult<i32> {
-        Ok(self.num)
+    fn get_num(&self) -> i32 {
+        self.num
     }
 
     #[getter(DATA)]
     /// a getter for data
-    fn get_data(&self) -> PyResult<i32> {
-        Ok(self.num)
+    fn get_data(&self) -> i32 {
+        self.num
     }
     #[setter(DATA)]
-    fn set_data(&mut self, value: i32) -> PyResult<()> {
+    fn set_data(&mut self, value: i32) {
         self.num = value;
-        Ok(())
     }
 
     #[getter]
@@ -79,8 +78,8 @@ struct GetterSetter {
 
 #[pymethods]
 impl GetterSetter {
-    fn get_num2(&self) -> PyResult<i32> {
-        Ok(self.num)
+    fn get_num2(&self) -> i32 {
+        self.num
     }
 }
 

tests/test_methods.rs

@@ -13,8 +13,8 @@ struct InstanceMethod {
 #[pymethods]
 impl InstanceMethod {
     /// Test method
-    fn method(&self) -> PyResult<i32> {
-        Ok(self.member)
+    fn method(&self) -> i32 {
+        self.member
     }
 
     // Checks that &Self works
@@ -30,7 +30,7 @@ fn instance_method() {
 
     let obj = PyCell::new(py, InstanceMethod { member: 42 }).unwrap();
     let obj_ref = obj.borrow();
-    assert_eq!(obj_ref.method().unwrap(), 42);
+    assert_eq!(obj_ref.method(), 42);
     py_assert!(py, obj, "obj.method() == 42");
     py_assert!(py, obj, "obj.add_other(obj) == 84");
     py_assert!(py, obj, "obj.method.__doc__ == 'Test method'");
@@ -43,8 +43,8 @@ struct InstanceMethodWithArgs {
 
 #[pymethods]
 impl InstanceMethodWithArgs {
-    fn method(&self, multiplier: i32) -> PyResult<i32> {
-        Ok(self.member * multiplier)
+    fn method(&self, multiplier: i32) -> i32 {
+        self.member * multiplier
     }
 }
 
@@ -56,7 +56,7 @@ fn instance_method_with_args() {
 
     let obj = PyCell::new(py, InstanceMethodWithArgs { member: 7 }).unwrap();
     let obj_ref = obj.borrow();
-    assert_eq!(obj_ref.method(6).unwrap(), 42);
+    assert_eq!(obj_ref.method(6), 42);
     let d = [("obj", obj)].into_py_dict(py);
     py.run("assert obj.method(3) == 21", None, Some(d)).unwrap();
     py.run("assert obj.method(multiplier=6) == 42", None, Some(d))
@@ -134,8 +134,8 @@ impl StaticMethod {
 
     #[staticmethod]
     /// Test static method.
-    fn method(_py: Python) -> PyResult<&'static str> {
-        Ok("StaticMethod.method()!")
+    fn method(_py: Python) -> &'static str {
+        "StaticMethod.method()!"
     }
 }
 
@@ -144,7 +144,7 @@ fn static_method() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    assert_eq!(StaticMethod::method(py).unwrap(), "StaticMethod.method()!");
+    assert_eq!(StaticMethod::method(py), "StaticMethod.method()!");
 
     let d = [("C", py.get_type::<StaticMethod>())].into_py_dict(py);
     let run = |code| {
@@ -164,8 +164,8 @@ struct StaticMethodWithArgs {}
 #[pymethods]
 impl StaticMethodWithArgs {
     #[staticmethod]
-    fn method(_py: Python, input: i32) -> PyResult<String> {
-        Ok(format!("0x{:x}", input))
+    fn method(_py: Python, input: i32) -> String {
+        format!("0x{:x}", input)
     }
 }
 
@@ -174,7 +174,7 @@ fn static_method_with_args() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    assert_eq!(StaticMethodWithArgs::method(py, 1234).unwrap(), "0x4d2");
+    assert_eq!(StaticMethodWithArgs::method(py, 1234), "0x4d2");
 
     let d = [("C", py.get_type::<StaticMethodWithArgs>())].into_py_dict(py);
     py.run("assert C.method(1337) == '0x539'", None, Some(d))
@@ -187,41 +187,36 @@ struct MethArgs {}
 #[pymethods]
 impl MethArgs {
     #[args(test)]
-    fn get_optional(&self, test: Option<i32>) -> PyResult<i32> {
-        Ok(test.unwrap_or(10))
+    fn get_optional(&self, test: Option<i32>) -> i32 {
+        test.unwrap_or(10)
     }
-    fn get_optional2(&self, test: Option<i32>) -> PyResult<Option<i32>> {
-        Ok(test)
+    fn get_optional2(&self, test: Option<i32>) -> Option<i32> {
+        test
     }
     #[args(test = "None")]
-    fn get_optional3(&self, test: Option<i32>) -> PyResult<Option<i32>> {
-        Ok(test)
+    fn get_optional3(&self, test: Option<i32>) -> Option<i32> {
+        test
     }
     fn get_optional_positional(
         &self,
         _t1: Option<i32>,
         t2: Option<i32>,
         _t3: Option<i32>,
-    ) -> PyResult<Option<i32>> {
-