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

rust 1.50: clippy and lint fixes #1422

Merged
merged 1 commit into from
Feb 12, 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
8 changes: 4 additions & 4 deletions examples/rustapi_module/src/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
3 changes: 1 addition & 2 deletions examples/rustapi_module/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
67 changes: 34 additions & 33 deletions src/class/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this module are technically breaking, but it's #[doc(hidden)] internals so I think this refactor is fine.

I did it because it removes some of the type assertions from runtime to compile time, which can only be a good thing for performance 🚀

Copy link
Member Author

@davidhewitt davidhewitt Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided this was fine to merge before the patch release based on comment by dtolnay here: https://www.reddit.com/r/rust/comments/ey0ul0/handling_breaking_api_changes/fgew8d2

/// 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]`
Expand All @@ -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,
}
Expand All @@ -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 {}

Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/ffi/codecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions src/ffi/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/ffi/complexobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions src/ffi/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion src/ffi/descrobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
2 changes: 1 addition & 1 deletion src/ffi/dictobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
4 changes: 2 additions & 2 deletions src/ffi/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion src/ffi/fileobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions src/ffi/genobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/ffi/intrcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
67 changes: 29 additions & 38 deletions src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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>> {
Expand Down Expand Up @@ -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>();
Expand Down Expand Up @@ -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)
Expand Down
Loading