From 0e142f05dd06344be14059246f5f2e43f734e931 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 18 Jun 2024 19:09:36 +0100 Subject: [PATCH] add `c_str!` macro to create `&'static CStr` (#4255) * add `c_str!` macro to create `&'static CStr` * newsfragment, just export as `pyo3::ffi::c_str` * fix doc link * fix doc * further `c_str!` based cleanups * [review]: mejrs Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com> * rustfmt * build fixes * clippy * allow lint on MSRV * fix GraalPy import --------- Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com> --- examples/sequential/src/id.rs | 21 ++++--- examples/sequential/src/module.rs | 12 ++-- examples/sequential/tests/test.rs | 20 +++---- examples/string-sum/src/lib.rs | 18 ++---- guide/src/class.md | 10 ++-- newsfragments/4255.added.md | 1 + newsfragments/4255.changed.md | 1 + pyo3-ffi/README.md | 26 +++------ pyo3-ffi/src/abstract_.rs | 2 +- pyo3-ffi/src/datetime.rs | 40 ++++++------- pyo3-ffi/src/lib.rs | 73 +++++++++++++++++------ pyo3-ffi/src/pyerrors.rs | 2 +- pyo3-macros-backend/src/konst.rs | 7 ++- pyo3-macros-backend/src/method.rs | 14 +++-- pyo3-macros-backend/src/module.rs | 7 ++- pyo3-macros-backend/src/pyclass.rs | 18 +++--- pyo3-macros-backend/src/pyfunction.rs | 2 +- pyo3-macros-backend/src/pyimpl.rs | 2 +- pyo3-macros-backend/src/pymethod.rs | 32 +++++------ pyo3-macros-backend/src/utils.rs | 20 ++++--- src/buffer.rs | 83 +++++++++++++-------------- src/conversions/num_rational.rs | 21 ++----- src/err/err_state.rs | 4 +- src/exceptions.rs | 4 +- src/impl_/pyclass.rs | 7 +-- src/impl_/pyclass/lazy_type_object.rs | 9 +-- src/impl_/pymethods.rs | 82 +++++++++----------------- src/impl_/pymodule.rs | 23 ++++---- src/internal_tricks.rs | 36 +----------- src/macros.rs | 2 +- src/marker.rs | 5 +- src/pyclass/create_type_object.rs | 75 ++++++++++-------------- src/types/function.rs | 61 ++++++++++---------- src/types/string.rs | 10 ++-- tests/test_buffer.rs | 5 +- tests/test_pyfunction.rs | 18 ++++-- 36 files changed, 357 insertions(+), 416 deletions(-) create mode 100644 newsfragments/4255.added.md create mode 100644 newsfragments/4255.changed.md diff --git a/examples/sequential/src/id.rs b/examples/sequential/src/id.rs index d80e84b4eab..fa72bb091c7 100644 --- a/examples/sequential/src/id.rs +++ b/examples/sequential/src/id.rs @@ -1,5 +1,6 @@ use core::sync::atomic::{AtomicU64, Ordering}; use core::{mem, ptr}; +use std::ffi::CString; use std::os::raw::{c_char, c_int, c_uint, c_ulonglong, c_void}; use pyo3_ffi::*; @@ -27,10 +28,10 @@ unsafe extern "C" fn id_new( kwds: *mut PyObject, ) -> *mut PyObject { if PyTuple_Size(args) != 0 || !kwds.is_null() { - PyErr_SetString( - PyExc_TypeError, - "Id() takes no arguments\0".as_ptr().cast::(), - ); + // We use pyo3-ffi's `c_str!` macro to create null-terminated literals because + // Rust's string literals are not null-terminated + // On Rust 1.77 or newer you can use `c"text"` instead. + PyErr_SetString(PyExc_TypeError, c_str!("Id() takes no arguments").as_ptr()); return ptr::null_mut(); } @@ -81,8 +82,12 @@ unsafe extern "C" fn id_richcompare( pyo3_ffi::Py_GT => slf > other, pyo3_ffi::Py_GE => slf >= other, unrecognized => { - let msg = format!("unrecognized richcompare opcode {}\0", unrecognized); - PyErr_SetString(PyExc_SystemError, msg.as_ptr().cast::()); + let msg = CString::new(&*format!( + "unrecognized richcompare opcode {}", + unrecognized + )) + .unwrap(); + PyErr_SetString(PyExc_SystemError, msg.as_ptr()); return ptr::null_mut(); } }; @@ -101,7 +106,7 @@ static mut SLOTS: &[PyType_Slot] = &[ }, PyType_Slot { slot: Py_tp_doc, - pfunc: "An id that is increased every time an instance is created\0".as_ptr() + pfunc: c_str!("An id that is increased every time an instance is created").as_ptr() as *mut c_void, }, PyType_Slot { @@ -123,7 +128,7 @@ static mut SLOTS: &[PyType_Slot] = &[ ]; pub static mut ID_SPEC: PyType_Spec = PyType_Spec { - name: "sequential.Id\0".as_ptr().cast::(), + name: c_str!("sequential.Id").as_ptr(), basicsize: mem::size_of::() as c_int, itemsize: 0, flags: (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE) as c_uint, diff --git a/examples/sequential/src/module.rs b/examples/sequential/src/module.rs index 5552baf3368..5e71f07a865 100644 --- a/examples/sequential/src/module.rs +++ b/examples/sequential/src/module.rs @@ -1,13 +1,11 @@ use core::{mem, ptr}; use pyo3_ffi::*; -use std::os::raw::{c_char, c_int, c_void}; +use std::os::raw::{c_int, c_void}; pub static mut MODULE_DEF: PyModuleDef = PyModuleDef { m_base: PyModuleDef_HEAD_INIT, - m_name: "sequential\0".as_ptr().cast::(), - m_doc: "A library for generating sequential ids, written in Rust.\0" - .as_ptr() - .cast::(), + m_name: c_str!("sequential").as_ptr(), + m_doc: c_str!("A library for generating sequential ids, written in Rust.").as_ptr(), m_size: mem::size_of::() as Py_ssize_t, m_methods: std::ptr::null_mut(), m_slots: unsafe { SEQUENTIAL_SLOTS as *const [PyModuleDef_Slot] as *mut PyModuleDef_Slot }, @@ -42,13 +40,13 @@ unsafe extern "C" fn sequential_exec(module: *mut PyObject) -> c_int { if id_type.is_null() { PyErr_SetString( PyExc_SystemError, - "cannot locate type object\0".as_ptr().cast::(), + c_str!("cannot locate type object").as_ptr(), ); return -1; } (*state).id_type = id_type.cast::(); - PyModule_AddObjectRef(module, "Id\0".as_ptr().cast::(), id_type) + PyModule_AddObjectRef(module, c_str!("Id").as_ptr(), id_type) } unsafe extern "C" fn sequential_traverse( diff --git a/examples/sequential/tests/test.rs b/examples/sequential/tests/test.rs index 6076edd4974..f2a08433cea 100644 --- a/examples/sequential/tests/test.rs +++ b/examples/sequential/tests/test.rs @@ -5,11 +5,13 @@ use std::thread; use pyo3_ffi::*; use sequential::PyInit_sequential; -static COMMAND: &'static str = " +static COMMAND: &'static str = c_str!( + " from sequential import Id s = sum(int(Id()) for _ in range(12)) -\0"; +" +); // Newtype to be able to pass it to another thread. struct State(*mut PyThreadState); @@ -19,10 +21,7 @@ unsafe impl Send for State {} #[test] fn lets_go_fast() -> Result<(), String> { unsafe { - let ret = PyImport_AppendInittab( - "sequential\0".as_ptr().cast::(), - Some(PyInit_sequential), - ); + let ret = PyImport_AppendInittab(c_str!("sequential").as_ptr(), Some(PyInit_sequential)); if ret == -1 { return Err("could not add module to inittab".into()); } @@ -122,11 +121,8 @@ unsafe fn fetch() -> String { fn run_code() -> Result { unsafe { - let code_obj = Py_CompileString( - COMMAND.as_ptr().cast::(), - "program\0".as_ptr().cast::(), - Py_file_input, - ); + let code_obj = + Py_CompileString(COMMAND.as_ptr(), c_str!("program").as_ptr(), Py_file_input); if code_obj.is_null() { return Err(fetch()); } @@ -138,7 +134,7 @@ fn run_code() -> Result { } else { Py_DECREF(res_ptr); } - let sum = PyDict_GetItemString(globals, "s\0".as_ptr().cast::()); /* borrowed reference */ + let sum = PyDict_GetItemString(globals, c_str!("s").as_ptr()); /* borrowed reference */ if sum.is_null() { Py_DECREF(globals); return Err("globals did not have `s`".into()); diff --git a/examples/string-sum/src/lib.rs b/examples/string-sum/src/lib.rs index 91072418038..ce71ab38f87 100644 --- a/examples/string-sum/src/lib.rs +++ b/examples/string-sum/src/lib.rs @@ -5,10 +5,8 @@ use pyo3_ffi::*; static mut MODULE_DEF: PyModuleDef = PyModuleDef { m_base: PyModuleDef_HEAD_INIT, - m_name: "string_sum\0".as_ptr().cast::(), - m_doc: "A Python module written in Rust.\0" - .as_ptr() - .cast::(), + m_name: c_str!("string_sum").as_ptr(), + m_doc: c_str!("A Python module written in Rust.").as_ptr(), m_size: 0, m_methods: unsafe { METHODS as *const [PyMethodDef] as *mut PyMethodDef }, m_slots: std::ptr::null_mut(), @@ -19,14 +17,12 @@ static mut MODULE_DEF: PyModuleDef = PyModuleDef { static mut METHODS: &[PyMethodDef] = &[ PyMethodDef { - ml_name: "sum_as_string\0".as_ptr().cast::(), + ml_name: c_str!("sum_as_string").as_ptr(), ml_meth: PyMethodDefPointer { _PyCFunctionFast: sum_as_string, }, ml_flags: METH_FASTCALL, - ml_doc: "returns the sum of two integers as a string\0" - .as_ptr() - .cast::(), + ml_doc: c_str!("returns the sum of two integers as a string").as_ptr(), }, // A zeroed PyMethodDef to mark the end of the array. PyMethodDef::zeroed(), @@ -93,9 +89,7 @@ pub unsafe extern "C" fn sum_as_string( if nargs != 2 { PyErr_SetString( PyExc_TypeError, - "sum_as_string expected 2 positional arguments\0" - .as_ptr() - .cast::(), + c_str!("sum_as_string expected 2 positional arguments").as_ptr(), ); return std::ptr::null_mut(); } @@ -119,7 +113,7 @@ pub unsafe extern "C" fn sum_as_string( None => { PyErr_SetString( PyExc_OverflowError, - "arguments too large to add\0".as_ptr().cast::(), + c_str!("arguments too large to add").as_ptr(), ); std::ptr::null_mut() } diff --git a/guide/src/class.md b/guide/src/class.md index ab0c82fc88b..94f3f333581 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -330,8 +330,8 @@ or [`PyRefMut`] instead of `&mut self`. Then you can access a parent class by `self_.as_super()` as `&PyRef`, or by `self_.into_super()` as `PyRef` (and similar for the `PyRefMut` case). For convenience, `self_.as_ref()` can also be used to get `&Self::BaseClass` -directly; however, this approach does not let you access base clases higher in the -inheritance hierarchy, for which you would need to chain multiple `as_super` or +directly; however, this approach does not let you access base clases higher in the +inheritance hierarchy, for which you would need to chain multiple `as_super` or `into_super` calls. ```rust @@ -400,7 +400,7 @@ impl SubSubClass { let val2 = self_.as_super().val2; (val1, val2, self_.val3) } - + fn double_values(mut self_: PyRefMut<'_, Self>) { self_.as_super().as_super().val1 *= 2; self_.as_super().val2 *= 2; @@ -1187,7 +1187,7 @@ Python::with_gil(|py| { }) ``` -Ordering of enum variants is optionally added using `#[pyo3(ord)]`. +Ordering of enum variants is optionally added using `#[pyo3(ord)]`. *Note: Implementation of the `PartialOrd` trait is required when passing the `ord` argument. If not implemented, a compile time error is raised.* ```rust @@ -1443,7 +1443,7 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { static DOC: pyo3::sync::GILOnceCell<::std::borrow::Cow<'static, ::std::ffi::CStr>> = pyo3::sync::GILOnceCell::new(); DOC.get_or_try_init(py, || { let collector = PyClassImplCollector::::new(); - build_pyclass_doc(::NAME, "\0", collector.new_text_signature()) + build_pyclass_doc(::NAME, pyo3::ffi::c_str!(""), collector.new_text_signature()) }).map(::std::ops::Deref::deref) } } diff --git a/newsfragments/4255.added.md b/newsfragments/4255.added.md new file mode 100644 index 00000000000..c70c5da279d --- /dev/null +++ b/newsfragments/4255.added.md @@ -0,0 +1 @@ +Add `pyo3_ffi::c_str` macro to create `&'static CStr` on Rust versions which don't have 1.77's `c""` literals. diff --git a/newsfragments/4255.changed.md b/newsfragments/4255.changed.md new file mode 100644 index 00000000000..185bd5cc39d --- /dev/null +++ b/newsfragments/4255.changed.md @@ -0,0 +1 @@ +`PyCFunction::new`, `PyCFunction::new_with_keywords` and `PyCFunction::new_closure` now take `&'static CStr` name and doc arguments (previously was `&'static str`). diff --git a/pyo3-ffi/README.md b/pyo3-ffi/README.md index 283a7072357..200c78cec14 100644 --- a/pyo3-ffi/README.md +++ b/pyo3-ffi/README.md @@ -51,10 +51,8 @@ use pyo3_ffi::*; static mut MODULE_DEF: PyModuleDef = PyModuleDef { m_base: PyModuleDef_HEAD_INIT, - m_name: "string_sum\0".as_ptr().cast::(), - m_doc: "A Python module written in Rust.\0" - .as_ptr() - .cast::(), + m_name: c_str!("string_sum").as_ptr(), + m_doc: c_str!("A Python module written in Rust.").as_ptr(), m_size: 0, m_methods: unsafe { METHODS.as_mut_ptr().cast() }, m_slots: std::ptr::null_mut(), @@ -65,14 +63,12 @@ static mut MODULE_DEF: PyModuleDef = PyModuleDef { static mut METHODS: [PyMethodDef; 2] = [ PyMethodDef { - ml_name: "sum_as_string\0".as_ptr().cast::(), + ml_name: c_str!("sum_as_string").as_ptr(), ml_meth: PyMethodDefPointer { _PyCFunctionFast: sum_as_string, }, ml_flags: METH_FASTCALL, - ml_doc: "returns the sum of two integers as a string\0" - .as_ptr() - .cast::(), + ml_doc: c_str!("returns the sum of two integers as a string").as_ptr(), }, // A zeroed PyMethodDef to mark the end of the array. PyMethodDef::zeroed() @@ -93,9 +89,7 @@ pub unsafe extern "C" fn sum_as_string( if nargs != 2 { PyErr_SetString( PyExc_TypeError, - "sum_as_string() expected 2 positional arguments\0" - .as_ptr() - .cast::(), + c_str!("sum_as_string() expected 2 positional arguments").as_ptr(), ); return std::ptr::null_mut(); } @@ -104,9 +98,7 @@ pub unsafe extern "C" fn sum_as_string( if PyLong_Check(arg1) == 0 { PyErr_SetString( PyExc_TypeError, - "sum_as_string() expected an int for positional argument 1\0" - .as_ptr() - .cast::(), + c_str!("sum_as_string() expected an int for positional argument 1").as_ptr(), ); return std::ptr::null_mut(); } @@ -120,9 +112,7 @@ pub unsafe extern "C" fn sum_as_string( if PyLong_Check(arg2) == 0 { PyErr_SetString( PyExc_TypeError, - "sum_as_string() expected an int for positional argument 2\0" - .as_ptr() - .cast::(), + c_str!("sum_as_string() expected an int for positional argument 2").as_ptr(), ); return std::ptr::null_mut(); } @@ -140,7 +130,7 @@ pub unsafe extern "C" fn sum_as_string( None => { PyErr_SetString( PyExc_OverflowError, - "arguments too large to add\0".as_ptr().cast::(), + c_str!("arguments too large to add").as_ptr(), ); std::ptr::null_mut() } diff --git a/pyo3-ffi/src/abstract_.rs b/pyo3-ffi/src/abstract_.rs index 1e0020462fb..175f9af734f 100644 --- a/pyo3-ffi/src/abstract_.rs +++ b/pyo3-ffi/src/abstract_.rs @@ -114,7 +114,7 @@ extern "C" { #[cfg(not(any(Py_3_8, PyPy)))] #[inline] pub unsafe fn PyIter_Check(o: *mut PyObject) -> c_int { - crate::PyObject_HasAttrString(crate::Py_TYPE(o).cast(), "__next__\0".as_ptr().cast()) + crate::PyObject_HasAttrString(crate::Py_TYPE(o).cast(), c_str!("__next__").as_ptr()) } extern "C" { diff --git a/pyo3-ffi/src/datetime.rs b/pyo3-ffi/src/datetime.rs index b985085fba3..5da2956c5e9 100644 --- a/pyo3-ffi/src/datetime.rs +++ b/pyo3-ffi/src/datetime.rs @@ -357,8 +357,8 @@ pub unsafe fn PyDateTime_DELTA_GET_MICROSECONDS(o: *mut PyObject) -> c_int { // but copying them seems suboptimal #[inline] #[cfg(GraalPy)] -pub unsafe fn _get_attr(obj: *mut PyObject, field: &str) -> c_int { - let result = PyObject_GetAttrString(obj, field.as_ptr().cast()); +pub unsafe fn _get_attr(obj: *mut PyObject, field: &std::ffi::CStr) -> c_int { + let result = PyObject_GetAttrString(obj, field.as_ptr()); Py_DecRef(result); // the original macros are borrowing if PyLong_Check(result) == 1 { PyLong_AsLong(result) as c_int @@ -370,55 +370,55 @@ pub unsafe fn _get_attr(obj: *mut PyObject, field: &str) -> c_int { #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_GET_YEAR(o: *mut PyObject) -> c_int { - _get_attr(o, "year\0") + _get_attr(o, c_str!("year")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_GET_MONTH(o: *mut PyObject) -> c_int { - _get_attr(o, "month\0") + _get_attr(o, c_str!("month")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_GET_DAY(o: *mut PyObject) -> c_int { - _get_attr(o, "day\0") + _get_attr(o, c_str!("day")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DATE_GET_HOUR(o: *mut PyObject) -> c_int { - _get_attr(o, "hour\0") + _get_attr(o, c_str!("hour")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DATE_GET_MINUTE(o: *mut PyObject) -> c_int { - _get_attr(o, "minute\0") + _get_attr(o, c_str!("minute")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DATE_GET_SECOND(o: *mut PyObject) -> c_int { - _get_attr(o, "second\0") + _get_attr(o, c_str!("second")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DATE_GET_MICROSECOND(o: *mut PyObject) -> c_int { - _get_attr(o, "microsecond\0") + _get_attr(o, c_str!("microsecond")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DATE_GET_FOLD(o: *mut PyObject) -> c_int { - _get_attr(o, "fold\0") + _get_attr(o, c_str!("fold")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DATE_GET_TZINFO(o: *mut PyObject) -> *mut PyObject { - let res = PyObject_GetAttrString(o, "tzinfo\0".as_ptr().cast()); + let res = PyObject_GetAttrString(o, c_str!("tzinfo").as_ptr().cast()); Py_DecRef(res); // the original macros are borrowing res } @@ -426,37 +426,37 @@ pub unsafe fn PyDateTime_DATE_GET_TZINFO(o: *mut PyObject) -> *mut PyObject { #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_TIME_GET_HOUR(o: *mut PyObject) -> c_int { - _get_attr(o, "hour\0") + _get_attr(o, c_str!("hour")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_TIME_GET_MINUTE(o: *mut PyObject) -> c_int { - _get_attr(o, "minute\0") + _get_attr(o, c_str!("minute")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_TIME_GET_SECOND(o: *mut PyObject) -> c_int { - _get_attr(o, "second\0") + _get_attr(o, c_str!("second")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_TIME_GET_MICROSECOND(o: *mut PyObject) -> c_int { - _get_attr(o, "microsecond\0") + _get_attr(o, c_str!("microsecond")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_TIME_GET_FOLD(o: *mut PyObject) -> c_int { - _get_attr(o, "fold\0") + _get_attr(o, c_str!("fold")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_TIME_GET_TZINFO(o: *mut PyObject) -> *mut PyObject { - let res = PyObject_GetAttrString(o, "tzinfo\0".as_ptr().cast()); + let res = PyObject_GetAttrString(o, c_str!("tzinfo").as_ptr().cast()); Py_DecRef(res); // the original macros are borrowing res } @@ -464,19 +464,19 @@ pub unsafe fn PyDateTime_TIME_GET_TZINFO(o: *mut PyObject) -> *mut PyObject { #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DELTA_GET_DAYS(o: *mut PyObject) -> c_int { - _get_attr(o, "days\0") + _get_attr(o, c_str!("days")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DELTA_GET_SECONDS(o: *mut PyObject) -> c_int { - _get_attr(o, "seconds\0") + _get_attr(o, c_str!("seconds")) } #[inline] #[cfg(GraalPy)] pub unsafe fn PyDateTime_DELTA_GET_MICROSECONDS(o: *mut PyObject) -> c_int { - _get_attr(o, "microseconds\0") + _get_attr(o, c_str!("microseconds")) } #[cfg(PyPy)] diff --git a/pyo3-ffi/src/lib.rs b/pyo3-ffi/src/lib.rs index 877d42dce33..c3f5225e87d 100644 --- a/pyo3-ffi/src/lib.rs +++ b/pyo3-ffi/src/lib.rs @@ -88,10 +88,8 @@ //! //! static mut MODULE_DEF: PyModuleDef = PyModuleDef { //! m_base: PyModuleDef_HEAD_INIT, -//! m_name: "string_sum\0".as_ptr().cast::(), -//! m_doc: "A Python module written in Rust.\0" -//! .as_ptr() -//! .cast::(), +//! m_name: c_str!("string_sum").as_ptr(), +//! m_doc: c_str!("A Python module written in Rust.").as_ptr(), //! m_size: 0, //! m_methods: unsafe { METHODS.as_mut_ptr().cast() }, //! m_slots: std::ptr::null_mut(), @@ -102,14 +100,12 @@ //! //! static mut METHODS: [PyMethodDef; 2] = [ //! PyMethodDef { -//! ml_name: "sum_as_string\0".as_ptr().cast::(), +//! ml_name: c_str!("sum_as_string").as_ptr(), //! ml_meth: PyMethodDefPointer { //! _PyCFunctionFast: sum_as_string, //! }, //! ml_flags: METH_FASTCALL, -//! ml_doc: "returns the sum of two integers as a string\0" -//! .as_ptr() -//! .cast::(), +//! ml_doc: c_str!("returns the sum of two integers as a string").as_ptr(), //! }, //! // A zeroed PyMethodDef to mark the end of the array. //! PyMethodDef::zeroed() @@ -130,9 +126,7 @@ //! if nargs != 2 { //! PyErr_SetString( //! PyExc_TypeError, -//! "sum_as_string() expected 2 positional arguments\0" -//! .as_ptr() -//! .cast::(), +//! c_str!("sum_as_string() expected 2 positional arguments").as_ptr(), //! ); //! return std::ptr::null_mut(); //! } @@ -141,9 +135,7 @@ //! if PyLong_Check(arg1) == 0 { //! PyErr_SetString( //! PyExc_TypeError, -//! "sum_as_string() expected an int for positional argument 1\0" -//! .as_ptr() -//! .cast::(), +//! c_str!("sum_as_string() expected an int for positional argument 1").as_ptr(), //! ); //! return std::ptr::null_mut(); //! } @@ -157,9 +149,7 @@ //! if PyLong_Check(arg2) == 0 { //! PyErr_SetString( //! PyExc_TypeError, -//! "sum_as_string() expected an int for positional argument 2\0" -//! .as_ptr() -//! .cast::(), +//! c_str!("sum_as_string() expected an int for positional argument 2").as_ptr(), //! ); //! return std::ptr::null_mut(); //! } @@ -177,7 +167,7 @@ //! None => { //! PyErr_SetString( //! PyExc_OverflowError, -//! "arguments too large to add\0".as_ptr().cast::(), +//! c_str!("arguments too large to add").as_ptr(), //! ); //! std::ptr::null_mut() //! } @@ -256,6 +246,53 @@ macro_rules! opaque_struct { }; } +/// This is a helper macro to create a `&'static CStr`. +/// +/// It can be used on all Rust versions supported by PyO3, unlike c"" literals which +/// were stabilised in Rust 1.77. +/// +/// Due to the nature of PyO3 making heavy use of C FFI interop with Python, it is +/// common for PyO3 to use CStr. +/// +/// Examples: +/// +/// ```rust +/// use std::ffi::CStr; +/// +/// const HELLO: &CStr = pyo3_ffi::c_str!("hello"); +/// static WORLD: &CStr = pyo3_ffi::c_str!("world"); +/// ``` +#[macro_export] +macro_rules! c_str { + ($s:expr) => {{ + const _: () = { + assert!( + $crate::str_contains_no_nul($s), + "string contains null bytes" + ); + }; + // SAFETY: the string is checked to not contain null bytes + #[allow(unsafe_op_in_unsafe_fn, unused_unsafe)] // MSRV 1.63 needs these allows + unsafe { + ::std::ffi::CStr::from_bytes_with_nul_unchecked(concat!($s, "\0").as_bytes()) + } + }}; +} + +#[doc(hidden)] +pub const fn str_contains_no_nul(s: &str) -> bool { + let bytes = s.as_bytes(); + let len = s.len(); + let mut i = 0; + while i < len { + if bytes[i] == 0 { + return false; + } + i += 1; + } + true +} + pub use self::abstract_::*; pub use self::bltinmodule::*; pub use self::boolobject::*; diff --git a/pyo3-ffi/src/pyerrors.rs b/pyo3-ffi/src/pyerrors.rs index 9da00ea390e..6c9313c4ab0 100644 --- a/pyo3-ffi/src/pyerrors.rs +++ b/pyo3-ffi/src/pyerrors.rs @@ -101,7 +101,7 @@ pub unsafe fn PyUnicodeDecodeError_Create( ) -> *mut PyObject { crate::_PyObject_CallFunction_SizeT( PyExc_UnicodeDecodeError, - b"sy#nns\0".as_ptr().cast::(), + c_str!("sy#nns").as_ptr(), encoding, object, length, diff --git a/pyo3-macros-backend/src/konst.rs b/pyo3-macros-backend/src/konst.rs index 9a41a2b7178..e7d8d554cae 100644 --- a/pyo3-macros-backend/src/konst.rs +++ b/pyo3-macros-backend/src/konst.rs @@ -29,9 +29,10 @@ impl ConstSpec<'_> { } /// Null-terminated Python name - pub fn null_terminated_python_name(&self) -> TokenStream { - let name = format!("{}\0", self.python_name()); - quote!({#name}) + pub fn null_terminated_python_name(&self, ctx: &Ctx) -> TokenStream { + let Ctx { pyo3_path } = ctx; + let name = self.python_name().to_string(); + quote!(#pyo3_path::ffi::c_str!(#name)) } } diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index c0e38bf8416..745426371c3 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -472,8 +472,12 @@ impl<'a> FnSpec<'a> { }) } - pub fn null_terminated_python_name(&self) -> syn::LitStr { - syn::LitStr::new(&format!("{}\0", self.python_name), self.python_name.span()) + pub fn null_terminated_python_name(&self, ctx: &Ctx) -> TokenStream { + let Ctx { pyo3_path } = ctx; + let span = self.python_name.span(); + let pyo3_path = pyo3_path.to_tokens_spanned(span); + let name = self.python_name.to_string(); + quote_spanned!(self.python_name.span() => #pyo3_path::ffi::c_str!(#name)) } fn parse_fn_type( @@ -830,7 +834,7 @@ impl<'a> FnSpec<'a> { /// calling convention. pub fn get_methoddef(&self, wrapper: impl ToTokens, doc: &PythonDoc, ctx: &Ctx) -> TokenStream { let Ctx { pyo3_path } = ctx; - let python_name = self.null_terminated_python_name(); + let python_name = self.null_terminated_python_name(ctx); match self.convention { CallingConvention::Noargs => quote! { #pyo3_path::impl_::pymethods::PyMethodDef::noargs( @@ -903,11 +907,11 @@ impl<'a> FnSpec<'a> { } /// Forwards to [utils::get_doc] with the text signature of this spec. - pub fn get_doc(&self, attrs: &[syn::Attribute]) -> PythonDoc { + pub fn get_doc(&self, attrs: &[syn::Attribute], ctx: &Ctx) -> PythonDoc { let text_signature = self .text_signature_call_signature() .map(|sig| format!("{}{}", self.python_name, sig)); - utils::get_doc(attrs, text_signature) + utils::get_doc(attrs, text_signature, ctx) } /// Creates the parenthesised arguments list for `__text_signature__` snippet based on this spec's signature diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index c1e46276544..0383046e0c4 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -92,7 +92,7 @@ pub fn pymodule_module_impl(mut module: syn::ItemMod) -> Result { let options = PyModuleOptions::from_attrs(attrs)?; let ctx = &Ctx::new(&options.krate); let Ctx { pyo3_path } = ctx; - let doc = get_doc(attrs, None); + let doc = get_doc(attrs, None, ctx); let name = options.name.unwrap_or_else(|| ident.unraw()); let full_name = if let Some(module) = &options.module { format!("{}.{}", module.value.value(), name) @@ -332,7 +332,7 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result let ident = &function.sig.ident; let name = options.name.unwrap_or_else(|| ident.unraw()); let vis = &function.vis; - let doc = get_doc(&function.attrs, None); + let doc = get_doc(&function.attrs, None, ctx); let initialization = module_initialization(&name, ctx); @@ -402,10 +402,11 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result fn module_initialization(name: &syn::Ident, ctx: &Ctx) -> TokenStream { let Ctx { pyo3_path } = ctx; let pyinit_symbol = format!("PyInit_{}", name); + let name = name.to_string(); quote! { #[doc(hidden)] - pub const __PYO3_NAME: &'static str = concat!(stringify!(#name), "\0"); + pub const __PYO3_NAME: &'static ::std::ffi::CStr = #pyo3_path::ffi::c_str!(#name); pub(super) struct MakeDef; #[doc(hidden)] diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 9484b3dbf26..3e40977e4af 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use proc_macro2::{Ident, Span, TokenStream}; -use quote::{format_ident, quote, quote_spanned}; +use quote::{format_ident, quote, quote_spanned, ToTokens}; use syn::ext::IdentExt; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; @@ -21,9 +21,10 @@ use crate::pymethod::{ impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType, SlotDef, __GETITEM__, __HASH__, __INT__, __LEN__, __REPR__, __RICHCMP__, }; +use crate::pyversions; use crate::utils::{self, apply_renaming_rule, PythonDoc}; use crate::utils::{is_abi3, Ctx}; -use crate::{pyversions, PyFunctionOptions}; +use crate::PyFunctionOptions; /// If the class is derived from a Rust `struct` or `enum`. #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -225,9 +226,9 @@ pub fn build_py_class( methods_type: PyClassMethodsType, ) -> syn::Result { args.options.take_pyo3_options(&mut class.attrs)?; - let doc = utils::get_doc(&class.attrs, None); let ctx = &Ctx::new(&args.options.krate); + let doc = utils::get_doc(&class.attrs, None, ctx); if let Some(lt) = class.generics.lifetimes().next() { bail_spanned!( @@ -465,7 +466,7 @@ pub fn build_py_enum( bail_spanned!(enum_.brace_token.span.join() => "#[pyclass] can't be used on enums without any variants"); } - let doc = utils::get_doc(&enum_.attrs, None); + let doc = utils::get_doc(&enum_.attrs, None, ctx); let enum_ = PyClassEnum::new(enum_)?; impl_enum(enum_, &args, doc, method_type, ctx) } @@ -1403,7 +1404,7 @@ pub fn gen_complex_enum_variant_attr( let member = &spec.rust_ident; let wrapper_ident = format_ident!("__pymethod_variant_cls_{}__", member); let deprecations = &spec.attributes.deprecations; - let python_name = &spec.null_terminated_python_name(); + let python_name = spec.null_terminated_python_name(ctx); let variant_cls = format_ident!("{}_{}", cls, member); let associated_method = quote! { @@ -1580,7 +1581,7 @@ fn complex_enum_variant_field_getter<'a>( let property_type = crate::pymethod::PropertyType::Function { self_type: &self_type, spec: &spec, - doc: crate::get_doc(&[], None), + doc: crate::get_doc(&[], None, ctx), }; let getter = crate::pymethod::impl_py_getter_def(variant_cls_type, property_type, ctx)?; @@ -2014,7 +2015,10 @@ impl<'a> PyClassImplsBuilder<'a> { fn impl_pyclassimpl(&self, ctx: &Ctx) -> Result { let Ctx { pyo3_path } = ctx; let cls = self.cls; - let doc = self.doc.as_ref().map_or(quote! {"\0"}, |doc| quote! {#doc}); + let doc = self.doc.as_ref().map_or( + quote! {#pyo3_path::ffi::c_str!("")}, + PythonDoc::to_token_stream, + ); let is_basetype = self.attr.options.subclass.is_some(); let base = match &self.attr.options.extends { Some(extends_attr) => extends_attr.value.clone(), diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index a5f090a0e2c..147193d18dc 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -260,7 +260,7 @@ pub fn impl_wrap_pyfunction( let wrapper_ident = format_ident!("__pyfunction_{}", spec.name); let wrapper = spec.get_wrapper_function(&wrapper_ident, None, ctx)?; - let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&func.attrs), ctx); + let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&func.attrs, ctx), ctx); let wrapped_pyfunction = quote! { diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index 27188254557..a1242d49f10 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -186,7 +186,7 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec<'_>, ctx: &Ctx) -> MethodA let member = &spec.rust_ident; let wrapper_ident = format_ident!("__pymethod_{}__", member); let deprecations = &spec.attributes.deprecations; - let python_name = &spec.null_terminated_python_name(); + let python_name = spec.null_terminated_python_name(ctx); let Ctx { pyo3_path } = ctx; let associated_method = quote! { diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 013b15010bf..735b55a169d 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -227,21 +227,21 @@ pub fn gen_py_method( (_, FnType::Fn(_)) => GeneratedPyMethod::Method(impl_py_method_def( cls, spec, - &spec.get_doc(meth_attrs), + &spec.get_doc(meth_attrs, ctx), None, ctx, )?), (_, FnType::FnClass(_)) => GeneratedPyMethod::Method(impl_py_method_def( cls, spec, - &spec.get_doc(meth_attrs), + &spec.get_doc(meth_attrs, ctx), Some(quote!(#pyo3_path::ffi::METH_CLASS)), ctx, )?), (_, FnType::FnStatic) => GeneratedPyMethod::Method(impl_py_method_def( cls, spec, - &spec.get_doc(meth_attrs), + &spec.get_doc(meth_attrs, ctx), Some(quote!(#pyo3_path::ffi::METH_STATIC)), ctx, )?), @@ -255,7 +255,7 @@ pub fn gen_py_method( PropertyType::Function { self_type, spec, - doc: spec.get_doc(meth_attrs), + doc: spec.get_doc(meth_attrs, ctx), }, ctx, )?), @@ -264,7 +264,7 @@ pub fn gen_py_method( PropertyType::Function { self_type, spec, - doc: spec.get_doc(meth_attrs), + doc: spec.get_doc(meth_attrs, ctx), }, ctx, )?), @@ -499,7 +499,7 @@ fn impl_py_class_attribute( }; let wrapper_ident = format_ident!("__pymethod_{}__", name); - let python_name = spec.null_terminated_python_name(); + let python_name = spec.null_terminated_python_name(ctx); let body = quotes::ok_wrap(fncall, ctx); let associated_method = quote! { @@ -560,8 +560,8 @@ pub fn impl_py_setter_def( ctx: &Ctx, ) -> Result { let Ctx { pyo3_path } = ctx; - let python_name = property_type.null_terminated_python_name()?; - let doc = property_type.doc(); + let python_name = property_type.null_terminated_python_name(ctx)?; + let doc = property_type.doc(ctx); let mut holders = Holders::new(); let setter_impl = match property_type { PropertyType::Descriptor { @@ -746,8 +746,8 @@ pub fn impl_py_getter_def( ctx: &Ctx, ) -> Result { let Ctx { pyo3_path } = ctx; - let python_name = property_type.null_terminated_python_name()?; - let doc = property_type.doc(); + let python_name = property_type.null_terminated_python_name(ctx)?; + let doc = property_type.doc(ctx); let mut holders = Holders::new(); let body = match property_type { @@ -870,7 +870,8 @@ pub enum PropertyType<'a> { } impl PropertyType<'_> { - fn null_terminated_python_name(&self) -> Result { + fn null_terminated_python_name(&self, ctx: &Ctx) -> Result { + let Ctx { pyo3_path } = ctx; match self { PropertyType::Descriptor { field, @@ -885,23 +886,22 @@ impl PropertyType<'_> { if let Some(rule) = renaming_rule { name = utils::apply_renaming_rule(*rule, &name); } - name.push('\0'); name } (None, None) => { bail_spanned!(field.span() => "`get` and `set` with tuple struct fields require `name`"); } }; - Ok(syn::LitStr::new(&name, field.span())) + Ok(quote_spanned!(field.span() => #pyo3_path::ffi::c_str!(#name))) } - PropertyType::Function { spec, .. } => Ok(spec.null_terminated_python_name()), + PropertyType::Function { spec, .. } => Ok(spec.null_terminated_python_name(ctx)), } } - fn doc(&self) -> Cow<'_, PythonDoc> { + fn doc(&self, ctx: &Ctx) -> Cow<'_, PythonDoc> { match self { PropertyType::Descriptor { field, .. } => { - Cow::Owned(utils::get_doc(&field.attrs, None)) + Cow::Owned(utils::get_doc(&field.attrs, None, ctx)) } PropertyType::Function { doc, .. } => Cow::Borrowed(doc), } diff --git a/pyo3-macros-backend/src/utils.rs b/pyo3-macros-backend/src/utils.rs index a4c2c5e8a3b..1586379ad10 100644 --- a/pyo3-macros-backend/src/utils.rs +++ b/pyo3-macros-backend/src/utils.rs @@ -1,5 +1,5 @@ use proc_macro2::{Span, TokenStream}; -use quote::ToTokens; +use quote::{quote, ToTokens}; use syn::{punctuated::Punctuated, Token}; use crate::attributes::{CrateAttribute, RenamingRule}; @@ -81,7 +81,12 @@ pub struct PythonDoc(TokenStream); /// If this doc is for a callable, the provided `text_signature` can be passed to prepend /// this to the documentation suitable for Python to extract this into the `__text_signature__` /// attribute. -pub fn get_doc(attrs: &[syn::Attribute], mut text_signature: Option) -> PythonDoc { +pub fn get_doc( + attrs: &[syn::Attribute], + mut text_signature: Option, + ctx: &Ctx, +) -> PythonDoc { + let Ctx { pyo3_path } = ctx; // insert special divider between `__text_signature__` and doc // (assume text_signature is itself well-formed) if let Some(text_signature) = &mut text_signature { @@ -120,7 +125,7 @@ pub fn get_doc(attrs: &[syn::Attribute], mut text_signature: Option) -> } } - if !parts.is_empty() { + let tokens = if !parts.is_empty() { // Doc contained macro pieces - return as `concat!` expression if !current_part.is_empty() { parts.push(current_part.to_token_stream()); @@ -133,15 +138,14 @@ pub fn get_doc(attrs: &[syn::Attribute], mut text_signature: Option) -> syn::token::Bracket(Span::call_site()).surround(&mut tokens, |tokens| { parts.to_tokens(tokens); syn::token::Comma(Span::call_site()).to_tokens(tokens); - syn::LitStr::new("\0", Span::call_site()).to_tokens(tokens); }); - PythonDoc(tokens) + tokens } else { // Just a string doc - return directly with nul terminator - current_part.push('\0'); - PythonDoc(current_part.to_token_stream()) - } + current_part.to_token_stream() + }; + PythonDoc(quote!(#pyo3_path::ffi::c_str!(#tokens))) } impl quote::ToTokens for PythonDoc { diff --git a/src/buffer.rs b/src/buffer.rs index 558fb5e9c8d..85e0e4ce990 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -352,7 +352,7 @@ impl PyBuffer { #[inline] pub fn format(&self) -> &CStr { if self.0.format.is_null() { - CStr::from_bytes_with_nul(b"B\0").unwrap() + ffi::c_str!("B") } else { unsafe { CStr::from_ptr(self.0.format) } } @@ -723,125 +723,124 @@ mod tests { fn test_element_type_from_format() { use super::ElementType; use super::ElementType::*; - use std::ffi::CStr; use std::mem::size_of; use std::os::raw; - for (cstr, expected) in &[ + for (cstr, expected) in [ // @ prefix goes to native_element_type_from_type_char ( - "@b\0", + ffi::c_str!("@b"), SignedInteger { bytes: size_of::(), }, ), ( - "@c\0", + ffi::c_str!("@c"), UnsignedInteger { bytes: size_of::(), }, ), ( - "@b\0", + ffi::c_str!("@b"), SignedInteger { bytes: size_of::(), }, ), ( - "@B\0", + ffi::c_str!("@B"), UnsignedInteger { bytes: size_of::(), }, ), - ("@?\0", Bool), + (ffi::c_str!("@?"), Bool), ( - "@h\0", + ffi::c_str!("@h"), SignedInteger { bytes: size_of::(), }, ), ( - "@H\0", + ffi::c_str!("@H"), UnsignedInteger { bytes: size_of::(), }, ), ( - "@i\0", + ffi::c_str!("@i"), SignedInteger { bytes: size_of::(), }, ), ( - "@I\0", + ffi::c_str!("@I"), UnsignedInteger { bytes: size_of::(), }, ), ( - "@l\0", + ffi::c_str!("@l"), SignedInteger { bytes: size_of::(), }, ), ( - "@L\0", + ffi::c_str!("@L"), UnsignedInteger { bytes: size_of::(), }, ), ( - "@q\0", + ffi::c_str!("@q"), SignedInteger { bytes: size_of::(), }, ), ( - "@Q\0", + ffi::c_str!("@Q"), UnsignedInteger { bytes: size_of::(), }, ), ( - "@n\0", + ffi::c_str!("@n"), SignedInteger { bytes: size_of::(), }, ), ( - "@N\0", + ffi::c_str!("@N"), UnsignedInteger { bytes: size_of::(), }, ), - ("@e\0", Float { bytes: 2 }), - ("@f\0", Float { bytes: 4 }), - ("@d\0", Float { bytes: 8 }), - ("@z\0", Unknown), + (ffi::c_str!("@e"), Float { bytes: 2 }), + (ffi::c_str!("@f"), Float { bytes: 4 }), + (ffi::c_str!("@d"), Float { bytes: 8 }), + (ffi::c_str!("@z"), Unknown), // = prefix goes to standard_element_type_from_type_char - ("=b\0", SignedInteger { bytes: 1 }), - ("=c\0", UnsignedInteger { bytes: 1 }), - ("=B\0", UnsignedInteger { bytes: 1 }), - ("=?\0", Bool), - ("=h\0", SignedInteger { bytes: 2 }), - ("=H\0", UnsignedInteger { bytes: 2 }), - ("=l\0", SignedInteger { bytes: 4 }), - ("=l\0", SignedInteger { bytes: 4 }), - ("=I\0", UnsignedInteger { bytes: 4 }), - ("=L\0", UnsignedInteger { bytes: 4 }), - ("=q\0", SignedInteger { bytes: 8 }), - ("=Q\0", UnsignedInteger { bytes: 8 }), - ("=e\0", Float { bytes: 2 }), - ("=f\0", Float { bytes: 4 }), - ("=d\0", Float { bytes: 8 }), - ("=z\0", Unknown), - ("=0\0", Unknown), + (ffi::c_str!("=b"), SignedInteger { bytes: 1 }), + (ffi::c_str!("=c"), UnsignedInteger { bytes: 1 }), + (ffi::c_str!("=B"), UnsignedInteger { bytes: 1 }), + (ffi::c_str!("=?"), Bool), + (ffi::c_str!("=h"), SignedInteger { bytes: 2 }), + (ffi::c_str!("=H"), UnsignedInteger { bytes: 2 }), + (ffi::c_str!("=l"), SignedInteger { bytes: 4 }), + (ffi::c_str!("=l"), SignedInteger { bytes: 4 }), + (ffi::c_str!("=I"), UnsignedInteger { bytes: 4 }), + (ffi::c_str!("=L"), UnsignedInteger { bytes: 4 }), + (ffi::c_str!("=q"), SignedInteger { bytes: 8 }), + (ffi::c_str!("=Q"), UnsignedInteger { bytes: 8 }), + (ffi::c_str!("=e"), Float { bytes: 2 }), + (ffi::c_str!("=f"), Float { bytes: 4 }), + (ffi::c_str!("=d"), Float { bytes: 8 }), + (ffi::c_str!("=z"), Unknown), + (ffi::c_str!("=0"), Unknown), // unknown prefix -> Unknown - (":b\0", Unknown), + (ffi::c_str!(":b"), Unknown), ] { assert_eq!( - ElementType::from_format(CStr::from_bytes_with_nul(cstr.as_bytes()).unwrap()), - *expected, + ElementType::from_format(cstr), + expected, "element from format &Cstr: {:?}", cstr, ); diff --git a/src/conversions/num_rational.rs b/src/conversions/num_rational.rs index 2129234dc4f..e3a0c7e6d3a 100644 --- a/src/conversions/num_rational.rs +++ b/src/conversions/num_rational.rs @@ -64,28 +64,15 @@ macro_rules! rational_conversion { impl<'py> FromPyObject<'py> for Ratio<$int> { fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult { let py = obj.py(); - let py_numerator_obj = unsafe { - Bound::from_owned_ptr_or_err( - py, - ffi::PyObject_GetAttrString(obj.as_ptr(), "numerator\0".as_ptr().cast()), - ) - }; - let py_denominator_obj = unsafe { - Bound::from_owned_ptr_or_err( - py, - ffi::PyObject_GetAttrString(obj.as_ptr(), "denominator\0".as_ptr().cast()), - ) - }; + let py_numerator_obj = obj.getattr(crate::intern!(py, "numerator"))?; + let py_denominator_obj = obj.getattr(crate::intern!(py, "denominator"))?; let numerator_owned = unsafe { - Bound::from_owned_ptr_or_err( - py, - ffi::PyNumber_Long(py_numerator_obj?.as_ptr()), - )? + Bound::from_owned_ptr_or_err(py, ffi::PyNumber_Long(py_numerator_obj.as_ptr()))? }; let denominator_owned = unsafe { Bound::from_owned_ptr_or_err( py, - ffi::PyNumber_Long(py_denominator_obj?.as_ptr()), + ffi::PyNumber_Long(py_denominator_obj.as_ptr()), )? }; let rs_numerator: $int = numerator_owned.extract()?; diff --git a/src/err/err_state.rs b/src/err/err_state.rs index 14345b275c9..dc07294a0fa 100644 --- a/src/err/err_state.rs +++ b/src/err/err_state.rs @@ -240,9 +240,7 @@ fn raise_lazy(py: Python<'_>, lazy: Box) { if ffi::PyExceptionClass_Check(ptype.as_ptr()) == 0 { ffi::PyErr_SetString( PyTypeError::type_object_raw(py).cast(), - "exceptions must derive from BaseException\0" - .as_ptr() - .cast(), + ffi::c_str!("exceptions must derive from BaseException").as_ptr(), ) } else { ffi::PyErr_SetObject(ptype.as_ptr(), pvalue.as_ptr()) diff --git a/src/exceptions.rs b/src/exceptions.rs index 82bf3b668c6..496d614fc20 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -736,10 +736,10 @@ impl PyUnicodeDecodeError { let pos = err.valid_up_to(); PyUnicodeDecodeError::new_bound( py, - CStr::from_bytes_with_nul(b"utf-8\0").unwrap(), + ffi::c_str!("utf-8"), input, pos..(pos + 1), - CStr::from_bytes_with_nul(b"invalid utf-8\0").unwrap(), + ffi::c_str!("invalid utf-8"), ) } } diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 84c00acdd74..acfce37e6cf 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -5,7 +5,6 @@ use crate::{ ffi, impl_::freelist::FreeList, impl_::pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectLayout}, - internal_tricks::extract_c_string, pyclass_init::PyObjectInit, types::any::PyAnyMethods, types::PyBool, @@ -214,7 +213,7 @@ pub trait PyClassImpl: Sized + 'static { /// specialization in to the `#[pyclass]` macro from the `#[pymethods]` macro. pub fn build_pyclass_doc( class_name: &'static str, - doc: &'static str, + doc: &'static CStr, text_signature: Option<&'static str>, ) -> PyResult> { if let Some(text_signature) = text_signature { @@ -222,12 +221,12 @@ pub fn build_pyclass_doc( "{}{}\n--\n\n{}", class_name, text_signature, - doc.trim_end_matches('\0') + doc.to_str().unwrap(), )) .map_err(|_| PyValueError::new_err("class doc cannot contain nul bytes"))?; Ok(Cow::Owned(doc)) } else { - extract_c_string(doc, "class doc cannot contain nul bytes") + Ok(Cow::Borrowed(doc)) } } diff --git a/src/impl_/pyclass/lazy_type_object.rs b/src/impl_/pyclass/lazy_type_object.rs index f83fa4c5186..7afaec8a99b 100644 --- a/src/impl_/pyclass/lazy_type_object.rs +++ b/src/impl_/pyclass/lazy_type_object.rs @@ -1,5 +1,4 @@ use std::{ - borrow::Cow, cell::RefCell, ffi::CStr, marker::PhantomData, @@ -151,10 +150,8 @@ impl LazyTypeObjectInner { for class_items in items_iter { for def in class_items.methods { if let PyMethodDefType::ClassAttribute(attr) = def { - let key = attr.attribute_c_string().unwrap(); - match (attr.meth)(py) { - Ok(val) => items.push((key, val)), + Ok(val) => items.push((attr.name, val)), Err(err) => { return Err(wrap_in_runtime_error( py, @@ -162,7 +159,7 @@ impl LazyTypeObjectInner { format!( "An error occurred while initializing `{}.{}`", name, - attr.name.trim_end_matches('\0') + attr.name.to_str().unwrap() ), )) } @@ -198,7 +195,7 @@ impl LazyTypeObjectInner { fn initialize_tp_dict( py: Python<'_>, type_object: *mut ffi::PyObject, - items: Vec<(Cow<'static, CStr>, PyObject)>, + items: Vec<(&'static CStr, PyObject)>, ) -> PyResult<()> { // We hold the GIL: the dictionary update can be considered atomic from // the POV of other threads. diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index 44b2af25650..2b63d1e4ae8 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -2,7 +2,6 @@ use crate::callback::IntoPyCallbackOutput; use crate::exceptions::PyStopAsyncIteration; use crate::gil::LockGIL; use crate::impl_::panic::PanicTrap; -use crate::internal_tricks::extract_c_string; use crate::pycell::{PyBorrowError, PyBorrowMutError}; use crate::pyclass::boolean_struct::False; use crate::types::any::PyAnyMethods; @@ -12,7 +11,6 @@ use crate::{ ffi, Borrowed, Bound, DowncastError, Py, PyAny, PyClass, PyClassInitializer, PyErr, PyObject, PyRef, PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, }; -use std::borrow::Cow; use std::ffi::CStr; use std::fmt; use std::os::raw::{c_int, c_void}; @@ -84,36 +82,30 @@ pub type PyClassAttributeFactory = for<'p> fn(Python<'p>) -> PyResult; #[derive(Clone, Debug)] pub struct PyMethodDef { - pub(crate) ml_name: &'static str, + pub(crate) ml_name: &'static CStr, pub(crate) ml_meth: PyMethodType, pub(crate) ml_flags: c_int, - pub(crate) ml_doc: &'static str, + pub(crate) ml_doc: &'static CStr, } #[derive(Copy, Clone)] pub struct PyClassAttributeDef { - pub(crate) name: &'static str, + pub(crate) name: &'static CStr, pub(crate) meth: PyClassAttributeFactory, } -impl PyClassAttributeDef { - pub(crate) fn attribute_c_string(&self) -> PyResult> { - extract_c_string(self.name, "class attribute name cannot contain nul bytes") - } -} - #[derive(Clone)] pub struct PyGetterDef { - pub(crate) name: &'static str, + pub(crate) name: &'static CStr, pub(crate) meth: Getter, - pub(crate) doc: &'static str, + pub(crate) doc: &'static CStr, } #[derive(Clone)] pub struct PySetterDef { - pub(crate) name: &'static str, + pub(crate) name: &'static CStr, pub(crate) meth: Setter, - pub(crate) doc: &'static str, + pub(crate) doc: &'static CStr, } unsafe impl Sync for PyMethodDef {} @@ -125,44 +117,44 @@ unsafe impl Sync for PySetterDef {} impl PyMethodDef { /// Define a function with no `*args` and `**kwargs`. pub const fn noargs( - name: &'static str, + ml_name: &'static CStr, cfunction: ffi::PyCFunction, - doc: &'static str, + ml_doc: &'static CStr, ) -> Self { Self { - ml_name: name, + ml_name, ml_meth: PyMethodType::PyCFunction(cfunction), ml_flags: ffi::METH_NOARGS, - ml_doc: doc, + ml_doc, } } /// Define a function that can take `*args` and `**kwargs`. pub const fn cfunction_with_keywords( - name: &'static str, + ml_name: &'static CStr, cfunction: ffi::PyCFunctionWithKeywords, - doc: &'static str, + ml_doc: &'static CStr, ) -> Self { Self { - ml_name: name, + ml_name, ml_meth: PyMethodType::PyCFunctionWithKeywords(cfunction), ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS, - ml_doc: doc, + ml_doc, } } /// Define a function that can take `*args` and `**kwargs`. #[cfg(not(Py_LIMITED_API))] pub const fn fastcall_cfunction_with_keywords( - name: &'static str, + ml_name: &'static CStr, cfunction: ffi::_PyCFunctionFastWithKeywords, - doc: &'static str, + ml_doc: &'static CStr, ) -> Self { Self { - ml_name: name, + ml_name, ml_meth: PyMethodType::PyCFunctionFastWithKeywords(cfunction), ml_flags: ffi::METH_FASTCALL | ffi::METH_KEYWORDS, - ml_doc: doc, + ml_doc, } } @@ -172,7 +164,7 @@ impl PyMethodDef { } /// Convert `PyMethodDef` to Python method definition struct `ffi::PyMethodDef` - pub(crate) fn as_method_def(&self) -> PyResult<(ffi::PyMethodDef, PyMethodDefDestructor)> { + pub(crate) fn as_method_def(&self) -> ffi::PyMethodDef { let meth = match self.ml_meth { PyMethodType::PyCFunction(meth) => ffi::PyMethodDefPointer { PyCFunction: meth }, PyMethodType::PyCFunctionWithKeywords(meth) => ffi::PyMethodDefPointer { @@ -184,22 +176,18 @@ impl PyMethodDef { }, }; - let name = get_name(self.ml_name)?; - let doc = get_doc(self.ml_doc)?; - let def = ffi::PyMethodDef { - ml_name: name.as_ptr(), + ffi::PyMethodDef { + ml_name: self.ml_name.as_ptr(), ml_meth: meth, ml_flags: self.ml_flags, - ml_doc: doc.as_ptr(), - }; - let destructor = PyMethodDefDestructor { name, doc }; - Ok((def, destructor)) + ml_doc: self.ml_doc.as_ptr(), + } } } impl PyClassAttributeDef { /// Define a class attribute. - pub const fn new(name: &'static str, meth: PyClassAttributeFactory) -> Self { + pub const fn new(name: &'static CStr, meth: PyClassAttributeFactory) -> Self { Self { name, meth } } } @@ -222,7 +210,7 @@ pub(crate) type Setter = impl PyGetterDef { /// Define a getter. - pub const fn new(name: &'static str, getter: Getter, doc: &'static str) -> Self { + pub const fn new(name: &'static CStr, getter: Getter, doc: &'static CStr) -> Self { Self { name, meth: getter, @@ -233,7 +221,7 @@ impl PyGetterDef { impl PySetterDef { /// Define a setter. - pub const fn new(name: &'static str, setter: Setter, doc: &'static str) -> Self { + pub const fn new(name: &'static CStr, setter: Setter, doc: &'static CStr) -> Self { Self { name, meth: setter, @@ -284,22 +272,6 @@ where retval } -pub(crate) struct PyMethodDefDestructor { - // These members are just to avoid leaking CStrings when possible - #[allow(dead_code)] - name: Cow<'static, CStr>, - #[allow(dead_code)] - doc: Cow<'static, CStr>, -} - -pub(crate) fn get_name(name: &'static str) -> PyResult> { - extract_c_string(name, "function name cannot contain NUL byte.") -} - -pub(crate) fn get_doc(doc: &'static str) -> PyResult> { - extract_c_string(doc, "function doc cannot contain NUL byte.") -} - // Autoref-based specialization for handling `__next__` returning `Option` pub struct IterBaseTag; diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 0c3d8951fc9..b05652bced8 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -1,6 +1,6 @@ //! Implementation details of `#[pymodule]` which need to be accessible from proc-macro generated code. -use std::{cell::UnsafeCell, marker::PhantomData}; +use std::{cell::UnsafeCell, ffi::CStr, marker::PhantomData}; #[cfg(all( not(any(PyPy, GraalPy)), @@ -49,12 +49,9 @@ unsafe impl Sync for ModuleDef {} impl ModuleDef { /// Make new module definition with given module name. - /// - /// # Safety - /// `name` and `doc` must be null-terminated strings. pub const unsafe fn new( - name: &'static str, - doc: &'static str, + name: &'static CStr, + doc: &'static CStr, initializer: ModuleInitializer, ) -> Self { const INIT: ffi::PyModuleDef = ffi::PyModuleDef { @@ -70,8 +67,8 @@ impl ModuleDef { }; let ffi_def = UnsafeCell::new(ffi::PyModuleDef { - m_name: name.as_ptr().cast(), - m_doc: doc.as_ptr().cast(), + m_name: name.as_ptr(), + m_doc: doc.as_ptr(), ..INIT }); @@ -215,10 +212,12 @@ impl PyAddToModule for ModuleDef { mod tests { use std::{ borrow::Cow, + ffi::CStr, sync::atomic::{AtomicBool, Ordering}, }; use crate::{ + ffi, types::{any::PyAnyMethods, module::PyModuleMethods, PyModule}, Bound, PyResult, Python, }; @@ -229,8 +228,8 @@ mod tests { fn module_init() { static MODULE_DEF: ModuleDef = unsafe { ModuleDef::new( - "test_module\0", - "some doc\0", + ffi::c_str!("test_module"), + ffi::c_str!("some doc"), ModuleInitializer(|m| { m.add("SOME_CONSTANT", 42)?; Ok(()) @@ -270,8 +269,8 @@ mod tests { fn module_def_new() { // To get coverage for ModuleDef::new() need to create a non-static ModuleDef, however init // etc require static ModuleDef, so this test needs to be separated out. - static NAME: &str = "test_module\0"; - static DOC: &str = "some doc\0"; + static NAME: &CStr = ffi::c_str!("test_module"); + static DOC: &CStr = ffi::c_str!("some doc"); static INIT_CALLED: AtomicBool = AtomicBool::new(false); diff --git a/src/internal_tricks.rs b/src/internal_tricks.rs index a8873dda007..0ee424f9db4 100644 --- a/src/internal_tricks.rs +++ b/src/internal_tricks.rs @@ -1,13 +1,4 @@ -use std::{ - borrow::Cow, - ffi::{CStr, CString}, -}; - -use crate::{ - exceptions::PyValueError, - ffi::{Py_ssize_t, PY_SSIZE_T_MAX}, - PyResult, -}; +use crate::ffi::{Py_ssize_t, PY_SSIZE_T_MAX}; pub struct PrivateMarker; macro_rules! private_decl { @@ -193,31 +184,6 @@ pub(crate) fn slice_index_order_fail(index: usize, end: usize) -> ! { panic!("slice index starts at {} but ends at {}", index, end); } -pub(crate) fn extract_c_string( - src: &'static str, - err_msg: &'static str, -) -> PyResult> { - let bytes = src.as_bytes(); - let cow = match bytes { - [] => { - // Empty string, we can trivially refer to a static "\0" string - Cow::Borrowed(unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") }) - } - [.., 0] => { - // Last byte is a nul; try to create as a CStr - let c_str = - CStr::from_bytes_with_nul(bytes).map_err(|_| PyValueError::new_err(err_msg))?; - Cow::Borrowed(c_str) - } - _ => { - // Allocate a new CString for this - let c_string = CString::new(bytes).map_err(|_| PyValueError::new_err(err_msg))?; - Cow::Owned(c_string) - } - }; - Ok(cow) -} - // TODO: use ptr::from_ref on MSRV 1.76 #[inline] pub(crate) const fn ptr_from_ref(t: &T) -> *const T { diff --git a/src/macros.rs b/src/macros.rs index 9316b871390..ab91d577ac5 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -214,7 +214,7 @@ macro_rules! append_to_inittab { ); } $crate::ffi::PyImport_AppendInittab( - $module::__PYO3_NAME.as_ptr().cast(), + $module::__PYO3_NAME.as_ptr(), ::std::option::Option::Some($module::__pyo3_init), ); } diff --git a/src/marker.rs b/src/marker.rs index 62d8a89ba53..dae4fcab44d 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -652,7 +652,7 @@ impl<'py> Python<'py> { ) -> PyResult> { let code = CString::new(code)?; unsafe { - let mptr = ffi::PyImport_AddModule("__main__\0".as_ptr().cast()); + let mptr = ffi::PyImport_AddModule(ffi::c_str!("__main__").as_ptr()); if mptr.is_null() { return Err(PyErr::fetch(self)); } @@ -685,7 +685,8 @@ impl<'py> Python<'py> { } } - let code_obj = ffi::Py_CompileString(code.as_ptr(), "\0".as_ptr() as _, start); + let code_obj = + ffi::Py_CompileString(code.as_ptr(), ffi::c_str!("").as_ptr(), start); if code_obj.is_null() { return Err(PyErr::fetch(self)); } diff --git a/src/pyclass/create_type_object.rs b/src/pyclass/create_type_object.rs index 01b357763ad..fd1d2b34998 100644 --- a/src/pyclass/create_type_object.rs +++ b/src/pyclass/create_type_object.rs @@ -7,7 +7,7 @@ use crate::{ assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, tp_dealloc_with_gc, PyClassItemsIter, }, - pymethods::{get_doc, get_name, Getter, Setter}, + pymethods::{Getter, Setter}, trampoline::trampoline, }, internal_tricks::ptr_from_ref, @@ -15,7 +15,6 @@ use crate::{ Py, PyClass, PyGetterDef, PyMethodDefType, PyResult, PySetterDef, PyTypeInfo, Python, }; use std::{ - borrow::Cow, collections::HashMap, ffi::{CStr, CString}, os::raw::{c_char, c_int, c_ulong, c_void}, @@ -103,7 +102,7 @@ type PyTypeBuilderCleanup = Box; struct PyTypeBuilder { slots: Vec, method_defs: Vec, - getset_builders: HashMap<&'static str, GetSetDefBuilder>, + getset_builders: HashMap<&'static CStr, GetSetDefBuilder>, /// Used to patch the type objects for the things there's no /// PyType_FromSpec API for... there's no reason this should work, /// except for that it does and we have tests. @@ -173,32 +172,25 @@ impl PyTypeBuilder { fn pymethod_def(&mut self, def: &PyMethodDefType) { match def { - PyMethodDefType::Getter(getter) => { - self.getset_builders - .entry(getter.name) - .or_default() - .add_getter(getter); - } - PyMethodDefType::Setter(setter) => { - self.getset_builders - .entry(setter.name) - .or_default() - .add_setter(setter); - } + PyMethodDefType::Getter(getter) => self + .getset_builders + .entry(getter.name) + .or_default() + .add_getter(getter), + PyMethodDefType::Setter(setter) => self + .getset_builders + .entry(setter.name) + .or_default() + .add_setter(setter), PyMethodDefType::Method(def) | PyMethodDefType::Class(def) - | PyMethodDefType::Static(def) => { - let (def, destructor) = def.as_method_def().unwrap(); - // FIXME: stop leaking destructor - std::mem::forget(destructor); - self.method_defs.push(def); - } + | PyMethodDefType::Static(def) => self.method_defs.push(def.as_method_def()), // These class attributes are added after the type gets created by LazyStaticType PyMethodDefType::ClassAttribute(_) => {} } } - fn finalize_methods_and_properties(&mut self) -> PyResult> { + fn finalize_methods_and_properties(&mut self) -> Vec { let method_defs: Vec = std::mem::take(&mut self.method_defs); // Safety: Py_tp_methods expects a raw vec of PyMethodDef unsafe { self.push_raw_vec_slot(ffi::Py_tp_methods, method_defs) }; @@ -210,11 +202,11 @@ impl PyTypeBuilder { .getset_builders .iter() .map(|(name, builder)| { - let (def, destructor) = builder.as_get_set_def(name)?; + let (def, destructor) = builder.as_get_set_def(name); getset_destructors.push(destructor); - Ok(def) + def }) - .collect::>()?; + .collect(); // PyPy automatically adds __dict__ getter / setter. #[cfg(not(PyPy))] @@ -261,7 +253,7 @@ impl PyTypeBuilder { } property_defs.push(ffi::PyGetSetDef { - name: "__dict__\0".as_ptr().cast(), + name: ffi::c_str!("__dict__").as_ptr(), get: Some(get_dict), set: Some(ffi::PyObject_GenericSetDict), doc: ptr::null(), @@ -300,7 +292,7 @@ impl PyTypeBuilder { } } - Ok(getset_destructors) + getset_destructors } fn set_is_basetype(mut self, is_basetype: bool) -> Self { @@ -358,7 +350,7 @@ impl PyTypeBuilder { #[cfg(Py_3_9)] { #[inline(always)] - fn offset_def(name: &'static str, offset: ffi::Py_ssize_t) -> ffi::PyMemberDef { + fn offset_def(name: &'static CStr, offset: ffi::Py_ssize_t) -> ffi::PyMemberDef { ffi::PyMemberDef { name: name.as_ptr().cast(), type_code: ffi::Py_T_PYSSIZET, @@ -372,12 +364,15 @@ impl PyTypeBuilder { // __dict__ support if let Some(dict_offset) = dict_offset { - members.push(offset_def("__dictoffset__\0", dict_offset)); + members.push(offset_def(ffi::c_str!("__dictoffset__"), dict_offset)); } // weakref support if let Some(weaklist_offset) = weaklist_offset { - members.push(offset_def("__weaklistoffset__\0", weaklist_offset)); + members.push(offset_def( + ffi::c_str!("__weaklistoffset__"), + weaklist_offset, + )); } // Safety: Py_tp_members expects a raw vec of PyMemberDef @@ -417,7 +412,7 @@ impl PyTypeBuilder { // on some platforms (like windows) #![allow(clippy::useless_conversion)] - let getset_destructors = self.finalize_methods_and_properties()?; + let getset_destructors = self.finalize_methods_and_properties(); unsafe { self.push_slot(ffi::Py_tp_base, self.tp_base) } @@ -531,7 +526,7 @@ unsafe extern "C" fn no_constructor_defined( #[derive(Default)] struct GetSetDefBuilder { - doc: Option<&'static str>, + doc: Option<&'static CStr>, getter: Option, setter: Option, } @@ -555,13 +550,7 @@ impl GetSetDefBuilder { self.setter = Some(setter.meth) } - fn as_get_set_def( - &self, - name: &'static str, - ) -> PyResult<(ffi::PyGetSetDef, GetSetDefDestructor)> { - let name = get_name(name)?; - let doc = self.doc.map(get_doc).transpose()?; - + fn as_get_set_def(&self, name: &'static CStr) -> (ffi::PyGetSetDef, GetSetDefDestructor) { let getset_type = match (self.getter, self.setter) { (Some(getter), None) => GetSetDefType::Getter(getter), (None, Some(setter)) => GetSetDefType::Setter(setter), @@ -573,20 +562,16 @@ impl GetSetDefBuilder { } }; - let getset_def = getset_type.create_py_get_set_def(&name, doc.as_deref()); + let getset_def = getset_type.create_py_get_set_def(name, self.doc); let destructor = GetSetDefDestructor { - name, - doc, closure: getset_type, }; - Ok((getset_def, destructor)) + (getset_def, destructor) } } #[allow(dead_code)] // a stack of fields which are purely to cache until dropped struct GetSetDefDestructor { - name: Cow<'static, CStr>, - doc: Option>, closure: GetSetDefType, } diff --git a/src/types/function.rs b/src/types/function.rs index a127b4e0574..c2eec04d42f 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -8,7 +8,7 @@ use crate::types::module::PyModuleMethods; use crate::PyNativeType; use crate::{ ffi, - impl_::pymethods::{self, PyMethodDef, PyMethodDefDestructor}, + impl_::pymethods::{self, PyMethodDef}, types::{PyCapsule, PyDict, PyModule, PyString, PyTuple}, }; use crate::{Bound, IntoPy, Py, PyAny, PyResult, Python}; @@ -30,8 +30,8 @@ impl PyCFunction { )] pub fn new_with_keywords<'a>( fun: ffi::PyCFunctionWithKeywords, - name: &'static str, - doc: &'static str, + name: &'static CStr, + doc: &'static CStr, py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a Self> { let (py, module) = py_or_module.into_py_and_maybe_module(); @@ -44,11 +44,14 @@ impl PyCFunction { } /// Create a new built-in function with keywords (*args and/or **kwargs). + /// + /// To create `name` and `doc` static strings on Rust versions older than 1.77 (which added c"" literals), + /// use the [`c_str!`](crate::ffi::c_str) macro. pub fn new_with_keywords_bound<'py>( py: Python<'py>, fun: ffi::PyCFunctionWithKeywords, - name: &'static str, - doc: &'static str, + name: &'static CStr, + doc: &'static CStr, module: Option<&Bound<'py, PyModule>>, ) -> PyResult> { Self::internal_new( @@ -66,8 +69,8 @@ impl PyCFunction { )] pub fn new<'a>( fun: ffi::PyCFunction, - name: &'static str, - doc: &'static str, + name: &'static CStr, + doc: &'static CStr, py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a Self> { let (py, module) = py_or_module.into_py_and_maybe_module(); @@ -80,11 +83,14 @@ impl PyCFunction { } /// Create a new built-in function which takes no arguments. + /// + /// To create `name` and `doc` static strings on Rust versions older than 1.77 (which added c"" literals), + /// use the [`c_str!`](crate::ffi::c_str) macro. pub fn new_bound<'py>( py: Python<'py>, fun: ffi::PyCFunction, - name: &'static str, - doc: &'static str, + name: &'static CStr, + doc: &'static CStr, module: Option<&Bound<'py, PyModule>>, ) -> PyResult> { Self::internal_new(py, &PyMethodDef::noargs(name, fun, doc), module) @@ -98,8 +104,8 @@ impl PyCFunction { )] pub fn new_closure<'a, F, R>( py: Python<'a>, - name: Option<&'static str>, - doc: Option<&'static str>, + name: Option<&'static CStr>, + doc: Option<&'static CStr>, closure: F, ) -> PyResult<&'a PyCFunction> where @@ -131,29 +137,27 @@ impl PyCFunction { /// ``` pub fn new_closure_bound<'py, F, R>( py: Python<'py>, - name: Option<&'static str>, - doc: Option<&'static str>, + name: Option<&'static CStr>, + doc: Option<&'static CStr>, closure: F, ) -> PyResult> where F: Fn(&Bound<'_, PyTuple>, Option<&Bound<'_, PyDict>>) -> R + Send + 'static, R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, { - let method_def = pymethods::PyMethodDef::cfunction_with_keywords( - name.unwrap_or("pyo3-closure\0"), - run_closure::, - doc.unwrap_or("\0"), - ); - let (def, def_destructor) = method_def.as_method_def()?; + let name = name.unwrap_or(ffi::c_str!("pyo3-closure")); + let doc = doc.unwrap_or(ffi::c_str!("")); + let method_def = + pymethods::PyMethodDef::cfunction_with_keywords(name, run_closure::, doc); + let def = method_def.as_method_def(); let capsule = PyCapsule::new_bound( py, ClosureDestructor:: { closure, def: UnsafeCell::new(def), - def_destructor, }, - Some(closure_capsule_name().to_owned()), + Some(CLOSURE_CAPSULE_NAME.to_owned()), )?; // Safety: just created the capsule with type ClosureDestructor above @@ -178,11 +182,10 @@ impl PyCFunction { } else { (std::ptr::null_mut(), None) }; - let (def, destructor) = method_def.as_method_def()?; + let def = method_def.as_method_def(); - // FIXME: stop leaking the def and destructor + // FIXME: stop leaking the def let def = Box::into_raw(Box::new(def)); - std::mem::forget(destructor); let module_name_ptr = module_name .as_ref() @@ -196,10 +199,7 @@ impl PyCFunction { } } -fn closure_capsule_name() -> &'static CStr { - // TODO replace this with const CStr once MSRV new enough - CStr::from_bytes_with_nul(b"pyo3-closure\0").unwrap() -} +static CLOSURE_CAPSULE_NAME: &CStr = ffi::c_str!("pyo3-closure"); unsafe extern "C" fn run_closure( capsule_ptr: *mut ffi::PyObject, @@ -218,7 +218,7 @@ where kwargs, |py, capsule_ptr, args, kwargs| { let boxed_fn: &ClosureDestructor = - &*(ffi::PyCapsule_GetPointer(capsule_ptr, closure_capsule_name().as_ptr()) + &*(ffi::PyCapsule_GetPointer(capsule_ptr, CLOSURE_CAPSULE_NAME.as_ptr()) as *mut ClosureDestructor); let args = Bound::ref_from_ptr(py, &args).downcast_unchecked::(); let kwargs = Bound::ref_from_ptr_or_opt(py, &kwargs) @@ -235,9 +235,6 @@ struct ClosureDestructor { // Wrapped in UnsafeCell because Python C-API wants a *mut pointer // to this member. def: UnsafeCell, - // Used to destroy the cstrings in `def`, if necessary. - #[allow(dead_code)] - def_destructor: PyMethodDefDestructor, } // Safety: F is send and none of the fields are ever mutated diff --git a/src/types/string.rs b/src/types/string.rs index 8556e41dcce..828e0024bda 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -78,7 +78,7 @@ impl<'a> PyStringData<'a> { Err(PyUnicodeDecodeError::new_bound( py, - CStr::from_bytes_with_nul(b"utf-16\0").unwrap(), + ffi::c_str!("utf-16"), self.as_bytes(), 0..self.as_bytes().len(), CStr::from_bytes_with_nul(&message).unwrap(), @@ -90,10 +90,10 @@ impl<'a> PyStringData<'a> { Some(s) => Ok(Cow::Owned(s)), None => Err(PyUnicodeDecodeError::new_bound( py, - CStr::from_bytes_with_nul(b"utf-32\0").unwrap(), + ffi::c_str!("utf-32"), self.as_bytes(), 0..self.as_bytes().len(), - CStr::from_bytes_with_nul(b"error converting utf-32\0").unwrap(), + ffi::c_str!("error converting utf-32"), )? .into()), }, @@ -414,8 +414,8 @@ impl<'a> Borrowed<'a, '_, PyString> { let bytes = unsafe { ffi::PyUnicode_AsEncodedString( ptr, - b"utf-8\0".as_ptr().cast(), - b"surrogatepass\0".as_ptr().cast(), + ffi::c_str!("utf-8").as_ptr(), + ffi::c_str!("surrogatepass").as_ptr(), ) .assume_owned(py) .downcast_into_unchecked::() diff --git a/tests/test_buffer.rs b/tests/test_buffer.rs index 0b3da881884..9e2a6a4d1c5 100644 --- a/tests/test_buffer.rs +++ b/tests/test_buffer.rs @@ -3,7 +3,6 @@ use pyo3::{buffer::PyBuffer, exceptions::PyBufferError, ffi, prelude::*}; use std::{ - ffi::CStr, os::raw::{c_int, c_void}, ptr, }; @@ -48,7 +47,7 @@ impl TestBufferErrors { (*view).readonly = 1; (*view).itemsize = std::mem::size_of::() as isize; - let msg = CStr::from_bytes_with_nul(b"I\0").unwrap(); + let msg = ffi::c_str!("I"); (*view).format = msg.as_ptr() as *mut _; (*view).ndim = 1; @@ -72,7 +71,7 @@ impl TestBufferErrors { (*view).itemsize += 1; } IncorrectFormat => { - (*view).format = CStr::from_bytes_with_nul(b"B\0").unwrap().as_ptr() as _; + (*view).format = ffi::c_str!("B").as_ptr() as _; } IncorrectAlignment => (*view).buf = (*view).buf.add(1), } diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 9028f71a232..f3a04a53a95 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; #[cfg(not(Py_LIMITED_API))] use pyo3::buffer::PyBuffer; +use pyo3::ffi::c_str; use pyo3::prelude::*; #[cfg(not(Py_LIMITED_API))] use pyo3::types::PyDateTime; @@ -344,8 +345,8 @@ fn test_pycfunction_new() { let py_fn = PyCFunction::new_bound( py, c_fn, - "py_fn", - "py_fn for test (this is the docstring)", + c_str!("py_fn"), + c_str!("py_fn for test (this is the docstring)"), None, ) .unwrap(); @@ -402,8 +403,8 @@ fn test_pycfunction_new_with_keywords() { let py_fn = PyCFunction::new_with_keywords_bound( py, c_fn, - "py_fn", - "py_fn for test (this is the docstring)", + c_str!("py_fn"), + c_str!("py_fn for test (this is the docstring)"), None, ) .unwrap(); @@ -443,8 +444,13 @@ fn test_closure() { Ok(res) }) }; - let closure_py = - PyCFunction::new_closure_bound(py, Some("test_fn"), Some("test_fn doc"), f).unwrap(); + let closure_py = PyCFunction::new_closure_bound( + py, + Some(c_str!("test_fn")), + Some(c_str!("test_fn doc")), + f, + ) + .unwrap(); py_assert!(py, closure_py, "closure_py(42) == [43]"); py_assert!(py, closure_py, "closure_py.__name__ == 'test_fn'");