From 9a5668572ba16794fddaedf8d9f61f8ac7576542 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 21 Nov 2023 15:54:15 +0000 Subject: [PATCH 1/2] implement `PyModuleMethods` --- src/prelude.rs | 1 + src/py_result_ext.rs | 8 +- src/types/mod.rs | 2 +- src/types/module.rs | 386 +++++++++++++++++++++++++++++++++++++------ 4 files changed, 340 insertions(+), 57 deletions(-) diff --git a/src/prelude.rs b/src/prelude.rs index 9be5dc0dbba..60ce03063b8 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -34,6 +34,7 @@ pub use crate::types::float::PyFloatMethods; pub use crate::types::frozenset::PyFrozenSetMethods; pub use crate::types::list::PyListMethods; pub use crate::types::mapping::PyMappingMethods; +pub use crate::types::module::PyModuleMethods; pub use crate::types::sequence::PySequenceMethods; pub use crate::types::set::PySetMethods; pub use crate::types::string::PyStringMethods; diff --git a/src/py_result_ext.rs b/src/py_result_ext.rs index da0e5c2a5e0..66309988dc4 100644 --- a/src/py_result_ext.rs +++ b/src/py_result_ext.rs @@ -1,4 +1,4 @@ -use crate::{types::any::PyAnyMethods, Bound, PyAny, PyResult}; +use crate::{types::any::PyAnyMethods, Bound, PyAny, PyResult, PyTypeCheck}; mod sealed { use super::*; @@ -11,10 +11,16 @@ mod sealed { use sealed::Sealed; pub(crate) trait PyResultExt<'py>: Sealed { + fn downcast_into(self) -> PyResult>; unsafe fn downcast_into_unchecked(self) -> PyResult>; } impl<'py> PyResultExt<'py> for PyResult> { + #[inline] + fn downcast_into(self) -> PyResult> where { + self.and_then(|instance| instance.downcast_into().map_err(Into::into)) + } + #[inline] unsafe fn downcast_into_unchecked(self) -> PyResult> { self.map(|instance| instance.downcast_into_unchecked()) diff --git a/src/types/mod.rs b/src/types/mod.rs index 705d366601d..c2885f4630c 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -294,7 +294,7 @@ mod iterator; pub(crate) mod list; pub(crate) mod mapping; mod memoryview; -mod module; +pub(crate) mod module; mod none; mod notimplemented; mod num; diff --git a/src/types/module.rs b/src/types/module.rs index 701a9131307..aa6649a9533 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -1,11 +1,12 @@ use crate::callback::IntoPyCallbackOutput; use crate::err::{PyErr, PyResult}; -use crate::exceptions; -use crate::ffi; +use crate::ffi_ptr_ext::FfiPtrExt; use crate::pyclass::PyClass; -use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString}; -use crate::{IntoPy, Py, PyObject, Python}; -use std::ffi::{CStr, CString}; +use crate::types::{ + any::PyAnyMethods, list::PyListMethods, PyAny, PyCFunction, PyDict, PyList, PyString, +}; +use crate::{exceptions, ffi, Bound, IntoPy, Py, PyNativeType, PyObject, Python}; +use std::ffi::CString; use std::str; /// Represents a Python [`module`][1] object. @@ -151,11 +152,7 @@ impl PyModule { /// Returns the module's `__dict__` attribute, which contains the module's symbol table. pub fn dict(&self) -> &PyDict { - unsafe { - // PyModule_GetDict returns borrowed ptr; must make owned for safety (see #890). - let ptr = ffi::PyModule_GetDict(self.as_ptr()); - self.py().from_owned_ptr(ffi::_Py_NewRef(ptr)) - } + self.as_borrowed().dict().into_gil_ref() } /// Returns the index (the `__all__` attribute) of the module, @@ -163,34 +160,14 @@ impl PyModule { /// /// `__all__` declares the items that will be imported with `from my_module import *`. pub fn index(&self) -> PyResult<&PyList> { - let __all__ = __all__(self.py()); - match self.getattr(__all__) { - Ok(idx) => idx.downcast().map_err(PyErr::from), - Err(err) => { - if err.is_instance_of::(self.py()) { - let l = PyList::empty(self.py()); - self.setattr(__all__, l).map_err(PyErr::from)?; - Ok(l) - } else { - Err(err) - } - } - } + self.as_borrowed().index().map(Bound::into_gil_ref) } /// Returns the name (the `__name__` attribute) of the module. /// /// May fail if the module does not have a `__name__` attribute. pub fn name(&self) -> PyResult<&str> { - let ptr = unsafe { ffi::PyModule_GetName(self.as_ptr()) }; - if ptr.is_null() { - Err(PyErr::fetch(self.py())) - } else { - let name = unsafe { CStr::from_ptr(ptr) } - .to_str() - .expect("PyModule_GetName expected to return utf8"); - Ok(name) - } + self.as_borrowed().name()?.into_gil_ref().to_str() } /// Returns the filename (the `__file__` attribute) of the module. @@ -198,11 +175,7 @@ impl PyModule { /// May fail if the module does not have a `__file__` attribute. #[cfg(not(PyPy))] pub fn filename(&self) -> PyResult<&str> { - unsafe { - self.py() - .from_owned_ptr_or_err::(ffi::PyModule_GetFilenameObject(self.as_ptr()))? - .to_str() - } + self.as_borrowed().filename()?.into_gil_ref().to_str() } /// Adds an attribute to the module. @@ -239,10 +212,7 @@ impl PyModule { where V: IntoPy, { - self.index()? - .append(name) - .expect("could not append __name__ to __all__"); - self.setattr(name, value.into_py(self.py())) + self.as_borrowed().add(name, value) } /// Adds a new class to the module. @@ -287,8 +257,7 @@ impl PyModule { where T: PyClass, { - let py = self.py(); - self.add(T::NAME, T::lazy_type_object().get_or_try_init(py)?) + self.as_borrowed().add_class::() } /// Adds a function or a (sub)module to a module, using the functions name as name. @@ -298,14 +267,7 @@ impl PyModule { where T: IntoPyCallbackOutput, { - self._add_wrapped(wrapper(self.py()).convert(self.py())?) - } - - fn _add_wrapped(&self, object: PyObject) -> PyResult<()> { - let py = self.py(); - let name = object.getattr(py, __name__(py))?; - let name = name.extract(py)?; - self.add(name, object) + self.as_borrowed().add_wrapped(wrapper) } /// Adds a submodule to a module. @@ -349,8 +311,7 @@ impl PyModule { /// [1]: https://github.com/PyO3/pyo3/issues/759 /// [2]: https://github.com/PyO3/pyo3/issues/1517#issuecomment-808664021 pub fn add_submodule(&self, module: &PyModule) -> PyResult<()> { - let name = module.name()?; - self.add(name, module) + self.as_borrowed().add_submodule(&module.as_borrowed()) } /// Add a function to a module. @@ -388,8 +349,314 @@ impl PyModule { /// [1]: crate::prelude::pyfunction /// [2]: crate::wrap_pyfunction pub fn add_function<'a>(&'a self, fun: &'a PyCFunction) -> PyResult<()> { - let name = fun.getattr(__name__(self.py()))?.extract()?; - self.add(name, fun) + self.as_borrowed().add_function(&fun.as_borrowed()) + } +} + +/// Implementation of functionality for [`PyModule`]. +/// +/// These methods are defined for the `Bound<'py, PyModule>` smart pointer, so to use method call +/// syntax these methods are separated into a trait, because stable Rust does not yet support +/// `arbitrary_self_types`. +#[doc(alias = "PyModule")] +pub trait PyModuleMethods<'py> { + /// Returns the module's `__dict__` attribute, which contains the module's symbol table. + fn dict(&self) -> Bound<'py, PyDict>; + + /// Returns the index (the `__all__` attribute) of the module, + /// creating one if needed. + /// + /// `__all__` declares the items that will be imported with `from my_module import *`. + fn index(&self) -> PyResult>; + + /// Returns the name (the `__name__` attribute) of the module. + /// + /// May fail if the module does not have a `__name__` attribute. + fn name(&self) -> PyResult>; + + /// Returns the filename (the `__file__` attribute) of the module. + /// + /// May fail if the module does not have a `__file__` attribute. + #[cfg(not(PyPy))] + fn filename(&self) -> PyResult>; + + /// Adds an attribute to the module. + /// + /// For adding classes, functions or modules, prefer to use [`PyModule::add_class`], + /// [`PyModule::add_function`] or [`PyModule::add_submodule`] instead, respectively. + /// + /// # Examples + /// + /// ```rust + /// use pyo3::prelude::*; + /// + /// #[pymodule] + /// fn my_module(_py: Python<'_>, module: &PyModule) -> PyResult<()> { + /// module.add("c", 299_792_458)?; + /// Ok(()) + /// } + /// ``` + /// + /// Python code can then do the following: + /// + /// ```python + /// from my_module import c + /// + /// print("c is", c) + /// ``` + /// + /// This will result in the following output: + /// + /// ```text + /// c is 299792458 + /// ``` + fn add(&self, name: N, value: V) -> PyResult<()> + where + N: IntoPy>, + V: IntoPy; + + /// Adds a new class to the module. + /// + /// Notice that this method does not take an argument. + /// Instead, this method is *generic*, and requires us to use the + /// "turbofish" syntax to specify the class we want to add. + /// + /// # Examples + /// + /// ```rust + /// use pyo3::prelude::*; + /// + /// #[pyclass] + /// struct Foo { /* fields omitted */ } + /// + /// #[pymodule] + /// fn my_module(_py: Python<'_>, module: &PyModule) -> PyResult<()> { + /// module.add_class::()?; + /// Ok(()) + /// } + /// ``` + /// + /// Python code can see this class as such: + /// ```python + /// from my_module import Foo + /// + /// print("Foo is", Foo) + /// ``` + /// + /// This will result in the following output: + /// ```text + /// Foo is + /// ``` + /// + /// Note that as we haven't defined a [constructor][1], Python code can't actually + /// make an *instance* of `Foo` (or *get* one for that matter, as we haven't exported + /// anything that can return instances of `Foo`). + /// + /// [1]: https://pyo3.rs/latest/class.html#constructor + fn add_class(&self) -> PyResult<()> + where + T: PyClass; + + /// Adds a function or a (sub)module to a module, using the functions name as name. + /// + /// Prefer to use [`PyModule::add_function`] and/or [`PyModule::add_submodule`] instead. + fn add_wrapped(&self, wrapper: &impl Fn(Python<'py>) -> T) -> PyResult<()> + where + T: IntoPyCallbackOutput; + + /// Adds a submodule to a module. + /// + /// This is especially useful for creating module hierarchies. + /// + /// Note that this doesn't define a *package*, so this won't allow Python code + /// to directly import submodules by using + /// `from my_module import submodule`. + /// For more information, see [#759][1] and [#1517][2]. + /// + /// # Examples + /// + /// ```rust + /// use pyo3::prelude::*; + /// + /// #[pymodule] + /// fn my_module(py: Python<'_>, module: &PyModule) -> PyResult<()> { + /// let submodule = PyModule::new(py, "submodule")?; + /// submodule.add("super_useful_constant", "important")?; + /// + /// module.add_submodule(submodule)?; + /// Ok(()) + /// } + /// ``` + /// + /// Python code can then do the following: + /// + /// ```python + /// import my_module + /// + /// print("super_useful_constant is", my_module.submodule.super_useful_constant) + /// ``` + /// + /// This will result in the following output: + /// + /// ```text + /// super_useful_constant is important + /// ``` + /// + /// [1]: https://github.com/PyO3/pyo3/issues/759 + /// [2]: https://github.com/PyO3/pyo3/issues/1517#issuecomment-808664021 + fn add_submodule(&self, module: &Bound<'_, PyModule>) -> PyResult<()>; + + /// Add a function to a module. + /// + /// Note that this also requires the [`wrap_pyfunction!`][2] macro + /// to wrap a function annotated with [`#[pyfunction]`][1]. + /// + /// ```rust + /// use pyo3::prelude::*; + /// + /// #[pyfunction] + /// fn say_hello() { + /// println!("Hello world!") + /// } + /// #[pymodule] + /// fn my_module(_py: Python<'_>, module: &PyModule) -> PyResult<()> { + /// module.add_function(wrap_pyfunction!(say_hello, module)?) + /// } + /// ``` + /// + /// Python code can then do the following: + /// + /// ```python + /// from my_module import say_hello + /// + /// say_hello() + /// ``` + /// + /// This will result in the following output: + /// + /// ```text + /// Hello world! + /// ``` + /// + /// [1]: crate::prelude::pyfunction + /// [2]: crate::wrap_pyfunction + fn add_function(&self, fun: &Bound<'_, PyCFunction>) -> PyResult<()>; +} + +impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { + fn dict(&self) -> Bound<'py, PyDict> { + unsafe { + // PyModule_GetDict returns borrowed ptr; must make owned for safety (see #890). + ffi::PyModule_GetDict(self.as_ptr()) + .assume_borrowed(self.py()) + .to_owned() + .downcast_into_unchecked() + } + } + + fn index(&self) -> PyResult> { + let __all__ = __all__(self.py()); + match self.getattr(__all__) { + Ok(idx) => idx.downcast_into().map_err(PyErr::from), + Err(err) => { + if err.is_instance_of::(self.py()) { + let l = PyList::empty(self.py()).as_borrowed().to_owned(); + self.setattr(__all__, &l).map_err(PyErr::from)?; + Ok(l) + } else { + Err(err) + } + } + } + } + + fn name(&self) -> PyResult> { + #[cfg(not(PyPy))] + { + use crate::py_result_ext::PyResultExt; + + unsafe { + ffi::PyModule_GetNameObject(self.as_ptr()) + .assume_owned_or_err(self.py()) + .downcast_into_unchecked() + } + } + + #[cfg(PyPy)] + { + self.dict() + .get_item("__name__") + .map_err(|_| exceptions::PyAttributeError::new_err("__name__"))? + .downcast_into() + .map_err(PyErr::from) + } + } + + #[cfg(not(PyPy))] + fn filename(&self) -> PyResult> { + use crate::py_result_ext::PyResultExt; + + unsafe { + ffi::PyModule_GetFilenameObject(self.as_ptr()) + .assume_owned_or_err(self.py()) + .downcast_into_unchecked() + } + } + + fn add(&self, name: N, value: V) -> PyResult<()> + where + N: IntoPy>, + V: IntoPy, + { + fn inner( + module: &Bound<'_, PyModule>, + name: Bound<'_, PyString>, + value: Bound<'_, PyAny>, + ) -> PyResult<()> { + module + .index()? + .append(&name) + .expect("could not append __name__ to __all__"); + module.setattr(name, value.into_py(module.py())) + } + + let py = self.py(); + inner( + self, + name.into_py(py).into_bound(py), + value.into_py(py).into_bound(py), + ) + } + + fn add_class(&self) -> PyResult<()> + where + T: PyClass, + { + let py = self.py(); + self.add(T::NAME, T::lazy_type_object().get_or_try_init(py)?) + } + + fn add_wrapped(&self, wrapper: &impl Fn(Python<'py>) -> T) -> PyResult<()> + where + T: IntoPyCallbackOutput, + { + fn inner(module: &Bound<'_, PyModule>, object: Bound<'_, PyAny>) -> PyResult<()> { + let name = object.getattr(__name__(module.py()))?; + module.add(name.downcast_into::()?, object) + } + + let py = self.py(); + inner(self, wrapper(py).convert(py)?.into_bound(py)) + } + + fn add_submodule(&self, module: &Bound<'_, PyModule>) -> PyResult<()> { + let name = module.name()?; + self.add(name, module) + } + + fn add_function(&self, fun: &Bound<'_, PyCFunction>) -> PyResult<()> { + let name = fun.getattr(__name__(self.py()))?; + self.add(name.downcast_into::()?, fun) } } @@ -412,4 +679,13 @@ mod tests { assert_eq!(builtins.name().unwrap(), "builtins"); }) } + + #[test] + #[cfg(not(PyPy))] + fn module_filename() { + Python::with_gil(|py| { + let site = PyModule::import(py, "site").unwrap(); + assert!(site.filename().unwrap().ends_with("site.py")); + }) + } } From e852a4b50256fbd56a9f2cb5b3f7a33a31105153 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 26 Dec 2023 22:54:39 +0000 Subject: [PATCH 2/2] be more lax with ordering in `invalid_result_conversion` ui test --- tests/test_compile_error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 0c278bdfa62..0154f3f12bc 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -33,7 +33,7 @@ fn test_compile_errors() { t.compile_fail("tests/ui/invalid_pymethod_receiver.rs"); t.compile_fail("tests/ui/missing_intopy.rs"); // adding extra error conversion impls changes the output - #[cfg(not(any(windows, feature = "eyre", feature = "anyhow")))] + #[cfg(not(any(windows, feature = "eyre", feature = "anyhow", Py_LIMITED_API)))] t.compile_fail("tests/ui/invalid_result_conversion.rs"); t.compile_fail("tests/ui/not_send.rs"); t.compile_fail("tests/ui/not_send2.rs");