From 8cabd2619c349ffede8e141c09e540c0c83adb79 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Mon, 1 Apr 2024 16:18:57 +0100 Subject: [PATCH 01/31] docs: updates to guide for PyO3 0.21 feedback (#4031) * docs: add notes on smart pointer conversions * guide: add more notes on `.extract::<&str>()` to migration guide --- guide/src/migration.md | 16 +++++++++++++++- guide/src/types.md | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/guide/src/migration.md b/guide/src/migration.md index 69cf8922255..d855f69d396 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -246,14 +246,26 @@ Because the new `Bound` API brings ownership out of the PyO3 framework and in - `Bound::iter_borrowed` is slightly more efficient than `Bound::iter`. The default iteration of `Bound` cannot return borrowed references because Rust does not (yet) have "lending iterators". Similarly `Bound::get_borrowed_item` is more efficient than `Bound::get_item` for the same reason. - `&Bound` does not implement `FromPyObject` (although it might be possible to do this in the future once the GIL Refs API is completely removed). Use `bound_any.downcast::()` instead of `bound_any.extract::<&Bound>()`. - `Bound::to_str` now borrows from the `Bound` rather than from the `'py` lifetime, so code will need to store the smart pointer as a value in some cases where previously `&PyString` was just used as a temporary. (There are some more details relating to this in [the section below](#deactivating-the-gil-refs-feature).) +- `.extract::<&str>()` now borrows from the source Python object. The simplest way to update is to change to `.extract::()`, which retains ownership of the Python reference. See more information [in the section on deactivating the `gil-refs` feature](#deactivating-the-gil-refs-feature). -To convert between `&PyAny` and `&Bound` you can use the `as_borrowed()` method: +To convert between `&PyAny` and `&Bound` use the `as_borrowed()` method: ```rust,ignore let gil_ref: &PyAny = ...; let bound: &Bound = &gil_ref.as_borrowed(); ``` +To convert between `Py` and `Bound` use the `bind()` / `into_bound()` methods, and `as_unbound()` / `unbind()` to go back from `Bound` to `Py`. + +```rust,ignore +let obj: Py = ...; +let bound: &Bound<'py, PyList> = obj.bind(py); +let bound: Bound<'py, PyList> = obj.into_bound(py); + +let obj: &Py = bound.as_unbound(); +let obj: Py = bound.unbind(); +``` +
⚠️ Warning: dangling pointer trap πŸ’£ @@ -325,6 +337,8 @@ There is just one case of code that changes upon disabling these features: `From To make PyO3's core functionality continue to work while the GIL Refs API is in the process of being removed, disabling the `gil-refs` feature moves the implementations of `FromPyObject` for `&str`, `Cow<'_, str>`, `&[u8]`, `Cow<'_, u8>` to a new temporary trait `FromPyObjectBound`. This trait is the expected future form of `FromPyObject` and has an additional lifetime `'a` to enable these types to borrow data from Python objects. +PyO3 0.21 has introduced the [`PyBackedStr`]({{#PYO3_DOCS_URL}}/pyo3/pybacked/struct.PyBackedStr.html) and [`PyBackedBytes`]({{#PYO3_DOCS_URL}}/pyo3/pybacked/struct.PyBackedBytes.html) types to help with this case. The easiest way to avoid lifetime challenges from extracting `&str` is to use these. For more complex types like `Vec<&str>`, is now impossible to extract directly from a Python object and `Vec` is the recommended upgrade path. + A key thing to note here is because extracting to these types now ties them to the input lifetime, some extremely common patterns may need to be split into multiple Rust lines. For example, the following snippet of calling `.extract::<&str>()` directly on the result of `.getattr()` needs to be adjusted when deactivating the `gil-refs-migration` feature. Before: diff --git a/guide/src/types.md b/guide/src/types.md index cd5e1052920..4c63d175991 100644 --- a/guide/src/types.md +++ b/guide/src/types.md @@ -160,6 +160,45 @@ for i in 0..=2 { # Python::with_gil(example).unwrap(); ``` +### Casting between smart pointer types + +To convert between `Py` and `Bound<'py, T>` use the `bind()` / `into_bound()` methods. Use the `as_unbound()` / `unbind()` methods to go back from `Bound<'py, T>` to `Py`. + +```rust,ignore +let obj: Py = ...; +let bound: &Bound<'py, PyAny> = obj.bind(py); +let bound: Bound<'py, PyAny> = obj.into_bound(py); + +let obj: &Py = bound.as_unbound(); +let obj: Py = bound.unbind(); +``` + +To convert between `Bound<'py, T>` and `Borrowed<'a, 'py, T>` use the `as_borrowed()` method. `Borrowed<'a, 'py, T>` has a deref coercion to `Bound<'py, T>`. Use the `to_owned()` method to increment the Python reference count and to create a new `Bound<'py, T>` from the `Borrowed<'a, 'py, T>`. + +```rust,ignore +let bound: Bound<'py, PyAny> = ...; +let borrowed: Borrowed<'_, 'py, PyAny> = bound.as_borrowed(); + +// deref coercion +let bound: &Bound<'py, PyAny> = &borrowed; + +// create a new Bound by increase the Python reference count +let bound: Bound<'py, PyAny> = borrowed.to_owned(); +``` + +To convert between `Py` and `Borrowed<'a, 'py, T>` use the `bind_borrowed()` method. Use either `as_unbound()` or `.to_owned().unbind()` to go back to `Py` from `Borrowed<'a, 'py, T>`, via `Bound<'py, T>`. + +```rust,ignore +let obj: Py = ...; +let borrowed: Borrowed<'_, 'py, PyAny> = bound.as_borrowed(); + +// via deref coercion to Bound and then using Bound::as_unbound +let obj: &Py = borrowed.as_unbound(); + +// via a new Bound by increasing the Python reference count, and unbind it +let obj: Py = borrowed.to_owned().unbind(). +``` + ## Concrete Python types In all of `Py`, `Bound<'py, T>`, and `Borrowed<'a, 'py, T>`, the type parameter `T` denotes the type of the Python object referred to by the smart pointer. From c1f11fb4bddc836f820e0db3db514b4742b8a3e7 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Mon, 1 Apr 2024 19:51:58 +0100 Subject: [PATCH 02/31] release: 0.21.1 (#4032) --- CHANGELOG.md | 24 +++++++++++++++++++ Cargo.toml | 8 +++---- README.md | 4 ++-- examples/decorator/.template/pre-script.rhai | 2 +- .../maturin-starter/.template/pre-script.rhai | 2 +- examples/plugin/.template/pre-script.rhai | 2 +- .../.template/pre-script.rhai | 2 +- examples/word-count/.template/pre-script.rhai | 2 +- newsfragments/3995.fixed.md | 1 - newsfragments/3998.changed.md | 1 - newsfragments/3998.fixed.md | 1 - newsfragments/4007.fixed.md | 1 - newsfragments/4009.fixed.md | 1 - newsfragments/4012.fixed.md | 1 - newsfragments/4015.fixed.md | 1 - newsfragments/4020.added.md | 1 - newsfragments/4024.changed.md | 1 - newsfragments/4027.added.md | 1 - pyo3-build-config/Cargo.toml | 2 +- pyo3-ffi/Cargo.toml | 4 ++-- pyo3-macros-backend/Cargo.toml | 4 ++-- pyo3-macros/Cargo.toml | 4 ++-- pyproject.toml | 2 +- 23 files changed, 43 insertions(+), 29 deletions(-) delete mode 100644 newsfragments/3995.fixed.md delete mode 100644 newsfragments/3998.changed.md delete mode 100644 newsfragments/3998.fixed.md delete mode 100644 newsfragments/4007.fixed.md delete mode 100644 newsfragments/4009.fixed.md delete mode 100644 newsfragments/4012.fixed.md delete mode 100644 newsfragments/4015.fixed.md delete mode 100644 newsfragments/4020.added.md delete mode 100644 newsfragments/4024.changed.md delete mode 100644 newsfragments/4027.added.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 047a4b21e1a..6f4ce218021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,28 @@ To see unreleased changes, please see the [CHANGELOG on the main branch guide](h +## [0.21.1] - 2024-04-01 + +### Added + +- Implement `Send` and `Sync` for `PyBackedStr` and `PyBackedBytes`. [#4007](https://github.com/PyO3/pyo3/pull/4007) +- Implement `Clone`, `Debug`, `PartialEq`, `Eq`, `PartialOrd`, `Ord` and `Hash` implementation for `PyBackedBytes` and `PyBackedStr`, and `Display` for `PyBackedStr`. [#4020](https://github.com/PyO3/pyo3/pull/4020) +- Add `import_exception_bound!` macro to import exception types without generating GIL Ref functionality for them. [#4027](https://github.com/PyO3/pyo3/pull/4027) + +### Changed + +- Emit deprecation warning for uses of GIL Refs as `#[setter]` function arguments. [#3998](https://github.com/PyO3/pyo3/pull/3998) +- Add `#[inline]` hints on many `Bound` and `Borrowed` methods. [#4024](https://github.com/PyO3/pyo3/pull/4024) + +### Fixed + +- Handle `#[pyo3(from_py_with = "")]` in `#[setter]` methods [#3995](https://github.com/PyO3/pyo3/pull/3995) +- Allow extraction of `&Bound` in `#[setter]` methods. [#3998](https://github.com/PyO3/pyo3/pull/3998) +- Fix some uncovered code blocks emitted by `#[pymodule]`, `#[pyfunction]` and `#[pyclass]` macros. [#4009](https://github.com/PyO3/pyo3/pull/4009) +- Fix typo in the panic message when a class referenced in `pyo3::import_exception!` does not exist. [#4012](https://github.com/PyO3/pyo3/pull/4012) +- Fix compile error when using an async `#[pymethod]` with a receiver and additional arguments. [#4015](https://github.com/PyO3/pyo3/pull/4015) + + ## [0.21.0] - 2024-03-25 ### Added @@ -1709,6 +1731,8 @@ Yanked - Initial release +[Unreleased]: https://github.com/pyo3/pyo3/compare/v0.21.1...HEAD +[0.21.1]: https://github.com/pyo3/pyo3/compare/v0.21.0...v0.21.1 [0.21.0]: https://github.com/pyo3/pyo3/compare/v0.20.3...v0.21.0 [0.21.0-beta.0]: https://github.com/pyo3/pyo3/compare/v0.20.3...v0.21.0-beta.0 [0.20.3]: https://github.com/pyo3/pyo3/compare/v0.20.2...v0.20.3 diff --git a/Cargo.toml b/Cargo.toml index dfbeb2d601d..fdd1fa9d29a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3" -version = "0.21.0" +version = "0.21.1" description = "Bindings to Python interpreter" authors = ["PyO3 Project and Contributors "] readme = "README.md" @@ -22,10 +22,10 @@ memoffset = "0.9" portable-atomic = "1.0" # ffi bindings to the python interpreter, split into a separate crate so they can be used independently -pyo3-ffi = { path = "pyo3-ffi", version = "=0.21.0" } +pyo3-ffi = { path = "pyo3-ffi", version = "=0.21.1" } # support crates for macros feature -pyo3-macros = { path = "pyo3-macros", version = "=0.21.0", optional = true } +pyo3-macros = { path = "pyo3-macros", version = "=0.21.1", optional = true } indoc = { version = "2.0.1", optional = true } unindent = { version = "0.2.1", optional = true } @@ -60,7 +60,7 @@ rayon = "1.6.1" futures = "0.3.28" [build-dependencies] -pyo3-build-config = { path = "pyo3-build-config", version = "=0.21.0", features = ["resolve-config"] } +pyo3-build-config = { path = "pyo3-build-config", version = "=0.21.1", features = ["resolve-config"] } [features] default = ["macros"] diff --git a/README.md b/README.md index 7d79f028dcd..8e7e2f75bca 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ name = "string_sum" crate-type = ["cdylib"] [dependencies] -pyo3 = { version = "0.21.0", features = ["extension-module"] } +pyo3 = { version = "0.21.1", features = ["extension-module"] } ``` **`src/lib.rs`** @@ -137,7 +137,7 @@ Start a new project with `cargo new` and add `pyo3` to the `Cargo.toml` like th ```toml [dependencies.pyo3] -version = "0.21.0" +version = "0.21.1" features = ["auto-initialize"] ``` diff --git a/examples/decorator/.template/pre-script.rhai b/examples/decorator/.template/pre-script.rhai index cebb06c0f61..1dc689ef6d4 100644 --- a/examples/decorator/.template/pre-script.rhai +++ b/examples/decorator/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.0"); +variable::set("PYO3_VERSION", "0.21.1"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/pyproject.toml", "pyproject.toml"); file::delete(".template"); diff --git a/examples/maturin-starter/.template/pre-script.rhai b/examples/maturin-starter/.template/pre-script.rhai index cebb06c0f61..1dc689ef6d4 100644 --- a/examples/maturin-starter/.template/pre-script.rhai +++ b/examples/maturin-starter/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.0"); +variable::set("PYO3_VERSION", "0.21.1"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/pyproject.toml", "pyproject.toml"); file::delete(".template"); diff --git a/examples/plugin/.template/pre-script.rhai b/examples/plugin/.template/pre-script.rhai index 4b290f7f7e0..78adb883f43 100644 --- a/examples/plugin/.template/pre-script.rhai +++ b/examples/plugin/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.0"); +variable::set("PYO3_VERSION", "0.21.1"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/plugin_api/Cargo.toml", "plugin_api/Cargo.toml"); file::delete(".template"); diff --git a/examples/setuptools-rust-starter/.template/pre-script.rhai b/examples/setuptools-rust-starter/.template/pre-script.rhai index fbbe7ed0e7c..212f62f76fe 100644 --- a/examples/setuptools-rust-starter/.template/pre-script.rhai +++ b/examples/setuptools-rust-starter/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.0"); +variable::set("PYO3_VERSION", "0.21.1"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/setup.cfg", "setup.cfg"); file::delete(".template"); diff --git a/examples/word-count/.template/pre-script.rhai b/examples/word-count/.template/pre-script.rhai index cebb06c0f61..1dc689ef6d4 100644 --- a/examples/word-count/.template/pre-script.rhai +++ b/examples/word-count/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.0"); +variable::set("PYO3_VERSION", "0.21.1"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/pyproject.toml", "pyproject.toml"); file::delete(".template"); diff --git a/newsfragments/3995.fixed.md b/newsfragments/3995.fixed.md deleted file mode 100644 index e47a71b790d..00000000000 --- a/newsfragments/3995.fixed.md +++ /dev/null @@ -1 +0,0 @@ -handle `#[pyo3(from_py_with = "")]` in `#[setter]` methods \ No newline at end of file diff --git a/newsfragments/3998.changed.md b/newsfragments/3998.changed.md deleted file mode 100644 index c02c6546c95..00000000000 --- a/newsfragments/3998.changed.md +++ /dev/null @@ -1 +0,0 @@ -Warn on uses of GIL Refs for `#[setter]` function arguments. \ No newline at end of file diff --git a/newsfragments/3998.fixed.md b/newsfragments/3998.fixed.md deleted file mode 100644 index 11f5d006ec0..00000000000 --- a/newsfragments/3998.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Allow extraction of `&Bound` in `#[setter]` methods. \ No newline at end of file diff --git a/newsfragments/4007.fixed.md b/newsfragments/4007.fixed.md deleted file mode 100644 index ff905fb4e94..00000000000 --- a/newsfragments/4007.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Implement `Send` and `Sync` for `PyBackedStr` and `PyBackedBytes`. diff --git a/newsfragments/4009.fixed.md b/newsfragments/4009.fixed.md deleted file mode 100644 index a5d378e12ad..00000000000 --- a/newsfragments/4009.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Fix some uncovered code blocks emitted by `#[pymodule]`, `#[pyfunction]` and `#[pyclass]` macros. diff --git a/newsfragments/4012.fixed.md b/newsfragments/4012.fixed.md deleted file mode 100644 index 352ec928487..00000000000 --- a/newsfragments/4012.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Fixed the error message when a class referenced in `pyo3::import_exception!` does not exist diff --git a/newsfragments/4015.fixed.md b/newsfragments/4015.fixed.md deleted file mode 100644 index a8f4f636c76..00000000000 --- a/newsfragments/4015.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Fix the bug that an async `#[pymethod]` with receiver can't have any other args. \ No newline at end of file diff --git a/newsfragments/4020.added.md b/newsfragments/4020.added.md deleted file mode 100644 index 53d0d6b8fd3..00000000000 --- a/newsfragments/4020.added.md +++ /dev/null @@ -1 +0,0 @@ -Adds `Clone`, `Debug`, `PartialEq`, `Eq`, `PartialOrd`, `Ord` and `Hash` implementation for `PyBackedBytes` and `PyBackedStr`, and `Display` for `PyBackedStr`. diff --git a/newsfragments/4024.changed.md b/newsfragments/4024.changed.md deleted file mode 100644 index 65afd8f0d0d..00000000000 --- a/newsfragments/4024.changed.md +++ /dev/null @@ -1 +0,0 @@ -Add `#[inline]` hints on many `Bound` and `Borrowed` methods. diff --git a/newsfragments/4027.added.md b/newsfragments/4027.added.md deleted file mode 100644 index ccf32952da2..00000000000 --- a/newsfragments/4027.added.md +++ /dev/null @@ -1 +0,0 @@ -Add `import_exception_bound!` macro to import exception types without generating GIL Ref functionality for them. diff --git a/pyo3-build-config/Cargo.toml b/pyo3-build-config/Cargo.toml index f19ded99697..1eb269c2132 100644 --- a/pyo3-build-config/Cargo.toml +++ b/pyo3-build-config/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-build-config" -version = "0.21.0" +version = "0.21.1" description = "Build configuration for the PyO3 ecosystem" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] diff --git a/pyo3-ffi/Cargo.toml b/pyo3-ffi/Cargo.toml index 091e5f898ef..64753976fab 100644 --- a/pyo3-ffi/Cargo.toml +++ b/pyo3-ffi/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-ffi" -version = "0.21.0" +version = "0.21.1" description = "Python-API bindings for the PyO3 ecosystem" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -38,7 +38,7 @@ abi3-py312 = ["abi3", "pyo3-build-config/abi3-py312"] generate-import-lib = ["pyo3-build-config/python3-dll-a"] [build-dependencies] -pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.0", features = ["resolve-config"] } +pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.1", features = ["resolve-config"] } [lints] workspace = true diff --git a/pyo3-macros-backend/Cargo.toml b/pyo3-macros-backend/Cargo.toml index 822dc372b47..6ca4eeade8c 100644 --- a/pyo3-macros-backend/Cargo.toml +++ b/pyo3-macros-backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-macros-backend" -version = "0.21.0" +version = "0.21.1" description = "Code generation for PyO3 package" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -16,7 +16,7 @@ edition = "2021" [dependencies] heck = "0.4" proc-macro2 = { version = "1", default-features = false } -pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.0", features = ["resolve-config"] } +pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.1", features = ["resolve-config"] } quote = { version = "1", default-features = false } [dependencies.syn] diff --git a/pyo3-macros/Cargo.toml b/pyo3-macros/Cargo.toml index a70ccb0ae7b..39a4b9198c6 100644 --- a/pyo3-macros/Cargo.toml +++ b/pyo3-macros/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-macros" -version = "0.21.0" +version = "0.21.1" description = "Proc macros for PyO3 package" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -22,7 +22,7 @@ experimental-declarative-modules = [] proc-macro2 = { version = "1", default-features = false } quote = "1" syn = { version = "2", features = ["full", "extra-traits"] } -pyo3-macros-backend = { path = "../pyo3-macros-backend", version = "=0.21.0" } +pyo3-macros-backend = { path = "../pyo3-macros-backend", version = "=0.21.1" } [lints] workspace = true diff --git a/pyproject.toml b/pyproject.toml index 1a3cfa4aaad..d474753ccd1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [tool.towncrier] filename = "CHANGELOG.md" -version = "0.21.0" +version = "0.21.1" start_string = "\n" template = ".towncrier.template.md" title_format = "## [{version}] - {project_date}" From a4aea230ed6087a5fccc415d6456c536645e0df1 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 2 Apr 2024 18:43:51 +0100 Subject: [PATCH 03/31] fix compile error for multiple async method arguments (#4035) --- newsfragments/4035.fixed.md | 1 + pyo3-macros-backend/src/method.rs | 25 ++++-------- tests/test_coroutine.rs | 66 +++++++++++++++---------------- 3 files changed, 41 insertions(+), 51 deletions(-) create mode 100644 newsfragments/4035.fixed.md diff --git a/newsfragments/4035.fixed.md b/newsfragments/4035.fixed.md new file mode 100644 index 00000000000..5425c5cbaf7 --- /dev/null +++ b/newsfragments/4035.fixed.md @@ -0,0 +1 @@ +Fix compile error for `async fn` in `#[pymethods]` with a `&self` receiver and more than one additional argument. diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 155c554025d..6af0ec97d04 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -522,33 +522,22 @@ impl<'a> FnSpec<'a> { Some(cls) => quote!(Some(<#cls as #pyo3_path::PyTypeInfo>::NAME)), None => quote!(None), }; - let evaluate_args = || -> (Vec, TokenStream) { - let mut arg_names = Vec::with_capacity(args.len()); - let mut evaluate_arg = quote! {}; - for arg in &args { - let arg_name = format_ident!("arg_{}", arg_names.len()); - arg_names.push(arg_name.clone()); - evaluate_arg.extend(quote! { - let #arg_name = #arg - }); - } - (arg_names, evaluate_arg) - }; + let arg_names = (0..args.len()) + .map(|i| format_ident!("arg_{}", i)) + .collect::>(); let future = match self.tp { FnType::Fn(SelfType::Receiver { mutable: false, .. }) => { - let (arg_name, evaluate_arg) = evaluate_args(); quote! {{ - #evaluate_arg; + #(let #arg_names = #args;)* let __guard = #pyo3_path::impl_::coroutine::RefGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?; - async move { function(&__guard, #(#arg_name),*).await } + async move { function(&__guard, #(#arg_names),*).await } }} } FnType::Fn(SelfType::Receiver { mutable: true, .. }) => { - let (arg_name, evaluate_arg) = evaluate_args(); quote! {{ - #evaluate_arg; + #(let #arg_names = #args;)* let mut __guard = #pyo3_path::impl_::coroutine::RefMutGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?; - async move { function(&mut __guard, #(#arg_name),*).await } + async move { function(&mut __guard, #(#arg_names),*).await } }} } _ => { diff --git a/tests/test_coroutine.rs b/tests/test_coroutine.rs index 0e698deb9af..4abba9f36b4 100644 --- a/tests/test_coroutine.rs +++ b/tests/test_coroutine.rs @@ -245,39 +245,6 @@ fn coroutine_panic() { }) } -#[test] -fn test_async_method_receiver_with_other_args() { - #[pyclass] - struct Value(i32); - #[pymethods] - impl Value { - #[new] - fn new() -> Self { - Self(0) - } - async fn get_value_plus_with(&self, v: i32) -> i32 { - self.0 + v - } - async fn set_value(&mut self, new_value: i32) -> i32 { - self.0 = new_value; - self.0 - } - } - - Python::with_gil(|gil| { - let test = r#" - import asyncio - - v = Value() - assert asyncio.run(v.get_value_plus_with(3)) == 3 - assert asyncio.run(v.set_value(10)) == 10 - assert asyncio.run(v.get_value_plus_with(1)) == 11 - "#; - let locals = [("Value", gil.get_type_bound::())].into_py_dict_bound(gil); - py_run!(gil, *locals, test); - }); -} - #[test] fn test_async_method_receiver() { #[pyclass] @@ -341,3 +308,36 @@ fn test_async_method_receiver() { assert!(IS_DROPPED.load(Ordering::SeqCst)); } + +#[test] +fn test_async_method_receiver_with_other_args() { + #[pyclass] + struct Value(i32); + #[pymethods] + impl Value { + #[new] + fn new() -> Self { + Self(0) + } + async fn get_value_plus_with(&self, v1: i32, v2: i32) -> i32 { + self.0 + v1 + v2 + } + async fn set_value(&mut self, new_value: i32) -> i32 { + self.0 = new_value; + self.0 + } + } + + Python::with_gil(|gil| { + let test = r#" + import asyncio + + v = Value() + assert asyncio.run(v.get_value_plus_with(3, 0)) == 3 + assert asyncio.run(v.set_value(10)) == 10 + assert asyncio.run(v.get_value_plus_with(1, 1)) == 12 + "#; + let locals = [("Value", gil.get_type_bound::())].into_py_dict_bound(gil); + py_run!(gil, *locals, test); + }); +} From 7a00b4d357c0250c2212e856782143f283a7c2bb Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Thu, 4 Apr 2024 21:08:51 +0200 Subject: [PATCH 04/31] add descriptive error msg for `__traverse__` receivers other than `&self` (#4045) * add descriptive error msg for `__traverse__` receivers other than `self` * add newsfragment * improve error message --- newsfragments/4045.fixed.md | 1 + pyo3-macros-backend/src/pymethod.rs | 21 ++++++++++-- tests/ui/traverse.rs | 50 ++++++++++++++++++++++++++--- tests/ui/traverse.stderr | 42 +++++++++++++----------- 4 files changed, 88 insertions(+), 26 deletions(-) create mode 100644 newsfragments/4045.fixed.md diff --git a/newsfragments/4045.fixed.md b/newsfragments/4045.fixed.md new file mode 100644 index 00000000000..6b2bbcfa01d --- /dev/null +++ b/newsfragments/4045.fixed.md @@ -0,0 +1 @@ +Add better error message on wrong receiver extraction in `__traverse__`. diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index ee7d3d7aaee..abe8c7ac8a3 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -434,9 +434,24 @@ fn impl_traverse_slot( let Ctx { pyo3_path } = ctx; if let (Some(py_arg), _) = split_off_python_arg(&spec.signature.arguments) { return Err(syn::Error::new_spanned(py_arg.ty, "__traverse__ may not take `Python`. \ - Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. \ - Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, \ - i.e. `Python::with_gil` will panic.")); + Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` \ + should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited \ + inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.")); + } + + // check that the receiver does not try to smuggle an (implicit) `Python` token into here + if let FnType::Fn(SelfType::TryFromBoundRef(span)) + | FnType::Fn(SelfType::Receiver { + mutable: true, + span, + }) = spec.tp + { + bail_spanned! { span => + "__traverse__ may not take a receiver other than `&self`. Usually, an implementation of \ + `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` \ + should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited \ + inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic." + } } let rust_fn_ident = spec.name; diff --git a/tests/ui/traverse.rs b/tests/ui/traverse.rs index 034224951c9..0cf7170db21 100644 --- a/tests/ui/traverse.rs +++ b/tests/ui/traverse.rs @@ -1,13 +1,55 @@ use pyo3::prelude::*; -use pyo3::PyVisit; use pyo3::PyTraverseError; +use pyo3::PyVisit; #[pyclass] struct TraverseTriesToTakePyRef {} #[pymethods] impl TraverseTriesToTakePyRef { - fn __traverse__(slf: PyRef, visit: PyVisit) {} + fn __traverse__(slf: PyRef, visit: PyVisit) -> Result<(), PyTraverseError> { + Ok(()) + } +} + +#[pyclass] +struct TraverseTriesToTakePyRefMut {} + +#[pymethods] +impl TraverseTriesToTakePyRefMut { + fn __traverse__(slf: PyRefMut, visit: PyVisit) -> Result<(), PyTraverseError> { + Ok(()) + } +} + +#[pyclass] +struct TraverseTriesToTakeBound {} + +#[pymethods] +impl TraverseTriesToTakeBound { + fn __traverse__(slf: Bound<'_, Self>, visit: PyVisit) -> Result<(), PyTraverseError> { + Ok(()) + } +} + +#[pyclass] +struct TraverseTriesToTakeMutSelf {} + +#[pymethods] +impl TraverseTriesToTakeMutSelf { + fn __traverse__(&mut self, visit: PyVisit) -> Result<(), PyTraverseError> { + Ok(()) + } +} + +#[pyclass] +struct TraverseTriesToTakeSelf {} + +#[pymethods] +impl TraverseTriesToTakeSelf { + fn __traverse__(&self, visit: PyVisit) -> Result<(), PyTraverseError> { + Ok(()) + } } #[pyclass] @@ -19,9 +61,7 @@ impl Class { Ok(()) } - fn __clear__(&mut self) { - } + fn __clear__(&mut self) {} } - fn main() {} diff --git a/tests/ui/traverse.stderr b/tests/ui/traverse.stderr index 4448e67e13a..5b1d1b6b2ec 100644 --- a/tests/ui/traverse.stderr +++ b/tests/ui/traverse.stderr @@ -1,23 +1,29 @@ -error: __traverse__ may not take `Python`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. - --> tests/ui/traverse.rs:18:32 +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. + --> tests/ui/traverse.rs:10:26 | -18 | fn __traverse__(&self, py: Python<'_>, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { - | ^^^^^^^^^^ +10 | fn __traverse__(slf: PyRef, visit: PyVisit) -> Result<(), PyTraverseError> { + | ^^^^^ + +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. + --> tests/ui/traverse.rs:20:26 + | +20 | fn __traverse__(slf: PyRefMut, visit: PyVisit) -> Result<(), PyTraverseError> { + | ^^^^^^^^ -error[E0308]: mismatched types - --> tests/ui/traverse.rs:9:6 +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. + --> tests/ui/traverse.rs:30:26 | -8 | #[pymethods] - | ------------ arguments to this function are incorrect -9 | impl TraverseTriesToTakePyRef { - | ______^ -10 | | fn __traverse__(slf: PyRef, visit: PyVisit) {} - | |___________________^ expected fn pointer, found fn item +30 | fn __traverse__(slf: Bound<'_, Self>, visit: PyVisit) -> Result<(), PyTraverseError> { + | ^^^^^ + +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. + --> tests/ui/traverse.rs:40:21 | - = note: expected fn pointer `for<'a, 'b> fn(&'a TraverseTriesToTakePyRef, PyVisit<'b>) -> Result<(), PyTraverseError>` - found fn item `for<'a, 'b> fn(pyo3::PyRef<'a, TraverseTriesToTakePyRef, >, PyVisit<'b>) {TraverseTriesToTakePyRef::__traverse__}` -note: function defined here - --> src/impl_/pymethods.rs +40 | fn __traverse__(&mut self, visit: PyVisit) -> Result<(), PyTraverseError> { + | ^ + +error: __traverse__ may not take `Python`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. + --> tests/ui/traverse.rs:60:32 | - | pub unsafe fn _call_traverse( - | ^^^^^^^^^^^^^^ +60 | fn __traverse__(&self, py: Python<'_>, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + | ^^^^^^^^^^ From 2f0869a6d643e790e71dd5a676a30a7e539e5138 Mon Sep 17 00:00:00 2001 From: Joseph Perez Date: Sun, 7 Apr 2024 00:36:52 +0200 Subject: [PATCH 05/31] fix: allow impl of Ungil for deprecated PyCell in nightly (#4053) --- src/marker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/marker.rs b/src/marker.rs index 67119b5e55e..b1f2d399209 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -280,6 +280,7 @@ mod nightly { impl !Ungil for crate::PyAny {} // All the borrowing wrappers + #[allow(deprecated)] impl !Ungil for crate::PyCell {} impl !Ungil for crate::PyRef<'_, T> {} impl !Ungil for crate::PyRefMut<'_, T> {} From 47f9ef41747ff1e7d89f584797ea9fc0d88dd1d5 Mon Sep 17 00:00:00 2001 From: "Jeong, Heon" Date: Thu, 11 Apr 2024 14:07:56 -0700 Subject: [PATCH 06/31] Fix typo (#4052) Fix incorrect closing brackets --- guide/src/ecosystem/async-await.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guide/src/ecosystem/async-await.md b/guide/src/ecosystem/async-await.md index 0319fa05063..9c0ad19bdef 100644 --- a/guide/src/ecosystem/async-await.md +++ b/guide/src/ecosystem/async-await.md @@ -120,7 +120,7 @@ Export an async function that makes use of `async-std`: use pyo3::{prelude::*, wrap_pyfunction}; #[pyfunction] -fn rust_sleep(py: Python<'_>) -> PyResult<&Bound<'_, PyAny>>> { +fn rust_sleep(py: Python<'_>) -> PyResult<&Bound<'_, PyAny>> { pyo3_asyncio::async_std::future_into_py(py, async { async_std::task::sleep(std::time::Duration::from_secs(1)).await; Ok(Python::with_gil(|py| py.None())) From 631c25f2f9ef2d2f73ea1510cc6aba0581b0af16 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 11 Apr 2024 21:08:44 +0000 Subject: [PATCH 07/31] build(deps): bump peaceiris/actions-gh-pages from 3 to 4 (#4062) Bumps [peaceiris/actions-gh-pages](https://github.com/peaceiris/actions-gh-pages) from 3 to 4. - [Release notes](https://github.com/peaceiris/actions-gh-pages/releases) - [Changelog](https://github.com/peaceiris/actions-gh-pages/blob/main/CHANGELOG.md) - [Commits](https://github.com/peaceiris/actions-gh-pages/compare/v3...v4) --- updated-dependencies: - dependency-name: peaceiris/actions-gh-pages dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/gh-pages.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gh-pages.yml b/.github/workflows/gh-pages.yml index a9a7669054a..a101eb6ad25 100644 --- a/.github/workflows/gh-pages.yml +++ b/.github/workflows/gh-pages.yml @@ -46,7 +46,7 @@ jobs: - name: Deploy docs and the guide if: ${{ github.event_name == 'release' }} - uses: peaceiris/actions-gh-pages@v3 + uses: peaceiris/actions-gh-pages@v4 with: github_token: ${{ secrets.GITHUB_TOKEN }} publish_dir: ./target/guide/ From 9a6b1962d39c0acd6ddc42b186151611018cbd69 Mon Sep 17 00:00:00 2001 From: Liam Date: Thu, 11 Apr 2024 22:11:51 +0100 Subject: [PATCH 08/31] Minor: Fix a typo in Contributing.md (#4066) --- Contributing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Contributing.md b/Contributing.md index 29d1bb758a7..054099b431a 100644 --- a/Contributing.md +++ b/Contributing.md @@ -190,7 +190,7 @@ Second, there is a Python-based benchmark contained in the `pytests` subdirector You can view what code is and isn't covered by PyO3's tests. We aim to have 100% coverage - please check coverage and add tests if you notice a lack of coverage! -- First, ensure the llmv-cov cargo plugin is installed. You may need to run the plugin through cargo once before using it with `nox`. +- First, ensure the llvm-cov cargo plugin is installed. You may need to run the plugin through cargo once before using it with `nox`. ```shell cargo install cargo-llvm-cov cargo llvm-cov From c8b59d71176a371f9feaa8abfa884f9fd451fbb5 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 12 Apr 2024 08:34:08 +0200 Subject: [PATCH 09/31] add `#[doc(hidden)]` to the Rust module created by `#[pymodule]` (#4067) * add `#[doc(hidden)]` to the Rust module created by `#[pymodule]` * add newsfragment --- newsfragments/4067.fixed.md | 1 + pyo3-macros-backend/src/module.rs | 1 + tests/test_compile_error.rs | 1 + tests/ui/pymodule_missing_docs.rs | 12 ++++++++++++ 4 files changed, 15 insertions(+) create mode 100644 newsfragments/4067.fixed.md create mode 100644 tests/ui/pymodule_missing_docs.rs diff --git a/newsfragments/4067.fixed.md b/newsfragments/4067.fixed.md new file mode 100644 index 00000000000..869b6addf15 --- /dev/null +++ b/newsfragments/4067.fixed.md @@ -0,0 +1 @@ +fixes `missing_docs` lint to trigger on documented `#[pymodule]` functions \ No newline at end of file diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 080a279a88c..8ff720cc616 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -324,6 +324,7 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result Ok(quote! { #function + #[doc(hidden)] #vis mod #ident { #initialization } diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 38b5c4727c6..44049620598 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -53,4 +53,5 @@ fn test_compile_errors() { #[cfg(feature = "experimental-async")] #[cfg(any(not(Py_LIMITED_API), Py_3_10))] // to avoid PyFunctionArgument for &str t.compile_fail("tests/ui/invalid_cancel_handle.rs"); + t.pass("tests/ui/pymodule_missing_docs.rs"); } diff --git a/tests/ui/pymodule_missing_docs.rs b/tests/ui/pymodule_missing_docs.rs new file mode 100644 index 00000000000..1b196fa65e0 --- /dev/null +++ b/tests/ui/pymodule_missing_docs.rs @@ -0,0 +1,12 @@ +#![deny(missing_docs)] +//! Some crate docs + +use pyo3::prelude::*; + +/// Some module documentation +#[pymodule] +pub fn python_module(_m: &Bound<'_, PyModule>) -> PyResult<()> { + Ok(()) +} + +fn main() {} From ee5216f406889a9b993bfbe2e9dd977027e57075 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 12 Apr 2024 08:34:27 +0200 Subject: [PATCH 10/31] fix declarative-modules compile error (#4054) * fix declarative-modules compile error * add newsfragment --- newsfragments/4054.fixed.md | 1 + pyo3-macros-backend/src/pyclass.rs | 2 +- tests/test_declarative_module.rs | 11 +++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 newsfragments/4054.fixed.md diff --git a/newsfragments/4054.fixed.md b/newsfragments/4054.fixed.md new file mode 100644 index 00000000000..4b8da92ca4d --- /dev/null +++ b/newsfragments/4054.fixed.md @@ -0,0 +1 @@ +Fixes a compile error when exporting a `#[pyclass]` living in a different Rust module using the declarative-module feature. diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index cf8d9aae801..aff23b879f5 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1614,7 +1614,7 @@ impl<'a> PyClassImplsBuilder<'a> { quote! { impl #cls { #[doc(hidden)] - const _PYO3_DEF: #pyo3_path::impl_::pymodule::AddClassToModule = #pyo3_path::impl_::pymodule::AddClassToModule::new(); + pub const _PYO3_DEF: #pyo3_path::impl_::pymodule::AddClassToModule = #pyo3_path::impl_::pymodule::AddClassToModule::new(); } } } diff --git a/tests/test_declarative_module.rs b/tests/test_declarative_module.rs index 5f8e2c9fa55..8e432c3ae58 100644 --- a/tests/test_declarative_module.rs +++ b/tests/test_declarative_module.rs @@ -9,6 +9,13 @@ use pyo3::types::PyBool; #[path = "../src/tests/common.rs"] mod common; +mod some_module { + use pyo3::prelude::*; + + #[pyclass] + pub struct SomePyClass; +} + #[pyclass] struct ValueClass { value: usize, @@ -50,6 +57,10 @@ mod declarative_module { #[pymodule_export] use super::{declarative_module2, double, MyError, ValueClass as Value}; + // test for #4036 + #[pymodule_export] + use super::some_module::SomePyClass; + #[pymodule] mod inner { use super::*; From 30348b4d3fc433508cdc9b71ae7a973ec5e40235 Mon Sep 17 00:00:00 2001 From: messense Date: Sat, 13 Apr 2024 15:43:06 +0800 Subject: [PATCH 11/31] Link libpython for AIX target (#4073) --- newsfragments/4073.fixed.md | 1 + pyo3-build-config/src/impl_.rs | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 newsfragments/4073.fixed.md diff --git a/newsfragments/4073.fixed.md b/newsfragments/4073.fixed.md new file mode 100644 index 00000000000..0f77647e42d --- /dev/null +++ b/newsfragments/4073.fixed.md @@ -0,0 +1 @@ +fixes undefined symbol errors when building extension module on AIX by linking `libpython` diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index d5373db9655..2ee68503faa 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -775,6 +775,8 @@ pub fn is_linking_libpython() -> bool { /// Must be called from a PyO3 crate build script. fn is_linking_libpython_for_target(target: &Triple) -> bool { target.operating_system == OperatingSystem::Windows + // See https://github.com/PyO3/pyo3/issues/4068#issuecomment-2051159852 + || target.operating_system == OperatingSystem::Aix || target.environment == Environment::Android || target.environment == Environment::Androideabi || !is_extension_module() From 4e5167db4241e9c003c922b148fefd870eb0edad Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sat, 13 Apr 2024 09:57:13 +0200 Subject: [PATCH 12/31] Extend guide on interaction between method receivers and lifetime elision. (#4069) --- guide/src/class.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/guide/src/class.md b/guide/src/class.md index f353cc4787e..b5ef95cb2f7 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -998,6 +998,44 @@ impl MyClass { Note that `text_signature` on `#[new]` is not compatible with compilation in `abi3` mode until Python 3.10 or greater. +### Method receivers and lifetime elision + +PyO3 supports writing instance methods using the normal method receivers for shared `&self` and unique `&mut self` references. This interacts with [lifetime elision][lifetime-elision] insofar as the lifetime of a such a receiver is assigned to all elided output lifetime parameters. + +This is a good default for general Rust code where return values are more likely to borrow from the receiver than from the other arguments, if they contain any lifetimes at all. However, when returning bound references `Bound<'py, T>` in PyO3-based code, the GIL lifetime `'py` should usually be derived from a GIL token `py: Python<'py>` passed as an argument instead of the receiver. + +Specifically, signatures like + +```rust,ignore +fn frobnicate(&self, py: Python) -> Bound; +``` + +will not work as they are inferred as + +```rust,ignore +fn frobnicate<'a, 'py>(&'a self, py: Python<'py>) -> Bound<'a, Foo>; +``` + +instead of the intended + +```rust,ignore +fn frobnicate<'a, 'py>(&'a self, py: Python<'py>) -> Bound<'py, Foo>; +``` + +and should usually be written as + +```rust,ignore +fn frobnicate<'py>(&self, py: Python<'py>) -> Bound<'py, Foo>; +``` + +The same problem does not exist for `#[pyfunction]`s as the special case for receiver lifetimes does not apply and indeed a signature like + +```rust,ignore +fn frobnicate(bar: &Bar, py: Python) -> Bound; +``` + +will yield compiler error [E0106 "missing lifetime specifier"][compiler-error-e0106]. + ## `#[pyclass]` enums Enum support in PyO3 comes in two flavors, depending on what kind of variants the enum has: simple and complex. @@ -1329,3 +1367,6 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { [classattr]: https://docs.python.org/3/tutorial/classes.html#class-and-instance-variables [`multiple-pymethods`]: features.md#multiple-pymethods + +[lifetime-elision]: https://doc.rust-lang.org/reference/lifetime-elision.html +[compiler-error-e0106]: https://doc.rust-lang.org/error_codes/E0106.html From 721100a9e981391de784022bf564285f772314ea Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sat, 13 Apr 2024 14:59:44 +0200 Subject: [PATCH 13/31] Suppress non_local_definitions lint as we often want the non-local effects in macro code (#4074) * Resolve references to legacy numerical constants and use the associated constants instead * Suppress non_local_definitions lint as we often want the non-local effects in macro code --- pyo3-ffi/src/pyport.rs | 4 ++-- pyo3-macros-backend/src/module.rs | 2 ++ pyo3-macros-backend/src/pyfunction.rs | 1 + pyo3-macros-backend/src/pymethod.rs | 1 + src/callback.rs | 7 +------ src/conversions/std/num.rs | 28 +++++++++++++-------------- src/impl_/pyclass.rs | 1 + src/pycell/impl_.rs | 2 +- src/types/any.rs | 1 + tests/test_proto_methods.rs | 2 +- 10 files changed, 25 insertions(+), 24 deletions(-) diff --git a/pyo3-ffi/src/pyport.rs b/pyo3-ffi/src/pyport.rs index 741b0db7bf8..a144c67fb1b 100644 --- a/pyo3-ffi/src/pyport.rs +++ b/pyo3-ffi/src/pyport.rs @@ -11,8 +11,8 @@ pub type Py_ssize_t = ::libc::ssize_t; pub type Py_hash_t = Py_ssize_t; pub type Py_uhash_t = ::libc::size_t; -pub const PY_SSIZE_T_MIN: Py_ssize_t = std::isize::MIN as Py_ssize_t; -pub const PY_SSIZE_T_MAX: Py_ssize_t = std::isize::MAX as Py_ssize_t; +pub const PY_SSIZE_T_MIN: Py_ssize_t = isize::MIN as Py_ssize_t; +pub const PY_SSIZE_T_MAX: Py_ssize_t = isize::MAX as Py_ssize_t; #[cfg(target_endian = "big")] pub const PY_BIG_ENDIAN: usize = 1; diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 8ff720cc616..3153279a2b8 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -249,6 +249,7 @@ pub fn pymodule_module_impl(mut module: syn::ItemMod) -> Result { #initialization + #[allow(unknown_lints, non_local_definitions)] impl MakeDef { const fn make_def() -> #pyo3_path::impl_::pymodule::ModuleDef { use #pyo3_path::impl_::pymodule as impl_; @@ -333,6 +334,7 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result // this avoids complications around the fact that the generated module has a different scope // (and `super` doesn't always refer to the outer scope, e.g. if the `#[pymodule] is // inside a function body) + #[allow(unknown_lints, non_local_definitions)] impl #ident::MakeDef { const fn make_def() -> #pyo3_path::impl_::pymodule::ModuleDef { fn __pyo3_pymodule(module: &#pyo3_path::Bound<'_, #pyo3_path::types::PyModule>) -> #pyo3_path::PyResult<()> { diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index 9f9557a3664..7c355533b83 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -273,6 +273,7 @@ pub fn impl_wrap_pyfunction( // this avoids complications around the fact that the generated module has a different scope // (and `super` doesn't always refer to the outer scope, e.g. if the `#[pyfunction] is // inside a function body) + #[allow(unknown_lints, non_local_definitions)] impl #name::MakeDef { const _PYO3_DEF: #pyo3_path::impl_::pymethods::PyMethodDef = #methoddef; } diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index abe8c7ac8a3..7a4c54db9da 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -366,6 +366,7 @@ pub fn impl_py_method_def_new( #deprecations use #pyo3_path::impl_::pyclass::*; + #[allow(unknown_lints, non_local_definitions)] impl PyClassNewTextSignature<#cls> for PyClassImplCollector<#cls> { #[inline] fn new_text_signature(self) -> ::std::option::Option<&'static str> { diff --git a/src/callback.rs b/src/callback.rs index a56b268aa1e..1e446039904 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -4,7 +4,6 @@ use crate::err::{PyErr, PyResult}; use crate::exceptions::PyOverflowError; use crate::ffi::{self, Py_hash_t}; use crate::{IntoPy, PyObject, Python}; -use std::isize; use std::os::raw::c_int; /// A type which can be the return type of a python C-API callback @@ -85,11 +84,7 @@ impl IntoPyCallbackOutput<()> for () { impl IntoPyCallbackOutput for usize { #[inline] fn convert(self, _py: Python<'_>) -> PyResult { - if self <= (isize::MAX as usize) { - Ok(self as isize) - } else { - Err(PyOverflowError::new_err(())) - } + self.try_into().map_err(|_err| PyOverflowError::new_err(())) } } diff --git a/src/conversions/std/num.rs b/src/conversions/std/num.rs index 44843141440..e2072d210e0 100644 --- a/src/conversions/std/num.rs +++ b/src/conversions/std/num.rs @@ -445,7 +445,7 @@ mod test_128bit_integers { #[test] fn test_i128_max() { Python::with_gil(|py| { - let v = std::i128::MAX; + let v = i128::MAX; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert_eq!(v as u128, obj.extract::(py).unwrap()); @@ -456,7 +456,7 @@ mod test_128bit_integers { #[test] fn test_i128_min() { Python::with_gil(|py| { - let v = std::i128::MIN; + let v = i128::MIN; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -467,7 +467,7 @@ mod test_128bit_integers { #[test] fn test_u128_max() { Python::with_gil(|py| { - let v = std::u128::MAX; + let v = u128::MAX; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -495,7 +495,7 @@ mod test_128bit_integers { #[test] fn test_nonzero_i128_max() { Python::with_gil(|py| { - let v = NonZeroI128::new(std::i128::MAX).unwrap(); + let v = NonZeroI128::new(i128::MAX).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert_eq!( @@ -509,7 +509,7 @@ mod test_128bit_integers { #[test] fn test_nonzero_i128_min() { Python::with_gil(|py| { - let v = NonZeroI128::new(std::i128::MIN).unwrap(); + let v = NonZeroI128::new(i128::MIN).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -520,7 +520,7 @@ mod test_128bit_integers { #[test] fn test_nonzero_u128_max() { Python::with_gil(|py| { - let v = NonZeroU128::new(std::u128::MAX).unwrap(); + let v = NonZeroU128::new(u128::MAX).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -573,7 +573,7 @@ mod tests { #[test] fn test_u32_max() { Python::with_gil(|py| { - let v = std::u32::MAX; + let v = u32::MAX; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert_eq!(u64::from(v), obj.extract::(py).unwrap()); @@ -584,7 +584,7 @@ mod tests { #[test] fn test_i64_max() { Python::with_gil(|py| { - let v = std::i64::MAX; + let v = i64::MAX; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert_eq!(v as u64, obj.extract::(py).unwrap()); @@ -595,7 +595,7 @@ mod tests { #[test] fn test_i64_min() { Python::with_gil(|py| { - let v = std::i64::MIN; + let v = i64::MIN; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -606,7 +606,7 @@ mod tests { #[test] fn test_u64_max() { Python::with_gil(|py| { - let v = std::u64::MAX; + let v = u64::MAX; let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -664,7 +664,7 @@ mod tests { #[test] fn test_nonzero_u32_max() { Python::with_gil(|py| { - let v = NonZeroU32::new(std::u32::MAX).unwrap(); + let v = NonZeroU32::new(u32::MAX).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert_eq!(NonZeroU64::from(v), obj.extract::(py).unwrap()); @@ -675,7 +675,7 @@ mod tests { #[test] fn test_nonzero_i64_max() { Python::with_gil(|py| { - let v = NonZeroI64::new(std::i64::MAX).unwrap(); + let v = NonZeroI64::new(i64::MAX).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert_eq!( @@ -689,7 +689,7 @@ mod tests { #[test] fn test_nonzero_i64_min() { Python::with_gil(|py| { - let v = NonZeroI64::new(std::i64::MIN).unwrap(); + let v = NonZeroI64::new(i64::MIN).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); @@ -700,7 +700,7 @@ mod tests { #[test] fn test_nonzero_u64_max() { Python::with_gil(|py| { - let v = NonZeroU64::new(std::u64::MAX).unwrap(); + let v = NonZeroU64::new(u64::MAX).unwrap(); let obj = v.to_object(py); assert_eq!(v, obj.extract::(py).unwrap()); assert!(obj.extract::(py).is_err()); diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 1a144f736e0..1302834ca4b 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -852,6 +852,7 @@ slot_fragment_trait! { #[macro_export] macro_rules! generate_pyclass_richcompare_slot { ($cls:ty) => {{ + #[allow(unknown_lints, non_local_definitions)] impl $cls { #[allow(non_snake_case)] unsafe extern "C" fn __pymethod___richcmp____( diff --git a/src/pycell/impl_.rs b/src/pycell/impl_.rs index 378bec04993..1bdd44b9e9c 100644 --- a/src/pycell/impl_.rs +++ b/src/pycell/impl_.rs @@ -55,7 +55,7 @@ struct BorrowFlag(usize); impl BorrowFlag { pub(crate) const UNUSED: BorrowFlag = BorrowFlag(0); - const HAS_MUTABLE_BORROW: BorrowFlag = BorrowFlag(usize::max_value()); + const HAS_MUTABLE_BORROW: BorrowFlag = BorrowFlag(usize::MAX); const fn increment(self) -> Self { Self(self.0 + 1) } diff --git a/src/types/any.rs b/src/types/any.rs index ab4f5727623..a5ea4c80d4c 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -2491,6 +2491,7 @@ class SimpleClass: #[cfg(feature = "macros")] #[test] + #[allow(unknown_lints, non_local_definitions)] fn test_hasattr_error() { use crate::exceptions::PyValueError; use crate::prelude::*; diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index dd707990c0b..c5d7306086d 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -3,7 +3,7 @@ use pyo3::exceptions::{PyAttributeError, PyIndexError, PyValueError}; use pyo3::types::{PyDict, PyList, PyMapping, PySequence, PySlice, PyType}; use pyo3::{prelude::*, py_run}; -use std::{isize, iter}; +use std::iter; #[path = "../src/tests/common.rs"] mod common; From cc7e16f4d6c1c4e9a19d0b514a4ef8ece6d14776 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 14 Apr 2024 16:19:57 +0200 Subject: [PATCH 14/31] Refactoring of `FnArg` (#4033) * refactor `FnArg` * add UI tests * use enum variant types * add comment * remove dead code * remove last FIXME * review feedback davidhewitt --- pyo3-macros-backend/src/method.rs | 179 ++++++++++++++---- pyo3-macros-backend/src/params.rs | 172 +++++++---------- pyo3-macros-backend/src/pyclass.rs | 30 +-- .../src/pyfunction/signature.rs | 110 +++++++---- pyo3-macros-backend/src/pymethod.rs | 53 +++--- tests/ui/invalid_cancel_handle.rs | 6 + tests/ui/invalid_cancel_handle.stderr | 6 + tests/ui/invalid_pyfunctions.rs | 10 +- tests/ui/invalid_pyfunctions.stderr | 20 +- 9 files changed, 348 insertions(+), 238 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 6af0ec97d04..982cf62946e 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -6,7 +6,7 @@ use syn::{ext::IdentExt, spanned::Spanned, Ident, Result}; use crate::utils::Ctx; use crate::{ - attributes::{TextSignatureAttribute, TextSignatureAttributeValue}, + attributes::{FromPyWithAttribute, TextSignatureAttribute, TextSignatureAttributeValue}, deprecations::{Deprecation, Deprecations}, params::{impl_arg_params, Holders}, pyfunction::{ @@ -17,19 +17,109 @@ use crate::{ }; #[derive(Clone, Debug)] -pub struct FnArg<'a> { +pub struct RegularArg<'a> { pub name: &'a syn::Ident, pub ty: &'a syn::Type, - pub optional: Option<&'a syn::Type>, - pub default: Option, - pub py: bool, - pub attrs: PyFunctionArgPyO3Attributes, - pub is_varargs: bool, - pub is_kwargs: bool, - pub is_cancel_handle: bool, + pub from_py_with: Option, + pub default_value: Option, + pub option_wrapped_type: Option<&'a syn::Type>, +} + +/// Pythons *args argument +#[derive(Clone, Debug)] +pub struct VarargsArg<'a> { + pub name: &'a syn::Ident, + pub ty: &'a syn::Type, +} + +/// Pythons **kwarg argument +#[derive(Clone, Debug)] +pub struct KwargsArg<'a> { + pub name: &'a syn::Ident, + pub ty: &'a syn::Type, +} + +#[derive(Clone, Debug)] +pub struct CancelHandleArg<'a> { + pub name: &'a syn::Ident, + pub ty: &'a syn::Type, +} + +#[derive(Clone, Debug)] +pub struct PyArg<'a> { + pub name: &'a syn::Ident, + pub ty: &'a syn::Type, +} + +#[derive(Clone, Debug)] +pub enum FnArg<'a> { + Regular(RegularArg<'a>), + VarArgs(VarargsArg<'a>), + KwArgs(KwargsArg<'a>), + Py(PyArg<'a>), + CancelHandle(CancelHandleArg<'a>), } impl<'a> FnArg<'a> { + pub fn name(&self) -> &'a syn::Ident { + match self { + FnArg::Regular(RegularArg { name, .. }) => name, + FnArg::VarArgs(VarargsArg { name, .. }) => name, + FnArg::KwArgs(KwargsArg { name, .. }) => name, + FnArg::Py(PyArg { name, .. }) => name, + FnArg::CancelHandle(CancelHandleArg { name, .. }) => name, + } + } + + pub fn ty(&self) -> &'a syn::Type { + match self { + FnArg::Regular(RegularArg { ty, .. }) => ty, + FnArg::VarArgs(VarargsArg { ty, .. }) => ty, + FnArg::KwArgs(KwargsArg { ty, .. }) => ty, + FnArg::Py(PyArg { ty, .. }) => ty, + FnArg::CancelHandle(CancelHandleArg { ty, .. }) => ty, + } + } + + #[allow(clippy::wrong_self_convention)] + pub fn from_py_with(&self) -> Option<&FromPyWithAttribute> { + if let FnArg::Regular(RegularArg { from_py_with, .. }) = self { + from_py_with.as_ref() + } else { + None + } + } + + pub fn to_varargs_mut(&mut self) -> Result<&mut Self> { + if let Self::Regular(RegularArg { + name, + ty, + option_wrapped_type: None, + .. + }) = self + { + *self = Self::VarArgs(VarargsArg { name, ty }); + Ok(self) + } else { + bail_spanned!(self.name().span() => "args cannot be optional") + } + } + + pub fn to_kwargs_mut(&mut self) -> Result<&mut Self> { + if let Self::Regular(RegularArg { + name, + ty, + option_wrapped_type: Some(..), + .. + }) = self + { + *self = Self::KwArgs(KwargsArg { name, ty }); + Ok(self) + } else { + bail_spanned!(self.name().span() => "kwargs must be Option<_>") + } + } + /// Transforms a rust fn arg parsed with syn into a method::FnArg pub fn parse(arg: &'a mut syn::FnArg) -> Result { match arg { @@ -41,32 +131,43 @@ impl<'a> FnArg<'a> { bail_spanned!(cap.ty.span() => IMPL_TRAIT_ERR); } - let arg_attrs = PyFunctionArgPyO3Attributes::from_attrs(&mut cap.attrs)?; + let PyFunctionArgPyO3Attributes { + from_py_with, + cancel_handle, + } = PyFunctionArgPyO3Attributes::from_attrs(&mut cap.attrs)?; let ident = match &*cap.pat { syn::Pat::Ident(syn::PatIdent { ident, .. }) => ident, other => return Err(handle_argument_error(other)), }; - let is_cancel_handle = arg_attrs.cancel_handle.is_some(); + if utils::is_python(&cap.ty) { + return Ok(Self::Py(PyArg { + name: ident, + ty: &cap.ty, + })); + } - Ok(FnArg { + if cancel_handle.is_some() { + // `PyFunctionArgPyO3Attributes::from_attrs` validates that + // only compatible attributes are specified, either + // `cancel_handle` or `from_py_with`, dublicates and any + // combination of the two are already rejected. + return Ok(Self::CancelHandle(CancelHandleArg { + name: ident, + ty: &cap.ty, + })); + } + + Ok(Self::Regular(RegularArg { name: ident, ty: &cap.ty, - optional: utils::option_type_argument(&cap.ty), - default: None, - py: utils::is_python(&cap.ty), - attrs: arg_attrs, - is_varargs: false, - is_kwargs: false, - is_cancel_handle, - }) + from_py_with, + default_value: None, + option_wrapped_type: utils::option_type_argument(&cap.ty), + })) } } } - - pub fn is_regular(&self) -> bool { - !self.py && !self.is_cancel_handle && !self.is_kwargs && !self.is_varargs - } } fn handle_argument_error(pat: &syn::Pat) -> syn::Error { @@ -492,12 +593,14 @@ impl<'a> FnSpec<'a> { .signature .arguments .iter() - .filter(|arg| arg.is_cancel_handle); + .filter(|arg| matches!(arg, FnArg::CancelHandle(..))); let cancel_handle = cancel_handle_iter.next(); - if let Some(arg) = cancel_handle { - ensure_spanned!(self.asyncness.is_some(), arg.name.span() => "`cancel_handle` attribute can only be used with `async fn`"); - if let Some(arg2) = cancel_handle_iter.next() { - bail_spanned!(arg2.name.span() => "`cancel_handle` may only be specified once"); + if let Some(FnArg::CancelHandle(CancelHandleArg { name, .. })) = cancel_handle { + ensure_spanned!(self.asyncness.is_some(), name.span() => "`cancel_handle` attribute can only be used with `async fn`"); + if let Some(FnArg::CancelHandle(CancelHandleArg { name, .. })) = + cancel_handle_iter.next() + { + bail_spanned!(name.span() => "`cancel_handle` may only be specified once"); } } @@ -605,14 +708,10 @@ impl<'a> FnSpec<'a> { .signature .arguments .iter() - .map(|arg| { - if arg.py { - quote!(py) - } else if arg.is_cancel_handle { - quote!(__cancel_handle) - } else { - unreachable!() - } + .map(|arg| match arg { + FnArg::Py(..) => quote!(py), + FnArg::CancelHandle(..) => quote!(__cancel_handle), + _ => unreachable!("`CallingConvention::Noargs` should not contain any arguments (reaching Python) except for `self`, which is handled below."), }) .collect(); let call = rust_call(args, &mut holders); @@ -635,7 +734,7 @@ impl<'a> FnSpec<'a> { } CallingConvention::Fastcall => { let mut holders = Holders::new(); - let (arg_convert, args) = impl_arg_params(self, cls, true, &mut holders, ctx)?; + let (arg_convert, args) = impl_arg_params(self, cls, true, &mut holders, ctx); let call = rust_call(args, &mut holders); let init_holders = holders.init_holders(ctx); let check_gil_refs = holders.check_gil_refs(); @@ -660,7 +759,7 @@ impl<'a> FnSpec<'a> { } CallingConvention::Varargs => { let mut holders = Holders::new(); - let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx)?; + let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx); let call = rust_call(args, &mut holders); let init_holders = holders.init_holders(ctx); let check_gil_refs = holders.check_gil_refs(); @@ -684,7 +783,7 @@ impl<'a> FnSpec<'a> { } CallingConvention::TpNew => { let mut holders = Holders::new(); - let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx)?; + let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx); let self_arg = self .tp .self_arg(cls, ExtractErrorMode::Raise, &mut holders, ctx); diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index fa50d260986..d9f77fa07bc 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -1,13 +1,12 @@ use crate::utils::Ctx; use crate::{ - method::{FnArg, FnSpec}, + method::{FnArg, FnSpec, RegularArg}, pyfunction::FunctionSignature, quotes::some_wrap, }; use proc_macro2::{Span, TokenStream}; -use quote::{quote, quote_spanned}; +use quote::{format_ident, quote, quote_spanned}; use syn::spanned::Spanned; -use syn::Result; pub struct Holders { holders: Vec, @@ -60,16 +59,7 @@ impl Holders { pub fn is_forwarded_args(signature: &FunctionSignature<'_>) -> bool { matches!( signature.arguments.as_slice(), - [ - FnArg { - is_varargs: true, - .. - }, - FnArg { - is_kwargs: true, - .. - }, - ] + [FnArg::VarArgs(..), FnArg::KwArgs(..),] ) } @@ -90,7 +80,7 @@ pub fn impl_arg_params( fastcall: bool, holders: &mut Holders, ctx: &Ctx, -) -> Result<(TokenStream, Vec)> { +) -> (TokenStream, Vec) { let args_array = syn::Ident::new("output", Span::call_site()); let Ctx { pyo3_path } = ctx; @@ -100,9 +90,8 @@ pub fn impl_arg_params( .iter() .enumerate() .filter_map(|(i, arg)| { - let from_py_with = &arg.attrs.from_py_with.as_ref()?.value; - let from_py_with_holder = - syn::Ident::new(&format!("from_py_with_{}", i), Span::call_site()); + let from_py_with = &arg.from_py_with()?.value; + let from_py_with_holder = format_ident!("from_py_with_{}", i); Some(quote_spanned! { from_py_with.span() => let e = #pyo3_path::impl_::deprecations::GilRefs::new(); let #from_py_with_holder = #pyo3_path::impl_::deprecations::inspect_fn(#from_py_with, &e); @@ -119,28 +108,16 @@ pub fn impl_arg_params( .arguments .iter() .enumerate() - .map(|(i, arg)| { - let from_py_with = - syn::Ident::new(&format!("from_py_with_{}", i), Span::call_site()); - let arg_value = quote!(#args_array[0].as_deref()); - - impl_arg_param(arg, from_py_with, arg_value, holders, ctx).map(|tokens| { - check_arg_for_gil_refs( - tokens, - holders.push_gil_refs_checker(arg.ty.span()), - ctx, - ) - }) - }) - .collect::>()?; - return Ok(( + .map(|(i, arg)| impl_arg_param(arg, i, &mut 0, holders, ctx)) + .collect(); + return ( quote! { let _args = #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_args); let _kwargs = #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr_or_opt(py, &_kwargs); #from_py_with }, arg_convert, - )); + ); }; let positional_parameter_names = &spec.signature.python_signature.positional_parameters; @@ -171,18 +148,8 @@ pub fn impl_arg_params( .arguments .iter() .enumerate() - .map(|(i, arg)| { - let from_py_with = syn::Ident::new(&format!("from_py_with_{}", i), Span::call_site()); - let arg_value = quote!(#args_array[#option_pos].as_deref()); - if arg.is_regular() { - option_pos += 1; - } - - impl_arg_param(arg, from_py_with, arg_value, holders, ctx).map(|tokens| { - check_arg_for_gil_refs(tokens, holders.push_gil_refs_checker(arg.ty.span()), ctx) - }) - }) - .collect::>()?; + .map(|(i, arg)| impl_arg_param(arg, i, &mut option_pos, holders, ctx)) + .collect(); let args_handler = if spec.signature.python_signature.varargs.is_some() { quote! { #pyo3_path::impl_::extract_argument::TupleVarargs } @@ -224,7 +191,7 @@ pub fn impl_arg_params( }; // create array of arguments, and then parse - Ok(( + ( quote! { const DESCRIPTION: #pyo3_path::impl_::extract_argument::FunctionDescription = #pyo3_path::impl_::extract_argument::FunctionDescription { cls_name: #cls_name, @@ -239,18 +206,64 @@ pub fn impl_arg_params( #from_py_with }, param_conversion, - )) + ) +} + +fn impl_arg_param( + arg: &FnArg<'_>, + pos: usize, + option_pos: &mut usize, + holders: &mut Holders, + ctx: &Ctx, +) -> TokenStream { + let Ctx { pyo3_path } = ctx; + let args_array = syn::Ident::new("output", Span::call_site()); + + match arg { + FnArg::Regular(arg) => { + let from_py_with = format_ident!("from_py_with_{}", pos); + let arg_value = quote!(#args_array[#option_pos].as_deref()); + *option_pos += 1; + let tokens = impl_regular_arg_param(arg, from_py_with, arg_value, holders, ctx); + check_arg_for_gil_refs(tokens, holders.push_gil_refs_checker(arg.ty.span()), ctx) + } + FnArg::VarArgs(arg) => { + let holder = holders.push_holder(arg.name.span()); + let name_str = arg.name.to_string(); + quote_spanned! { arg.name.span() => + #pyo3_path::impl_::extract_argument::extract_argument( + &_args, + &mut #holder, + #name_str + )? + } + } + FnArg::KwArgs(arg) => { + let holder = holders.push_holder(arg.name.span()); + let name_str = arg.name.to_string(); + quote_spanned! { arg.name.span() => + #pyo3_path::impl_::extract_argument::extract_optional_argument( + _kwargs.as_deref(), + &mut #holder, + #name_str, + || ::std::option::Option::None + )? + } + } + FnArg::Py(..) => quote! { py }, + FnArg::CancelHandle(..) => quote! { __cancel_handle }, + } } /// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument /// index and the index in option diverge when using py: Python -pub(crate) fn impl_arg_param( - arg: &FnArg<'_>, +pub(crate) fn impl_regular_arg_param( + arg: &RegularArg<'_>, from_py_with: syn::Ident, arg_value: TokenStream, // expected type: Option<&'a Bound<'py, PyAny>> holders: &mut Holders, ctx: &Ctx, -) -> Result { +) -> TokenStream { let Ctx { pyo3_path } = ctx; let pyo3_path = pyo3_path.to_tokens_spanned(arg.ty.span()); @@ -260,64 +273,19 @@ pub(crate) fn impl_arg_param( ($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => $($tokens)*) } } - if arg.py { - return Ok(quote! { py }); - } - - if arg.is_cancel_handle { - return Ok(quote! { __cancel_handle }); - } - - let name = arg.name; - let name_str = name.to_string(); - - if arg.is_varargs { - ensure_spanned!( - arg.optional.is_none(), - arg.name.span() => "args cannot be optional" - ); - let holder = holders.push_holder(arg.ty.span()); - return Ok(quote_arg_span! { - #pyo3_path::impl_::extract_argument::extract_argument( - &_args, - &mut #holder, - #name_str - )? - }); - } else if arg.is_kwargs { - ensure_spanned!( - arg.optional.is_some(), - arg.name.span() => "kwargs must be Option<_>" - ); - let holder = holders.push_holder(arg.name.span()); - return Ok(quote_arg_span! { - #pyo3_path::impl_::extract_argument::extract_optional_argument( - _kwargs.as_deref(), - &mut #holder, - #name_str, - || ::std::option::Option::None - )? - }); - } - - let mut default = arg.default.as_ref().map(|expr| quote!(#expr)); + let name_str = arg.name.to_string(); + let mut default = arg.default_value.as_ref().map(|expr| quote!(#expr)); // Option arguments have special treatment: the default should be specified _without_ the // Some() wrapper. Maybe this should be changed in future?! - if arg.optional.is_some() { + if arg.option_wrapped_type.is_some() { default = Some(default.map_or_else( || quote!(::std::option::Option::None), |tokens| some_wrap(tokens, ctx), )); } - let tokens = if arg - .attrs - .from_py_with - .as_ref() - .map(|attr| &attr.value) - .is_some() - { + if arg.from_py_with.is_some() { if let Some(default) = default { quote_arg_span! { #pyo3_path::impl_::extract_argument::from_py_with_with_default( @@ -339,7 +307,7 @@ pub(crate) fn impl_arg_param( )? } } - } else if arg.optional.is_some() { + } else if arg.option_wrapped_type.is_some() { let holder = holders.push_holder(arg.name.span()); quote_arg_span! { #pyo3_path::impl_::extract_argument::extract_optional_argument( @@ -374,7 +342,5 @@ pub(crate) fn impl_arg_param( #name_str )? } - }; - - Ok(tokens) + } } diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index aff23b879f5..d9c84655b42 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -7,7 +7,7 @@ use crate::attributes::{ }; use crate::deprecations::Deprecations; use crate::konst::{ConstAttributes, ConstSpec}; -use crate::method::{FnArg, FnSpec}; +use crate::method::{FnArg, FnSpec, PyArg, RegularArg}; use crate::pyimpl::{gen_py_const, PyClassMethodsType}; use crate::pymethod::{ impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType, @@ -1143,36 +1143,22 @@ fn complex_enum_struct_variant_new<'a>( let arg_py_type: syn::Type = parse_quote!(#pyo3_path::Python<'_>); let args = { - let mut no_pyo3_attrs = vec![]; - let attrs = crate::pyfunction::PyFunctionArgPyO3Attributes::from_attrs(&mut no_pyo3_attrs)?; - let mut args = vec![ // py: Python<'_> - FnArg { + FnArg::Py(PyArg { name: &arg_py_ident, ty: &arg_py_type, - optional: None, - default: None, - py: true, - attrs: attrs.clone(), - is_varargs: false, - is_kwargs: false, - is_cancel_handle: false, - }, + }), ]; for field in &variant.fields { - args.push(FnArg { + args.push(FnArg::Regular(RegularArg { name: field.ident, ty: field.ty, - optional: None, - default: None, - py: false, - attrs: attrs.clone(), - is_varargs: false, - is_kwargs: false, - is_cancel_handle: false, - }); + from_py_with: None, + default_value: None, + option_wrapped_type: None, + })); } args }; diff --git a/pyo3-macros-backend/src/pyfunction/signature.rs b/pyo3-macros-backend/src/pyfunction/signature.rs index baf01285658..3daa79c89f5 100644 --- a/pyo3-macros-backend/src/pyfunction/signature.rs +++ b/pyo3-macros-backend/src/pyfunction/signature.rs @@ -10,7 +10,7 @@ use syn::{ use crate::{ attributes::{kw, KeywordAttribute}, - method::FnArg, + method::{FnArg, RegularArg}, }; pub struct Signature { @@ -351,36 +351,39 @@ impl<'a> FunctionSignature<'a> { let mut next_non_py_argument_checked = |name: &syn::Ident| { for fn_arg in args_iter.by_ref() { - if fn_arg.py { - // If the user incorrectly tried to include py: Python in the - // signature, give a useful error as a hint. - ensure_spanned!( - name != fn_arg.name, - name.span() => "arguments of type `Python` must not be part of the signature" - ); - // Otherwise try next argument. - continue; - } - if fn_arg.is_cancel_handle { - // If the user incorrectly tried to include cancel: CoroutineCancel in the - // signature, give a useful error as a hint. - ensure_spanned!( - name != fn_arg.name, - name.span() => "`cancel_handle` argument must not be part of the signature" - ); - // Otherwise try next argument. - continue; + match fn_arg { + crate::method::FnArg::Py(..) => { + // If the user incorrectly tried to include py: Python in the + // signature, give a useful error as a hint. + ensure_spanned!( + name != fn_arg.name(), + name.span() => "arguments of type `Python` must not be part of the signature" + ); + // Otherwise try next argument. + continue; + } + crate::method::FnArg::CancelHandle(..) => { + // If the user incorrectly tried to include cancel: CoroutineCancel in the + // signature, give a useful error as a hint. + ensure_spanned!( + name != fn_arg.name(), + name.span() => "`cancel_handle` argument must not be part of the signature" + ); + // Otherwise try next argument. + continue; + } + _ => { + ensure_spanned!( + name == fn_arg.name(), + name.span() => format!( + "expected argument from function definition `{}` but got argument `{}`", + fn_arg.name().unraw(), + name.unraw(), + ) + ); + return Ok(fn_arg); + } } - - ensure_spanned!( - name == fn_arg.name, - name.span() => format!( - "expected argument from function definition `{}` but got argument `{}`", - fn_arg.name.unraw(), - name.unraw(), - ) - ); - return Ok(fn_arg); } bail_spanned!( name.span() => "signature entry does not have a corresponding function argument" @@ -398,7 +401,15 @@ impl<'a> FunctionSignature<'a> { arg.span(), )?; if let Some((_, default)) = &arg.eq_and_default { - fn_arg.default = Some(default.clone()); + if let FnArg::Regular(arg) = fn_arg { + arg.default_value = Some(default.clone()); + } else { + unreachable!( + "`Python` and `CancelHandle` are already handled above and `*args`/`**kwargs` are \ + parsed and transformed below. Because the have to come last and are only allowed \ + once, this has to be a regular argument." + ); + } } } SignatureItem::VarargsSep(sep) => { @@ -406,12 +417,12 @@ impl<'a> FunctionSignature<'a> { } SignatureItem::Varargs(varargs) => { let fn_arg = next_non_py_argument_checked(&varargs.ident)?; - fn_arg.is_varargs = true; + fn_arg.to_varargs_mut()?; parse_state.add_varargs(&mut python_signature, varargs)?; } SignatureItem::Kwargs(kwargs) => { let fn_arg = next_non_py_argument_checked(&kwargs.ident)?; - fn_arg.is_kwargs = true; + fn_arg.to_kwargs_mut()?; parse_state.add_kwargs(&mut python_signature, kwargs)?; } SignatureItem::PosargsSep(sep) => { @@ -421,9 +432,11 @@ impl<'a> FunctionSignature<'a> { } // Ensure no non-py arguments remain - if let Some(arg) = args_iter.find(|arg| !arg.py && !arg.is_cancel_handle) { + if let Some(arg) = + args_iter.find(|arg| !matches!(arg, FnArg::Py(..) | FnArg::CancelHandle(..))) + { bail_spanned!( - attribute.kw.span() => format!("missing signature entry for argument `{}`", arg.name) + attribute.kw.span() => format!("missing signature entry for argument `{}`", arg.name()) ); } @@ -439,15 +452,20 @@ impl<'a> FunctionSignature<'a> { let mut python_signature = PythonSignature::default(); for arg in &arguments { // Python<'_> arguments don't show in Python signature - if arg.py || arg.is_cancel_handle { + if matches!(arg, FnArg::Py(..) | FnArg::CancelHandle(..)) { continue; } - if arg.optional.is_none() { + if let FnArg::Regular(RegularArg { + ty, + option_wrapped_type: None, + .. + }) = arg + { // This argument is required, all previous arguments must also have been required ensure_spanned!( python_signature.required_positional_parameters == python_signature.positional_parameters.len(), - arg.ty.span() => "required arguments after an `Option<_>` argument are ambiguous\n\ + ty.span() => "required arguments after an `Option<_>` argument are ambiguous\n\ = help: add a `#[pyo3(signature)]` annotation on this function to unambiguously specify the default values for all optional parameters" ); @@ -457,7 +475,7 @@ impl<'a> FunctionSignature<'a> { python_signature .positional_parameters - .push(arg.name.unraw().to_string()); + .push(arg.name().unraw().to_string()); } Ok(Self { @@ -469,8 +487,12 @@ impl<'a> FunctionSignature<'a> { fn default_value_for_parameter(&self, parameter: &str) -> String { let mut default = "...".to_string(); - if let Some(fn_arg) = self.arguments.iter().find(|arg| arg.name == parameter) { - if let Some(arg_default) = fn_arg.default.as_ref() { + if let Some(fn_arg) = self.arguments.iter().find(|arg| arg.name() == parameter) { + if let FnArg::Regular(RegularArg { + default_value: Some(arg_default), + .. + }) = fn_arg + { match arg_default { // literal values syn::Expr::Lit(syn::ExprLit { lit, .. }) => match lit { @@ -496,7 +518,11 @@ impl<'a> FunctionSignature<'a> { // others, unsupported yet so defaults to `...` _ => {} } - } else if fn_arg.optional.is_some() { + } else if let FnArg::Regular(RegularArg { + option_wrapped_type: Some(..), + .. + }) = fn_arg + { // functions without a `#[pyo3(signature = (...))]` option // will treat trailing `Option` arguments as having a default of `None` default = "None".to_string(); diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 7a4c54db9da..aac804316f8 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -1,8 +1,8 @@ use std::borrow::Cow; use crate::attributes::{NameAttribute, RenamingRule}; -use crate::method::{CallingConvention, ExtractErrorMode}; -use crate::params::{check_arg_for_gil_refs, impl_arg_param, Holders}; +use crate::method::{CallingConvention, ExtractErrorMode, PyArg}; +use crate::params::{check_arg_for_gil_refs, impl_regular_arg_param, Holders}; use crate::utils::Ctx; use crate::utils::PythonDoc; use crate::{ @@ -487,7 +487,7 @@ fn impl_py_class_attribute( let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); ensure_spanned!( args.is_empty(), - args[0].ty.span() => "#[classattr] can only have one argument (of type pyo3::Python)" + args[0].ty().span() => "#[classattr] can only have one argument (of type pyo3::Python)" ); let name = &spec.name; @@ -537,7 +537,7 @@ fn impl_call_setter( bail_spanned!(spec.name.span() => "setter function expected to have one argument"); } else if args.len() > 1 { bail_spanned!( - args[1].ty.span() => + args[1].ty().span() => "setter function can have at most two arguments ([pyo3::Python,] and value)" ); } @@ -607,7 +607,7 @@ pub fn impl_py_setter_def( let (_, args) = split_off_python_arg(&spec.signature.arguments); let value_arg = &args[0]; let (from_py_with, ident) = if let Some(from_py_with) = - &value_arg.attrs.from_py_with.as_ref().map(|f| &f.value) + &value_arg.from_py_with().as_ref().map(|f| &f.value) { let ident = syn::Ident::new("from_py_with", from_py_with.span()); ( @@ -622,20 +622,21 @@ pub fn impl_py_setter_def( (quote!(), syn::Ident::new("dummy", Span::call_site())) }; - let extract = impl_arg_param( - &args[0], + let arg = if let FnArg::Regular(arg) = &value_arg { + arg + } else { + bail_spanned!(value_arg.name().span() => "The #[setter] value argument can't be *args, **kwargs or `cancel_handle`."); + }; + + let tokens = impl_regular_arg_param( + arg, ident, quote!(::std::option::Option::Some(_value.into())), &mut holders, ctx, - ) - .map(|tokens| { - check_arg_for_gil_refs( - tokens, - holders.push_gil_refs_checker(value_arg.ty.span()), - ctx, - ) - })?; + ); + let extract = + check_arg_for_gil_refs(tokens, holders.push_gil_refs_checker(arg.ty.span()), ctx); quote! { #from_py_with let _val = #extract; @@ -721,7 +722,7 @@ fn impl_call_getter( let slf = self_type.receiver(cls, ExtractErrorMode::Raise, holders, ctx); ensure_spanned!( args.is_empty(), - args[0].ty.span() => "getter function can only have one argument (of type pyo3::Python)" + args[0].ty().span() => "getter function can only have one argument (of type pyo3::Python)" ); let name = &spec.name; @@ -843,9 +844,9 @@ pub fn impl_py_getter_def( } /// Split an argument of pyo3::Python from the front of the arg list, if present -fn split_off_python_arg<'a>(args: &'a [FnArg<'a>]) -> (Option<&FnArg<'_>>, &[FnArg<'_>]) { +fn split_off_python_arg<'a>(args: &'a [FnArg<'a>]) -> (Option<&PyArg<'_>>, &[FnArg<'_>]) { match args { - [py, args @ ..] if utils::is_python(py.ty) => (Some(py), args), + [FnArg::Py(py), args @ ..] => (Some(py), args), args => (None, args), } } @@ -1052,14 +1053,14 @@ impl Ty { ctx: &Ctx, ) -> TokenStream { let Ctx { pyo3_path } = ctx; - let name_str = arg.name.unraw().to_string(); + let name_str = arg.name().unraw().to_string(); match self { Ty::Object => extract_object( extract_error_mode, holders, &name_str, quote! { #ident }, - arg.ty.span(), + arg.ty().span(), ctx ), Ty::MaybeNullObject => extract_object( @@ -1073,7 +1074,7 @@ impl Ty { #ident } }, - arg.ty.span(), + arg.ty().span(), ctx ), Ty::NonNullObject => extract_object( @@ -1081,7 +1082,7 @@ impl Ty { holders, &name_str, quote! { #ident.as_ptr() }, - arg.ty.span(), + arg.ty().span(), ctx ), Ty::IPowModulo => extract_object( @@ -1089,7 +1090,7 @@ impl Ty { holders, &name_str, quote! { #ident.as_ptr() }, - arg.ty.span(), + arg.ty().span(), ctx ), Ty::CompareOp => extract_error_mode.handle_error( @@ -1100,7 +1101,7 @@ impl Ty { ctx ), Ty::PySsizeT => { - let ty = arg.ty; + let ty = arg.ty(); extract_error_mode.handle_error( quote! { ::std::convert::TryInto::<#ty>::try_into(#ident).map_err(|e| #pyo3_path::exceptions::PyValueError::new_err(e.to_string())) @@ -1523,12 +1524,12 @@ fn extract_proto_arguments( let mut non_python_args = 0; for arg in &spec.signature.arguments { - if arg.py { + if let FnArg::Py(..) = arg { args.push(quote! { py }); } else { let ident = syn::Ident::new(&format!("arg{}", non_python_args), Span::call_site()); let conversions = proto_args.get(non_python_args) - .ok_or_else(|| err_spanned!(arg.ty.span() => format!("Expected at most {} non-python arguments", proto_args.len())))? + .ok_or_else(|| err_spanned!(arg.ty().span() => format!("Expected at most {} non-python arguments", proto_args.len())))? .extract(&ident, arg, extract_error_mode, holders, ctx); non_python_args += 1; args.push(conversions); diff --git a/tests/ui/invalid_cancel_handle.rs b/tests/ui/invalid_cancel_handle.rs index 59076b14418..cff6c5dcbad 100644 --- a/tests/ui/invalid_cancel_handle.rs +++ b/tests/ui/invalid_cancel_handle.rs @@ -19,4 +19,10 @@ async fn cancel_handle_wrong_type(#[pyo3(cancel_handle)] _param: String) {} #[pyfunction] async fn missing_cancel_handle_attribute(_param: pyo3::coroutine::CancelHandle) {} +#[pyfunction] +async fn cancel_handle_and_from_py_with( + #[pyo3(cancel_handle, from_py_with = "cancel_handle")] _param: pyo3::coroutine::CancelHandle, +) { +} + fn main() {} diff --git a/tests/ui/invalid_cancel_handle.stderr b/tests/ui/invalid_cancel_handle.stderr index ffd0b3fd0da..41a2c0854b7 100644 --- a/tests/ui/invalid_cancel_handle.stderr +++ b/tests/ui/invalid_cancel_handle.stderr @@ -16,6 +16,12 @@ error: `cancel_handle` attribute can only be used with `async fn` 14 | fn cancel_handle_synchronous(#[pyo3(cancel_handle)] _param: String) {} | ^^^^^^ +error: `from_py_with` and `cancel_handle` cannot be specified together + --> tests/ui/invalid_cancel_handle.rs:24:12 + | +24 | #[pyo3(cancel_handle, from_py_with = "cancel_handle")] _param: pyo3::coroutine::CancelHandle, + | ^^^^^^^^^^^^^ + error[E0308]: mismatched types --> tests/ui/invalid_cancel_handle.rs:16:1 | diff --git a/tests/ui/invalid_pyfunctions.rs b/tests/ui/invalid_pyfunctions.rs index eaa241c074b..1a95c9e4a34 100644 --- a/tests/ui/invalid_pyfunctions.rs +++ b/tests/ui/invalid_pyfunctions.rs @@ -1,5 +1,5 @@ use pyo3::prelude::*; -use pyo3::types::PyString; +use pyo3::types::{PyDict, PyString, PyTuple}; #[pyfunction] fn generic_function(value: T) {} @@ -16,6 +16,14 @@ fn destructured_argument((a, b): (i32, i32)) {} #[pyfunction] fn function_with_required_after_option(_opt: Option, _x: i32) {} +#[pyfunction] +#[pyo3(signature=(*args))] +fn function_with_optional_args(args: Option>) {} + +#[pyfunction] +#[pyo3(signature=(**kwargs))] +fn function_with_required_kwargs(kwargs: Bound<'_, PyDict>) {} + #[pyfunction(pass_module)] fn pass_module_but_no_arguments<'py>() {} diff --git a/tests/ui/invalid_pyfunctions.stderr b/tests/ui/invalid_pyfunctions.stderr index 299b20687cb..893d7cbec2c 100644 --- a/tests/ui/invalid_pyfunctions.stderr +++ b/tests/ui/invalid_pyfunctions.stderr @@ -29,16 +29,28 @@ error: required arguments after an `Option<_>` argument are ambiguous 17 | fn function_with_required_after_option(_opt: Option, _x: i32) {} | ^^^ +error: args cannot be optional + --> tests/ui/invalid_pyfunctions.rs:21:32 + | +21 | fn function_with_optional_args(args: Option>) {} + | ^^^^ + +error: kwargs must be Option<_> + --> tests/ui/invalid_pyfunctions.rs:25:34 + | +25 | fn function_with_required_kwargs(kwargs: Bound<'_, PyDict>) {} + | ^^^^^^ + error: expected `&PyModule` or `Py` as first argument with `pass_module` - --> tests/ui/invalid_pyfunctions.rs:20:37 + --> tests/ui/invalid_pyfunctions.rs:28:37 | -20 | fn pass_module_but_no_arguments<'py>() {} +28 | fn pass_module_but_no_arguments<'py>() {} | ^^ error[E0277]: the trait bound `&str: From>` is not satisfied - --> tests/ui/invalid_pyfunctions.rs:24:13 + --> tests/ui/invalid_pyfunctions.rs:32:13 | -24 | string: &str, +32 | string: &str, | ^ the trait `From>` is not implemented for `&str`, which is required by `BoundRef<'_, '_, pyo3::prelude::PyModule>: Into<_>` | = help: the following other types implement trait `From`: From 8ed5c17b9311c53d94c8fd21f2763fc602979779 Mon Sep 17 00:00:00 2001 From: David Matos Date: Sun, 14 Apr 2024 22:07:17 +0200 Subject: [PATCH 15/31] Allow use of scientific notation on rust decimal (#4079) * Allow use of scientific notation on rust decimal * Add newsfragment * Pull out var * Fix clippy warning * Modify let bindings location --- newsfragments/4079.added.md | 1 + src/conversions/rust_decimal.rs | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 newsfragments/4079.added.md diff --git a/newsfragments/4079.added.md b/newsfragments/4079.added.md new file mode 100644 index 00000000000..afe26728f9a --- /dev/null +++ b/newsfragments/4079.added.md @@ -0,0 +1 @@ +Added support for scientific notation in `Decimal` conversion diff --git a/src/conversions/rust_decimal.rs b/src/conversions/rust_decimal.rs index b75cd875128..782ca2e80f0 100644 --- a/src/conversions/rust_decimal.rs +++ b/src/conversions/rust_decimal.rs @@ -64,8 +64,11 @@ impl FromPyObject<'_> for Decimal { if let Ok(val) = obj.extract() { Ok(Decimal::new(val, 0)) } else { - Decimal::from_str(&obj.str()?.to_cow()?) - .map_err(|e| PyValueError::new_err(e.to_string())) + let py_str = &obj.str()?; + let rs_str = &py_str.to_cow()?; + Decimal::from_str(rs_str).or_else(|_| { + Decimal::from_scientific(rs_str).map_err(|e| PyValueError::new_err(e.to_string())) + }) } } } @@ -194,6 +197,23 @@ mod test_rust_decimal { }) } + #[test] + fn test_scientific_notation() { + Python::with_gil(|py| { + let locals = PyDict::new_bound(py); + py.run_bound( + "import decimal\npy_dec = decimal.Decimal(\"1e3\")", + None, + Some(&locals), + ) + .unwrap(); + let py_dec = locals.get_item("py_dec").unwrap().unwrap(); + let roundtripped: Decimal = py_dec.extract().unwrap(); + let rs_dec = Decimal::from_scientific("1e3").unwrap(); + assert_eq!(rs_dec, roundtripped); + }) + } + #[test] fn test_infinity() { Python::with_gil(|py| { From a5201c04afc73605ac3ef467a42d3052af5feb14 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sun, 14 Apr 2024 22:38:40 +0100 Subject: [PATCH 16/31] Deprecate the `PySet::empty` gil-ref constructor (#4082) * Deprecate the `PySet::empty` gil-ref constructor * add newsfragment --- newsfragments/4082.changed.md | 1 + src/types/set.rs | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 newsfragments/4082.changed.md diff --git a/newsfragments/4082.changed.md b/newsfragments/4082.changed.md new file mode 100644 index 00000000000..231ea0ae576 --- /dev/null +++ b/newsfragments/4082.changed.md @@ -0,0 +1 @@ +Deprecate the `PySet::empty()` gil-ref constructor. diff --git a/src/types/set.rs b/src/types/set.rs index 4f1fcf8499f..f648bc2be1f 100644 --- a/src/types/set.rs +++ b/src/types/set.rs @@ -58,7 +58,14 @@ impl PySet { } /// Deprecated form of [`PySet::empty_bound`]. - pub fn empty(py: Python<'_>) -> PyResult<&'_ PySet> { + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.2", + note = "`PySet::empty` will be replaced by `PySet::empty_bound` in a future PyO3 version" + ) + )] + pub fn empty(py: Python<'_>) -> PyResult<&PySet> { Self::empty_bound(py).map(Bound::into_gil_ref) } From 2ad2a3f208a92d3a6e11909b621edd8d87b356e4 Mon Sep 17 00:00:00 2001 From: David Matos Date: Tue, 16 Apr 2024 10:17:41 +0200 Subject: [PATCH 17/31] docs: Make contributing.md slightly more clear for newer contributors (#4080) * docs: Make contributing.md slightly more clear for newer contributors * Remove accidental backticks Rearrange overview of commands * Placed on wrong line * Add extra overview command --- .github/pull_request_template.md | 2 +- Contributing.md | 22 +++++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index b344525cabe..11375e966b3 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -8,6 +8,6 @@ Please consider adding the following to your pull request: - docs to all new functions and / or detail in the guide - tests for all new or changed functions -PyO3's CI pipeline will check your pull request. To run its tests +PyO3's CI pipeline will check your pull request, thus make sure you have checked the `Contributing.md` guidelines. To run most of its tests locally, you can run ```nox```. See ```nox --list-sessions``` for a list of supported actions. diff --git a/Contributing.md b/Contributing.md index 054099b431a..111e814ac8f 100644 --- a/Contributing.md +++ b/Contributing.md @@ -92,9 +92,7 @@ Here are a few things to note when you are writing PRs. ### Continuous Integration -The PyO3 repo uses GitHub Actions. PRs are blocked from merging if CI is not successful. - -Formatting, linting and tests are checked for all Rust and Python code. In addition, all warnings in Rust code are disallowed (using `RUSTFLAGS="-D warnings"`). +The PyO3 repo uses GitHub Actions. PRs are blocked from merging if CI is not successful. Formatting, linting and tests are checked for all Rust and Python code. In addition, all warnings in Rust code are disallowed (using `RUSTFLAGS="-D warnings"`). Tests run with all supported Python versions with the latest stable Rust compiler, as well as for Python 3.9 with the minimum supported Rust version. @@ -103,6 +101,24 @@ If you are adding a new feature, you should add it to the `full` feature in our You can run these tests yourself with `nox`. Use `nox -l` to list the full set of subcommands you can run. +#### Linting Python code +`nox -s ruff` + +#### Linting Rust code +`nox -s rustfmt` + +#### Semver checks +`cargo semver-checks check-release` + +#### Clippy +`nox -s clippy-all` + +#### Tests +`cargo test --features full` + +#### Check all conditional compilation +`nox -s check-feature-powerset` + #### UI Tests PyO3 uses [`trybuild`][trybuild] to develop UI tests to capture error messages from the Rust compiler for some of the macro functionality. From 03f59eaf45c5786e7a9c591b8f81b1243a50b5ef Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Tue, 16 Apr 2024 22:12:18 +0200 Subject: [PATCH 18/31] fix declarative module compile error with `create_exception!` (#4086) * fix declarative module compile error with `create_exception!` * add newsfragment --- newsfragments/4086.fixed.md | 1 + src/types/mod.rs | 2 +- tests/test_declarative_module.rs | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 newsfragments/4086.fixed.md diff --git a/newsfragments/4086.fixed.md b/newsfragments/4086.fixed.md new file mode 100644 index 00000000000..e9cae7733f9 --- /dev/null +++ b/newsfragments/4086.fixed.md @@ -0,0 +1 @@ +Fixes a compile error when exporting an exception created with `create_exception!` living in a different Rust module using the `declarative-module` feature. \ No newline at end of file diff --git a/src/types/mod.rs b/src/types/mod.rs index a03d01b301a..e7aead7fbe5 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -252,7 +252,7 @@ macro_rules! pyobject_native_type_info( impl $name { #[doc(hidden)] - const _PYO3_DEF: $crate::impl_::pymodule::AddTypeToModule = $crate::impl_::pymodule::AddTypeToModule::new(); + pub const _PYO3_DEF: $crate::impl_::pymodule::AddTypeToModule = $crate::impl_::pymodule::AddTypeToModule::new(); } }; ); diff --git a/tests/test_declarative_module.rs b/tests/test_declarative_module.rs index 8e432c3ae58..2e46f4a64d1 100644 --- a/tests/test_declarative_module.rs +++ b/tests/test_declarative_module.rs @@ -10,10 +10,14 @@ use pyo3::types::PyBool; mod common; mod some_module { + use pyo3::create_exception; + use pyo3::exceptions::PyException; use pyo3::prelude::*; #[pyclass] pub struct SomePyClass; + + create_exception!(some_module, SomeException, PyException); } #[pyclass] @@ -61,6 +65,10 @@ mod declarative_module { #[pymodule_export] use super::some_module::SomePyClass; + // test for #4036 + #[pymodule_export] + use super::some_module::SomeException; + #[pymodule] mod inner { use super::*; From 9761abf3a543b3e3abfa0c41613331103ad79e13 Mon Sep 17 00:00:00 2001 From: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com> Date: Wed, 17 Apr 2024 03:07:11 +0200 Subject: [PATCH 19/31] Specify higher target-lexicon version (#4087) --- pyo3-build-config/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyo3-build-config/Cargo.toml b/pyo3-build-config/Cargo.toml index 1eb269c2132..a0942831340 100644 --- a/pyo3-build-config/Cargo.toml +++ b/pyo3-build-config/Cargo.toml @@ -13,11 +13,11 @@ edition = "2021" [dependencies] once_cell = "1" python3-dll-a = { version = "0.2.6", optional = true } -target-lexicon = "0.12" +target-lexicon = "0.12.14" [build-dependencies] python3-dll-a = { version = "0.2.6", optional = true } -target-lexicon = "0.12" +target-lexicon = "0.12.14" [features] default = [] From 03c50a18392cdb183aa059d65d74dbca9fdc3ec3 Mon Sep 17 00:00:00 2001 From: Jacob Zhong Date: Thu, 18 Apr 2024 15:33:07 +0800 Subject: [PATCH 20/31] Change the types of `PySliceIndices` and `PySlice::indices (#3761) * Change the type of `PySliceIndices::slicelength` and `PySlice::indices()` * Fix example * Fix fmt --- examples/getitem/src/lib.rs | 5 ++--- newsfragments/3761.changed.md | 1 + src/types/slice.rs | 23 +++++++++++++---------- 3 files changed, 16 insertions(+), 13 deletions(-) create mode 100644 newsfragments/3761.changed.md diff --git a/examples/getitem/src/lib.rs b/examples/getitem/src/lib.rs index c3c662ab92f..ce162a70bf9 100644 --- a/examples/getitem/src/lib.rs +++ b/examples/getitem/src/lib.rs @@ -2,7 +2,6 @@ use pyo3::exceptions::PyTypeError; use pyo3::prelude::*; use pyo3::types::PySlice; -use std::os::raw::c_long; #[derive(FromPyObject)] enum IntOrSlice<'py> { @@ -29,7 +28,7 @@ impl ExampleContainer { } else if let Ok(slice) = key.downcast::() { // METHOD 1 - the use PySliceIndices to help with bounds checking and for cases when only start or end are provided // in this case the start/stop/step all filled in to give valid values based on the max_length given - let index = slice.indices(self.max_length as c_long).unwrap(); + let index = slice.indices(self.max_length as isize).unwrap(); let _delta = index.stop - index.start; // METHOD 2 - Do the getattr manually really only needed if you have some special cases for stop/_step not being present @@ -62,7 +61,7 @@ impl ExampleContainer { fn __setitem__(&self, idx: IntOrSlice, value: u32) -> PyResult<()> { match idx { IntOrSlice::Slice(slice) => { - let index = slice.indices(self.max_length as c_long).unwrap(); + let index = slice.indices(self.max_length as isize).unwrap(); println!( "Got a slice! {}-{}, step: {}, value: {}", index.start, index.stop, index.step, value diff --git a/newsfragments/3761.changed.md b/newsfragments/3761.changed.md new file mode 100644 index 00000000000..fd0847211d8 --- /dev/null +++ b/newsfragments/3761.changed.md @@ -0,0 +1 @@ +Change the type of `PySliceIndices::slicelength` and the `length` parameter of `PySlice::indices()`. diff --git a/src/types/slice.rs b/src/types/slice.rs index 8e7545208dc..b6895d09e10 100644 --- a/src/types/slice.rs +++ b/src/types/slice.rs @@ -1,13 +1,12 @@ use crate::err::{PyErr, PyResult}; -use crate::ffi::{self, Py_ssize_t}; +use crate::ffi; use crate::ffi_ptr_ext::FfiPtrExt; use crate::types::any::PyAnyMethods; use crate::{Bound, PyAny, PyNativeType, PyObject, Python, ToPyObject}; -use std::os::raw::c_long; /// Represents a Python `slice`. /// -/// Only `c_long` indices supported at the moment by the `PySlice` object. +/// Only `isize` indices supported at the moment by the `PySlice` object. #[repr(transparent)] pub struct PySlice(PyAny); @@ -22,13 +21,17 @@ pyobject_native_type!( #[derive(Debug, Eq, PartialEq)] pub struct PySliceIndices { /// Start of the slice + /// + /// It can be -1 when the step is negative, otherwise it's non-negative. pub start: isize, /// End of the slice + /// + /// It can be -1 when the step is negative, otherwise it's non-negative. pub stop: isize, /// Increment to use when iterating the slice from `start` to `stop`. pub step: isize, /// The length of the slice calculated from the original input sequence. - pub slicelength: isize, + pub slicelength: usize, } impl PySliceIndices { @@ -94,7 +97,7 @@ impl PySlice { /// assuming a sequence of length `length`, and stores the length of the /// slice in its `slicelength` member. #[inline] - pub fn indices(&self, length: c_long) -> PyResult { + pub fn indices(&self, length: isize) -> PyResult { self.as_borrowed().indices(length) } } @@ -109,12 +112,11 @@ pub trait PySliceMethods<'py>: crate::sealed::Sealed { /// Retrieves the start, stop, and step indices from the slice object, /// assuming a sequence of length `length`, and stores the length of the /// slice in its `slicelength` member. - fn indices(&self, length: c_long) -> PyResult; + fn indices(&self, length: isize) -> PyResult; } impl<'py> PySliceMethods<'py> for Bound<'py, PySlice> { - fn indices(&self, length: c_long) -> PyResult { - // non-negative Py_ssize_t should always fit into Rust usize + fn indices(&self, length: isize) -> PyResult { unsafe { let mut slicelength: isize = 0; let mut start: isize = 0; @@ -122,7 +124,7 @@ impl<'py> PySliceMethods<'py> for Bound<'py, PySlice> { let mut step: isize = 0; let r = ffi::PySlice_GetIndicesEx( self.as_ptr(), - length as Py_ssize_t, + length, &mut start, &mut stop, &mut step, @@ -133,7 +135,8 @@ impl<'py> PySliceMethods<'py> for Bound<'py, PySlice> { start, stop, step, - slicelength, + // non-negative isize should always fit into usize + slicelength: slicelength as _, }) } else { Err(PyErr::fetch(self.py())) From e64eb7290338352e6344545bfc836d186f315c6e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 18 Apr 2024 08:58:51 +0000 Subject: [PATCH 21/31] build(deps): update chrono-tz requirement from >= 0.6, < 0.9 to >= 0.6, < 0.10 (#4061) * build(deps): update chrono-tz requirement from >= 0.6, < 0.9 to >= 0.6, < 0.10 Updates the requirements on [chrono-tz](https://github.com/chronotope/chrono-tz) to permit the latest version. - [Release notes](https://github.com/chronotope/chrono-tz/releases) - [Changelog](https://github.com/chronotope/chrono-tz/blob/main/CHANGELOG.md) - [Commits](https://github.com/chronotope/chrono-tz/compare/v0.6.0...v0.9.0) --- updated-dependencies: - dependency-name: chrono-tz dependency-type: direct:production ... Signed-off-by: dependabot[bot] * newsfragment --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: David Hewitt --- Cargo.toml | 4 ++-- newsfragments/4061.packaging.md | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 newsfragments/4061.packaging.md diff --git a/Cargo.toml b/Cargo.toml index fdd1fa9d29a..789d52c6356 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,7 @@ inventory = { version = "0.3.0", optional = true } # crate integrations that can be added using the eponymous features anyhow = { version = "1.0", optional = true } chrono = { version = "0.4.25", default-features = false, optional = true } -chrono-tz = { version = ">= 0.6, < 0.9", default-features = false, optional = true } +chrono-tz = { version = ">= 0.6, < 0.10", default-features = false, optional = true } either = { version = "1.9", optional = true } eyre = { version = ">= 0.4, < 0.7", optional = true } hashbrown = { version = ">= 0.9, < 0.15", optional = true } @@ -49,7 +49,7 @@ smallvec = { version = "1.0", optional = true } [dev-dependencies] assert_approx_eq = "1.1.0" chrono = "0.4.25" -chrono-tz = ">= 0.6, < 0.9" +chrono-tz = ">= 0.6, < 0.10" # Required for "and $N others" normalization trybuild = ">=1.0.70" proptest = { version = "1.0", default-features = false, features = ["std"] } diff --git a/newsfragments/4061.packaging.md b/newsfragments/4061.packaging.md new file mode 100644 index 00000000000..5e51f50290d --- /dev/null +++ b/newsfragments/4061.packaging.md @@ -0,0 +1 @@ +Extend range of supported versions of `chrono-tz` optional dependency to include version 0.10. From 2c205d4586bef8f839eb6a55eb91b905074ddce9 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 18 Apr 2024 09:59:02 +0100 Subject: [PATCH 22/31] release notes for 0.21.2 (#4091) --- CHANGELOG.md | 17 ++++++++++++++++- Cargo.toml | 8 ++++---- README.md | 4 ++-- examples/decorator/.template/pre-script.rhai | 2 +- .../maturin-starter/.template/pre-script.rhai | 2 +- examples/plugin/.template/pre-script.rhai | 2 +- .../.template/pre-script.rhai | 2 +- examples/word-count/.template/pre-script.rhai | 2 +- newsfragments/4035.fixed.md | 1 - newsfragments/4045.fixed.md | 1 - newsfragments/4054.fixed.md | 1 - newsfragments/4067.fixed.md | 1 - newsfragments/4073.fixed.md | 1 - newsfragments/4082.changed.md | 1 - pyo3-build-config/Cargo.toml | 2 +- pyo3-ffi/Cargo.toml | 4 ++-- pyo3-macros-backend/Cargo.toml | 4 ++-- pyo3-macros/Cargo.toml | 4 ++-- pyproject.toml | 2 +- 19 files changed, 35 insertions(+), 26 deletions(-) delete mode 100644 newsfragments/4035.fixed.md delete mode 100644 newsfragments/4045.fixed.md delete mode 100644 newsfragments/4054.fixed.md delete mode 100644 newsfragments/4067.fixed.md delete mode 100644 newsfragments/4073.fixed.md delete mode 100644 newsfragments/4082.changed.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f4ce218021..86055a9a80c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,20 @@ To see unreleased changes, please see the [CHANGELOG on the main branch guide](h +## [0.21.2] - 2024-04-16 + +### Changed + +- Deprecate the `PySet::empty()` gil-ref constructor. [#4082](https://github.com/PyO3/pyo3/pull/4082) + +### Fixed + +- Fix compile error for `async fn` in `#[pymethods]` with a `&self` receiver and more than one additional argument. [#4035](https://github.com/PyO3/pyo3/pull/4035) +- Improve error message for wrong receiver type in `__traverse__`. [#4045](https://github.com/PyO3/pyo3/pull/4045) +- Fix compile error when exporting a `#[pyclass]` living in a different Rust module using the `experimental-declarative-modules` feature. [#4054](https://github.com/PyO3/pyo3/pull/4054) +- Fix `missing_docs` lint triggering on documented `#[pymodule]` functions. [#4067](https://github.com/PyO3/pyo3/pull/4067) +- Fix undefined symbol errors for extension modules on AIX (by linking `libpython`). [#4073](https://github.com/PyO3/pyo3/pull/4073) + ## [0.21.1] - 2024-04-01 ### Added @@ -1731,7 +1745,8 @@ Yanked - Initial release -[Unreleased]: https://github.com/pyo3/pyo3/compare/v0.21.1...HEAD +[Unreleased]: https://github.com/pyo3/pyo3/compare/v0.21.2...HEAD +[0.21.2]: https://github.com/pyo3/pyo3/compare/v0.21.1...v0.21.2 [0.21.1]: https://github.com/pyo3/pyo3/compare/v0.21.0...v0.21.1 [0.21.0]: https://github.com/pyo3/pyo3/compare/v0.20.3...v0.21.0 [0.21.0-beta.0]: https://github.com/pyo3/pyo3/compare/v0.20.3...v0.21.0-beta.0 diff --git a/Cargo.toml b/Cargo.toml index 789d52c6356..90bcc03c80c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3" -version = "0.21.1" +version = "0.21.2" description = "Bindings to Python interpreter" authors = ["PyO3 Project and Contributors "] readme = "README.md" @@ -22,10 +22,10 @@ memoffset = "0.9" portable-atomic = "1.0" # ffi bindings to the python interpreter, split into a separate crate so they can be used independently -pyo3-ffi = { path = "pyo3-ffi", version = "=0.21.1" } +pyo3-ffi = { path = "pyo3-ffi", version = "=0.21.2" } # support crates for macros feature -pyo3-macros = { path = "pyo3-macros", version = "=0.21.1", optional = true } +pyo3-macros = { path = "pyo3-macros", version = "=0.21.2", optional = true } indoc = { version = "2.0.1", optional = true } unindent = { version = "0.2.1", optional = true } @@ -60,7 +60,7 @@ rayon = "1.6.1" futures = "0.3.28" [build-dependencies] -pyo3-build-config = { path = "pyo3-build-config", version = "=0.21.1", features = ["resolve-config"] } +pyo3-build-config = { path = "pyo3-build-config", version = "=0.21.2", features = ["resolve-config"] } [features] default = ["macros"] diff --git a/README.md b/README.md index 8e7e2f75bca..4da2447e8c4 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ name = "string_sum" crate-type = ["cdylib"] [dependencies] -pyo3 = { version = "0.21.1", features = ["extension-module"] } +pyo3 = { version = "0.21.2", features = ["extension-module"] } ``` **`src/lib.rs`** @@ -137,7 +137,7 @@ Start a new project with `cargo new` and add `pyo3` to the `Cargo.toml` like th ```toml [dependencies.pyo3] -version = "0.21.1" +version = "0.21.2" features = ["auto-initialize"] ``` diff --git a/examples/decorator/.template/pre-script.rhai b/examples/decorator/.template/pre-script.rhai index 1dc689ef6d4..37372854cd8 100644 --- a/examples/decorator/.template/pre-script.rhai +++ b/examples/decorator/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.1"); +variable::set("PYO3_VERSION", "0.21.2"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/pyproject.toml", "pyproject.toml"); file::delete(".template"); diff --git a/examples/maturin-starter/.template/pre-script.rhai b/examples/maturin-starter/.template/pre-script.rhai index 1dc689ef6d4..37372854cd8 100644 --- a/examples/maturin-starter/.template/pre-script.rhai +++ b/examples/maturin-starter/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.1"); +variable::set("PYO3_VERSION", "0.21.2"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/pyproject.toml", "pyproject.toml"); file::delete(".template"); diff --git a/examples/plugin/.template/pre-script.rhai b/examples/plugin/.template/pre-script.rhai index 78adb883f43..7c2f375fbfb 100644 --- a/examples/plugin/.template/pre-script.rhai +++ b/examples/plugin/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.1"); +variable::set("PYO3_VERSION", "0.21.2"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/plugin_api/Cargo.toml", "plugin_api/Cargo.toml"); file::delete(".template"); diff --git a/examples/setuptools-rust-starter/.template/pre-script.rhai b/examples/setuptools-rust-starter/.template/pre-script.rhai index 212f62f76fe..dd2950665eb 100644 --- a/examples/setuptools-rust-starter/.template/pre-script.rhai +++ b/examples/setuptools-rust-starter/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.1"); +variable::set("PYO3_VERSION", "0.21.2"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/setup.cfg", "setup.cfg"); file::delete(".template"); diff --git a/examples/word-count/.template/pre-script.rhai b/examples/word-count/.template/pre-script.rhai index 1dc689ef6d4..37372854cd8 100644 --- a/examples/word-count/.template/pre-script.rhai +++ b/examples/word-count/.template/pre-script.rhai @@ -1,4 +1,4 @@ -variable::set("PYO3_VERSION", "0.21.1"); +variable::set("PYO3_VERSION", "0.21.2"); file::rename(".template/Cargo.toml", "Cargo.toml"); file::rename(".template/pyproject.toml", "pyproject.toml"); file::delete(".template"); diff --git a/newsfragments/4035.fixed.md b/newsfragments/4035.fixed.md deleted file mode 100644 index 5425c5cbaf7..00000000000 --- a/newsfragments/4035.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Fix compile error for `async fn` in `#[pymethods]` with a `&self` receiver and more than one additional argument. diff --git a/newsfragments/4045.fixed.md b/newsfragments/4045.fixed.md deleted file mode 100644 index 6b2bbcfa01d..00000000000 --- a/newsfragments/4045.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Add better error message on wrong receiver extraction in `__traverse__`. diff --git a/newsfragments/4054.fixed.md b/newsfragments/4054.fixed.md deleted file mode 100644 index 4b8da92ca4d..00000000000 --- a/newsfragments/4054.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Fixes a compile error when exporting a `#[pyclass]` living in a different Rust module using the declarative-module feature. diff --git a/newsfragments/4067.fixed.md b/newsfragments/4067.fixed.md deleted file mode 100644 index 869b6addf15..00000000000 --- a/newsfragments/4067.fixed.md +++ /dev/null @@ -1 +0,0 @@ -fixes `missing_docs` lint to trigger on documented `#[pymodule]` functions \ No newline at end of file diff --git a/newsfragments/4073.fixed.md b/newsfragments/4073.fixed.md deleted file mode 100644 index 0f77647e42d..00000000000 --- a/newsfragments/4073.fixed.md +++ /dev/null @@ -1 +0,0 @@ -fixes undefined symbol errors when building extension module on AIX by linking `libpython` diff --git a/newsfragments/4082.changed.md b/newsfragments/4082.changed.md deleted file mode 100644 index 231ea0ae576..00000000000 --- a/newsfragments/4082.changed.md +++ /dev/null @@ -1 +0,0 @@ -Deprecate the `PySet::empty()` gil-ref constructor. diff --git a/pyo3-build-config/Cargo.toml b/pyo3-build-config/Cargo.toml index a0942831340..60bf9a13ef9 100644 --- a/pyo3-build-config/Cargo.toml +++ b/pyo3-build-config/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-build-config" -version = "0.21.1" +version = "0.21.2" description = "Build configuration for the PyO3 ecosystem" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] diff --git a/pyo3-ffi/Cargo.toml b/pyo3-ffi/Cargo.toml index 64753976fab..8f7767254f1 100644 --- a/pyo3-ffi/Cargo.toml +++ b/pyo3-ffi/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-ffi" -version = "0.21.1" +version = "0.21.2" description = "Python-API bindings for the PyO3 ecosystem" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -38,7 +38,7 @@ abi3-py312 = ["abi3", "pyo3-build-config/abi3-py312"] generate-import-lib = ["pyo3-build-config/python3-dll-a"] [build-dependencies] -pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.1", features = ["resolve-config"] } +pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.2", features = ["resolve-config"] } [lints] workspace = true diff --git a/pyo3-macros-backend/Cargo.toml b/pyo3-macros-backend/Cargo.toml index 6ca4eeade8c..f2675f2b75e 100644 --- a/pyo3-macros-backend/Cargo.toml +++ b/pyo3-macros-backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-macros-backend" -version = "0.21.1" +version = "0.21.2" description = "Code generation for PyO3 package" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -16,7 +16,7 @@ edition = "2021" [dependencies] heck = "0.4" proc-macro2 = { version = "1", default-features = false } -pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.1", features = ["resolve-config"] } +pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.2", features = ["resolve-config"] } quote = { version = "1", default-features = false } [dependencies.syn] diff --git a/pyo3-macros/Cargo.toml b/pyo3-macros/Cargo.toml index 39a4b9198c6..690924c76a5 100644 --- a/pyo3-macros/Cargo.toml +++ b/pyo3-macros/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-macros" -version = "0.21.1" +version = "0.21.2" description = "Proc macros for PyO3 package" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -22,7 +22,7 @@ experimental-declarative-modules = [] proc-macro2 = { version = "1", default-features = false } quote = "1" syn = { version = "2", features = ["full", "extra-traits"] } -pyo3-macros-backend = { path = "../pyo3-macros-backend", version = "=0.21.1" } +pyo3-macros-backend = { path = "../pyo3-macros-backend", version = "=0.21.2" } [lints] workspace = true diff --git a/pyproject.toml b/pyproject.toml index d474753ccd1..9a70116f301 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [tool.towncrier] filename = "CHANGELOG.md" -version = "0.21.1" +version = "0.21.2" start_string = "\n" template = ".towncrier.template.md" title_format = "## [{version}] - {project_date}" From b11174e96d85de439141670aa1b65cc0f5f6bcd1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 18 Apr 2024 11:22:34 +0100 Subject: [PATCH 23/31] Update heck requirement from 0.4 to 0.5 (#3966) * Update heck requirement from 0.4 to 0.5 --- updated-dependencies: - dependency-name: heck dependency-type: direct:production ... Signed-off-by: dependabot[bot] * newsfragment --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: David Hewitt --- newsfragments/3966.packaging.md | 1 + pyo3-macros-backend/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 newsfragments/3966.packaging.md diff --git a/newsfragments/3966.packaging.md b/newsfragments/3966.packaging.md new file mode 100644 index 00000000000..81220bc4e85 --- /dev/null +++ b/newsfragments/3966.packaging.md @@ -0,0 +1 @@ +Update `heck` dependency to 0.5. diff --git a/pyo3-macros-backend/Cargo.toml b/pyo3-macros-backend/Cargo.toml index f2675f2b75e..c2ffd53b0fc 100644 --- a/pyo3-macros-backend/Cargo.toml +++ b/pyo3-macros-backend/Cargo.toml @@ -14,7 +14,7 @@ edition = "2021" # not to depend on proc-macro itself. # See https://github.com/PyO3/pyo3/pull/810 for more. [dependencies] -heck = "0.4" +heck = "0.5" proc-macro2 = { version = "1", default-features = false } pyo3-build-config = { path = "../pyo3-build-config", version = "=0.21.2", features = ["resolve-config"] } quote = { version = "1", default-features = false } From d42c00d21dbff0fdf688ba49ed3ab3a41aad2bd7 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 19 Apr 2024 09:24:26 +0200 Subject: [PATCH 24/31] feature gate deprecated APIs for `PySet` (#4096) --- src/types/set.rs | 60 +++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/src/types/set.rs b/src/types/set.rs index f648bc2be1f..83938f3bf42 100644 --- a/src/types/set.rs +++ b/src/types/set.rs @@ -31,12 +31,10 @@ pyobject_native_type_core!( impl PySet { /// Deprecated form of [`PySet::new_bound`]. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PySet::new` will be replaced by `PySet::new_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PySet::new` will be replaced by `PySet::new_bound` in a future PyO3 version" )] #[inline] pub fn new<'a, 'p, T: ToPyObject + 'a>( @@ -58,12 +56,10 @@ impl PySet { } /// Deprecated form of [`PySet::empty_bound`]. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.2", - note = "`PySet::empty` will be replaced by `PySet::empty_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.2", + note = "`PySet::empty` will be replaced by `PySet::empty_bound` in a future PyO3 version" )] pub fn empty(py: Python<'_>) -> PyResult<&PySet> { Self::empty_bound(py).map(Bound::into_gil_ref) @@ -396,27 +392,29 @@ pub(crate) fn new_from_iter( } #[cfg(test)] -#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use super::PySet; - use crate::{Python, ToPyObject}; + use crate::{ + types::{PyAnyMethods, PySetMethods}, + Python, ToPyObject, + }; use std::collections::HashSet; #[test] fn test_set_new() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); assert_eq!(1, set.len()); let v = vec![1]; - assert!(PySet::new(py, &[v]).is_err()); + assert!(PySet::new_bound(py, &[v]).is_err()); }); } #[test] fn test_set_empty() { Python::with_gil(|py| { - let set = PySet::empty(py).unwrap(); + let set = PySet::empty_bound(py).unwrap(); assert_eq!(0, set.len()); assert!(set.is_empty()); }); @@ -427,11 +425,11 @@ mod tests { Python::with_gil(|py| { let mut v = HashSet::new(); let ob = v.to_object(py); - let set: &PySet = ob.downcast(py).unwrap(); + let set = ob.downcast_bound::(py).unwrap(); assert_eq!(0, set.len()); v.insert(7); let ob = v.to_object(py); - let set2: &PySet = ob.downcast(py).unwrap(); + let set2 = ob.downcast_bound::(py).unwrap(); assert_eq!(1, set2.len()); }); } @@ -439,7 +437,7 @@ mod tests { #[test] fn test_set_clear() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); assert_eq!(1, set.len()); set.clear(); assert_eq!(0, set.len()); @@ -449,7 +447,7 @@ mod tests { #[test] fn test_set_contains() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); assert!(set.contains(1).unwrap()); }); } @@ -457,7 +455,7 @@ mod tests { #[test] fn test_set_discard() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); assert!(!set.discard(2).unwrap()); assert_eq!(1, set.len()); @@ -472,7 +470,7 @@ mod tests { #[test] fn test_set_add() { Python::with_gil(|py| { - let set = PySet::new(py, &[1, 2]).unwrap(); + let set = PySet::new_bound(py, &[1, 2]).unwrap(); set.add(1).unwrap(); // Add a dupliated element assert!(set.contains(1).unwrap()); }); @@ -481,13 +479,13 @@ mod tests { #[test] fn test_set_pop() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); let val = set.pop(); assert!(val.is_some()); let val2 = set.pop(); assert!(val2.is_none()); assert!(py - .eval("print('Exception state should not be set.')", None, None) + .eval_bound("print('Exception state should not be set.')", None, None) .is_ok()); }); } @@ -495,7 +493,7 @@ mod tests { #[test] fn test_set_iter() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); for el in set { assert_eq!(1i32, el.extract::<'_, i32>().unwrap()); @@ -520,9 +518,9 @@ mod tests { #[should_panic] fn test_set_iter_mutation() { Python::with_gil(|py| { - let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap(); + let set = PySet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap(); - for _ in set { + for _ in &set { let _ = set.add(42); } }); @@ -532,9 +530,9 @@ mod tests { #[should_panic] fn test_set_iter_mutation_same_len() { Python::with_gil(|py| { - let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap(); + let set = PySet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap(); - for item in set { + for item in &set { let item: i32 = item.extract().unwrap(); let _ = set.del_item(item); let _ = set.add(item + 10); @@ -545,7 +543,7 @@ mod tests { #[test] fn test_set_iter_size_hint() { Python::with_gil(|py| { - let set = PySet::new(py, &[1]).unwrap(); + let set = PySet::new_bound(py, &[1]).unwrap(); let mut iter = set.iter(); // Exact size From cd28e1408e9f4d2eee4832e0123b8771d11b4731 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 19 Apr 2024 12:44:36 +0100 Subject: [PATCH 25/31] add `#[track_caller]` to all `Py`/`Bound`/`Borrowed` methods which panic (#4098) --- newsfragments/4098.changed.md | 1 + src/err/mod.rs | 1 + src/ffi_ptr_ext.rs | 2 ++ src/instance.rs | 9 +++++++++ src/pycell.rs | 2 ++ 5 files changed, 15 insertions(+) create mode 100644 newsfragments/4098.changed.md diff --git a/newsfragments/4098.changed.md b/newsfragments/4098.changed.md new file mode 100644 index 00000000000..5df526a52e3 --- /dev/null +++ b/newsfragments/4098.changed.md @@ -0,0 +1 @@ +Add `#[track_caller]` to all `Py`, `Bound<'py, T>` and `Borrowed<'a, 'py, T>` methods which can panic. diff --git a/src/err/mod.rs b/src/err/mod.rs index 8dd16b26b47..a61c8c62d31 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -1091,6 +1091,7 @@ fn display_downcast_error( ) } +#[track_caller] pub fn panic_after_error(_py: Python<'_>) -> ! { unsafe { ffi::PyErr_Print(); diff --git a/src/ffi_ptr_ext.rs b/src/ffi_ptr_ext.rs index 3ca8671f1f6..183b0e3734e 100644 --- a/src/ffi_ptr_ext.rs +++ b/src/ffi_ptr_ext.rs @@ -39,6 +39,7 @@ impl FfiPtrExt for *mut ffi::PyObject { } #[inline] + #[track_caller] unsafe fn assume_owned(self, py: Python<'_>) -> Bound<'_, PyAny> { Bound::from_owned_ptr(py, self) } @@ -57,6 +58,7 @@ impl FfiPtrExt for *mut ffi::PyObject { } #[inline] + #[track_caller] unsafe fn assume_borrowed<'a>(self, py: Python<'_>) -> Borrowed<'a, '_, PyAny> { Borrowed::from_ptr(py, self) } diff --git a/src/instance.rs b/src/instance.rs index 88a550ffd30..b2715abe2b9 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -104,6 +104,7 @@ impl<'py> Bound<'py, PyAny> { /// - `ptr` must be a valid pointer to a Python object /// - `ptr` must be an owned Python reference, as the `Bound<'py, PyAny>` will assume ownership #[inline] + #[track_caller] pub unsafe fn from_owned_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { Self(py, ManuallyDrop::new(Py::from_owned_ptr(py, ptr))) } @@ -141,6 +142,7 @@ impl<'py> Bound<'py, PyAny> { /// /// - `ptr` must be a valid pointer to a Python object #[inline] + #[track_caller] pub unsafe fn from_borrowed_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { Self(py, ManuallyDrop::new(Py::from_borrowed_ptr(py, ptr))) } @@ -242,6 +244,7 @@ where /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use /// [`try_borrow`](#method.try_borrow). #[inline] + #[track_caller] pub fn borrow(&self) -> PyRef<'py, T> { PyRef::borrow(self) } @@ -276,6 +279,7 @@ where /// Panics if the value is currently borrowed. For a non-panicking variant, use /// [`try_borrow_mut`](#method.try_borrow_mut). #[inline] + #[track_caller] pub fn borrow_mut(&self) -> PyRefMut<'py, T> where T: PyClass, @@ -573,6 +577,7 @@ impl<'a, 'py> Borrowed<'a, 'py, PyAny> { /// the caller and it is the caller's responsibility to ensure that the reference this is /// derived from is valid for the lifetime `'a`. #[inline] + #[track_caller] pub unsafe fn from_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { Self( NonNull::new(ptr).unwrap_or_else(|| crate::err::panic_after_error(py)), @@ -1138,6 +1143,7 @@ where /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use /// [`try_borrow`](#method.try_borrow). #[inline] + #[track_caller] pub fn borrow<'py>(&'py self, py: Python<'py>) -> PyRef<'py, T> { self.bind(py).borrow() } @@ -1175,6 +1181,7 @@ where /// Panics if the value is currently borrowed. For a non-panicking variant, use /// [`try_borrow_mut`](#method.try_borrow_mut). #[inline] + #[track_caller] pub fn borrow_mut<'py>(&'py self, py: Python<'py>) -> PyRefMut<'py, T> where T: PyClass, @@ -1585,6 +1592,7 @@ impl Py { /// # Panics /// Panics if `ptr` is null. #[inline] + #[track_caller] pub unsafe fn from_owned_ptr(py: Python<'_>, ptr: *mut ffi::PyObject) -> Py { match NonNull::new(ptr) { Some(nonnull_ptr) => Py(nonnull_ptr, PhantomData), @@ -1628,6 +1636,7 @@ impl Py { /// # Panics /// Panics if `ptr` is null. #[inline] + #[track_caller] pub unsafe fn from_borrowed_ptr(py: Python<'_>, ptr: *mut ffi::PyObject) -> Py { match Self::from_borrowed_ptr_or_opt(py, ptr) { Some(slf) => slf, diff --git a/src/pycell.rs b/src/pycell.rs index a649ad02412..80ccff0a030 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -652,6 +652,7 @@ impl<'py, T: PyClass> PyRef<'py, T> { self.inner.clone().into_ptr() } + #[track_caller] pub(crate) fn borrow(obj: &Bound<'py, T>) -> Self { Self::try_borrow(obj).expect("Already mutably borrowed") } @@ -848,6 +849,7 @@ impl<'py, T: PyClass> PyRefMut<'py, T> { } #[inline] + #[track_caller] pub(crate) fn borrow(obj: &Bound<'py, T>) -> Self { Self::try_borrow(obj).expect("Already borrowed") } From 947b372dcc776f717e5a9862c87c85609a9e8d0d Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 19 Apr 2024 20:35:52 +0100 Subject: [PATCH 26/31] change `PyAnyMethods::dir` to be fallible (#4100) --- newsfragments/4100.changed.md | 1 + src/types/any.rs | 34 +++++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 newsfragments/4100.changed.md diff --git a/newsfragments/4100.changed.md b/newsfragments/4100.changed.md new file mode 100644 index 00000000000..13fd2e02aa8 --- /dev/null +++ b/newsfragments/4100.changed.md @@ -0,0 +1 @@ +Change `PyAnyMethods::dir` to be fallible and return `PyResult>` (and similar for `PyAny::dir`). diff --git a/src/types/any.rs b/src/types/any.rs index a5ea4c80d4c..6ba34c86bc6 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -845,8 +845,8 @@ impl PyAny { /// Returns the list of attributes of this object. /// /// This is equivalent to the Python expression `dir(self)`. - pub fn dir(&self) -> &PyList { - self.as_borrowed().dir().into_gil_ref() + pub fn dir(&self) -> PyResult<&PyList> { + self.as_borrowed().dir().map(Bound::into_gil_ref) } /// Checks whether this object is an instance of type `ty`. @@ -1674,7 +1674,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// Returns the list of attributes of this object. /// /// This is equivalent to the Python expression `dir(self)`. - fn dir(&self) -> Bound<'py, PyList>; + fn dir(&self) -> PyResult>; /// Checks whether this object is an instance of type `ty`. /// @@ -2220,10 +2220,10 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { Ok(v as usize) } - fn dir(&self) -> Bound<'py, PyList> { + fn dir(&self) -> PyResult> { unsafe { ffi::PyObject_Dir(self.as_ptr()) - .assume_owned(self.py()) + .assume_owned_or_err(self.py()) .downcast_into_unchecked() } } @@ -2471,6 +2471,7 @@ class SimpleClass: .unwrap(); let a = obj .dir() + .unwrap() .into_iter() .map(|x| x.extract::().unwrap()); let b = dir.into_iter().map(|x| x.extract::().unwrap()); @@ -2745,4 +2746,27 @@ class SimpleClass: assert!(not_container.is_empty().is_err()); }); } + + #[cfg(feature = "macros")] + #[test] + #[allow(unknown_lints, non_local_definitions)] + fn test_fallible_dir() { + use crate::exceptions::PyValueError; + use crate::prelude::*; + + #[pyclass(crate = "crate")] + struct DirFail; + + #[pymethods(crate = "crate")] + impl DirFail { + fn __dir__(&self) -> PyResult { + Err(PyValueError::new_err("uh-oh!")) + } + } + + Python::with_gil(|py| { + let obj = Bound::new(py, DirFail).unwrap(); + assert!(obj.dir().unwrap_err().is_instance_of::(py)); + }) + } } From b0ad1e10aaf15a446635fd9bb8488079dc250624 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Mon, 22 Apr 2024 09:19:01 +0200 Subject: [PATCH 27/31] feature gate deprecated APIs for `PyTuple` (#4107) --- src/types/tuple.rs | 85 +++++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/src/types/tuple.rs b/src/types/tuple.rs index 636a2f3e11f..563a81983fa 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -59,12 +59,10 @@ pyobject_native_type_core!(PyTuple, pyobject_native_static_type_object!(ffi::PyT impl PyTuple { /// Deprecated form of `PyTuple::new_bound`. #[track_caller] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PyTuple::new` will be replaced by `PyTuple::new_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyTuple::new` will be replaced by `PyTuple::new_bound` in a future PyO3 version" )] pub fn new( py: Python<'_>, @@ -117,12 +115,10 @@ impl PyTuple { } /// Deprecated form of `PyTuple::empty_bound`. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PyTuple::empty` will be replaced by `PyTuple::empty_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyTuple::empty` will be replaced by `PyTuple::empty_bound` in a future PyO3 version" )] pub fn empty(py: Python<'_>) -> &PyTuple { Self::empty_bound(py).into_gil_ref() @@ -832,24 +828,23 @@ tuple_conversion!( ); #[cfg(test)] -#[allow(deprecated)] // TODO: remove allow when GIL Pool is removed mod tests { - use crate::types::{any::PyAnyMethods, tuple::PyTupleMethods, PyAny, PyList, PyTuple}; + use crate::types::{any::PyAnyMethods, tuple::PyTupleMethods, PyList, PyTuple}; use crate::{Python, ToPyObject}; use std::collections::HashSet; #[test] fn test_new() { Python::with_gil(|py| { - let ob = PyTuple::new(py, [1, 2, 3]); + let ob = PyTuple::new_bound(py, [1, 2, 3]); assert_eq!(3, ob.len()); - let ob: &PyAny = ob.into(); + let ob = ob.as_any(); assert_eq!((1, 2, 3), ob.extract().unwrap()); let mut map = HashSet::new(); map.insert(1); map.insert(2); - PyTuple::new(py, map); + PyTuple::new_bound(py, map); }); } @@ -857,10 +852,10 @@ mod tests { fn test_len() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); assert_eq!(3, tuple.len()); assert!(!tuple.is_empty()); - let ob: &PyAny = tuple.into(); + let ob = tuple.as_any(); assert_eq!((1, 2, 3), ob.extract().unwrap()); }); } @@ -868,7 +863,7 @@ mod tests { #[test] fn test_empty() { Python::with_gil(|py| { - let tuple = PyTuple::empty(py); + let tuple = PyTuple::empty_bound(py); assert!(tuple.is_empty()); assert_eq!(0, tuple.len()); }); @@ -877,7 +872,7 @@ mod tests { #[test] fn test_slice() { Python::with_gil(|py| { - let tup = PyTuple::new(py, [2, 3, 5, 7]); + let tup = PyTuple::new_bound(py, [2, 3, 5, 7]); let slice = tup.get_slice(1, 3); assert_eq!(2, slice.len()); let slice = tup.get_slice(1, 7); @@ -889,7 +884,7 @@ mod tests { fn test_iter() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); assert_eq!(3, tuple.len()); let mut iter = tuple.iter(); @@ -913,7 +908,7 @@ mod tests { fn test_iter_rev() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); assert_eq!(3, tuple.len()); let mut iter = tuple.iter().rev(); @@ -983,7 +978,7 @@ mod tests { fn test_into_iter() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); assert_eq!(3, tuple.len()); for (i, item) in tuple.iter().enumerate() { @@ -1014,7 +1009,7 @@ mod tests { fn test_as_slice() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); let slice = tuple.as_slice(); assert_eq!(3, slice.len()); @@ -1092,7 +1087,7 @@ mod tests { fn test_tuple_get_item_invalid_index() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); let obj = tuple.get_item(5); assert!(obj.is_err()); assert_eq!( @@ -1106,7 +1101,7 @@ mod tests { fn test_tuple_get_item_sanity() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); let obj = tuple.get_item(0); assert_eq!(obj.unwrap().extract::().unwrap(), 1); }); @@ -1117,13 +1112,15 @@ mod tests { fn test_tuple_get_item_unchecked_sanity() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); let obj = unsafe { tuple.get_item_unchecked(0) }; assert_eq!(obj.extract::().unwrap(), 1); }); } #[test] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1136,6 +1133,8 @@ mod tests { #[test] #[should_panic] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait_panic() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1145,6 +1144,8 @@ mod tests { } #[test] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait_ranges() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1165,6 +1166,8 @@ mod tests { #[test] #[should_panic = "range start index 5 out of range for tuple of length 3"] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait_range_panic_start() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1175,6 +1178,8 @@ mod tests { #[test] #[should_panic = "range end index 10 out of range for tuple of length 3"] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait_range_panic_end() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1185,6 +1190,8 @@ mod tests { #[test] #[should_panic = "slice index starts at 2 but ends at 1"] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait_range_panic_wrong_order() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1196,6 +1203,8 @@ mod tests { #[test] #[should_panic = "range start index 8 out of range for tuple of length 3"] + #[cfg(feature = "gil-refs")] + #[allow(deprecated)] fn test_tuple_index_trait_range_from_panic() { Python::with_gil(|py| { let ob = (1, 2, 3).to_object(py); @@ -1208,7 +1217,7 @@ mod tests { fn test_tuple_contains() { Python::with_gil(|py| { let ob = (1, 1, 2, 3, 5, 8).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); assert_eq!(6, tuple.len()); let bad_needle = 7i32.to_object(py); @@ -1226,7 +1235,7 @@ mod tests { fn test_tuple_index() { Python::with_gil(|py| { let ob = (1, 1, 2, 3, 5, 8).to_object(py); - let tuple: &PyTuple = ob.downcast(py).unwrap(); + let tuple = ob.downcast_bound::(py).unwrap(); assert_eq!(0, tuple.index(1i32).unwrap()); assert_eq!(2, tuple.index(2i32).unwrap()); assert_eq!(3, tuple.index(3i32).unwrap()); @@ -1263,7 +1272,7 @@ mod tests { fn too_long_iterator() { Python::with_gil(|py| { let iter = FaultyIter(0..usize::MAX, 73); - let _tuple = PyTuple::new(py, iter); + let _tuple = PyTuple::new_bound(py, iter); }) } @@ -1274,7 +1283,7 @@ mod tests { fn too_short_iterator() { Python::with_gil(|py| { let iter = FaultyIter(0..35, 73); - let _tuple = PyTuple::new(py, iter); + let _tuple = PyTuple::new_bound(py, iter); }) } @@ -1286,14 +1295,14 @@ mod tests { Python::with_gil(|py| { let iter = FaultyIter(0..0, usize::MAX); - let _tuple = PyTuple::new(py, iter); + let _tuple = PyTuple::new_bound(py, iter); }) } #[cfg(feature = "macros")] #[test] fn bad_clone_mem_leaks() { - use crate::{IntoPy, Py}; + use crate::{IntoPy, Py, PyAny}; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0); @@ -1346,7 +1355,7 @@ mod tests { Python::with_gil(|py| { std::panic::catch_unwind(|| { let iter = FaultyIter(0..50, 50); - let _tuple = PyTuple::new(py, iter); + let _tuple = PyTuple::new_bound(py, iter); }) .unwrap_err(); }); @@ -1361,7 +1370,7 @@ mod tests { #[cfg(feature = "macros")] #[test] fn bad_clone_mem_leaks_2() { - use crate::{IntoPy, Py}; + use crate::{IntoPy, Py, PyAny}; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0); @@ -1412,9 +1421,9 @@ mod tests { #[test] fn test_tuple_to_list() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, vec![1, 2, 3]); + let tuple = PyTuple::new_bound(py, vec![1, 2, 3]); let list = tuple.to_list(); - let list_expected = PyList::new(py, vec![1, 2, 3]); + let list_expected = PyList::new_bound(py, vec![1, 2, 3]); assert!(list.eq(list_expected).unwrap()); }) } From da2d1aa8b46636b62ea8fd07d7a3a412a3a28280 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Mon, 22 Apr 2024 09:20:32 +0200 Subject: [PATCH 28/31] feature gate deprecated APIs for `PyString` (#4101) --- src/types/string.rs | 122 +++++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 59 deletions(-) diff --git a/src/types/string.rs b/src/types/string.rs index 4bbc6fb86b0..4aa73341ae9 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -136,12 +136,10 @@ pyobject_native_type_core!(PyString, pyobject_native_static_type_object!(ffi::Py impl PyString { /// Deprecated form of [`PyString::new_bound`]. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PyString::new` will be replaced by `PyString::new_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyString::new` will be replaced by `PyString::new_bound` in a future PyO3 version" )] pub fn new<'py>(py: Python<'py>, s: &str) -> &'py Self { Self::new_bound(py, s).into_gil_ref() @@ -161,12 +159,10 @@ impl PyString { } /// Deprecated form of [`PyString::intern_bound`]. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PyString::intern` will be replaced by `PyString::intern_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyString::intern` will be replaced by `PyString::intern_bound` in a future PyO3 version" )] pub fn intern<'py>(py: Python<'py>, s: &str) -> &'py Self { Self::intern_bound(py, s).into_gil_ref() @@ -176,8 +172,8 @@ impl PyString { /// /// This will return a reference to the same Python string object if called repeatedly with the same string. /// - /// Note that while this is more memory efficient than [`PyString::new`], it unconditionally allocates a - /// temporary Python string object and is thereby slower than [`PyString::new`]. + /// Note that while this is more memory efficient than [`PyString::new_bound`], it unconditionally allocates a + /// temporary Python string object and is thereby slower than [`PyString::new_bound`]. /// /// Panics if out of memory. pub fn intern_bound<'py>(py: Python<'py>, s: &str) -> Bound<'py, PyString> { @@ -193,12 +189,10 @@ impl PyString { } /// Deprecated form of [`PyString::from_object_bound`]. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PyString::from_object` will be replaced by `PyString::from_object_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyString::from_object` will be replaced by `PyString::from_object_bound` in a future PyO3 version" )] pub fn from_object<'py>(src: &'py PyAny, encoding: &str, errors: &str) -> PyResult<&'py Self> { Self::from_object_bound(&src.as_borrowed(), encoding, errors).map(Bound::into_gil_ref) @@ -502,37 +496,37 @@ impl IntoPy> for &'_ Py { } #[cfg(test)] -#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use super::*; use crate::{PyObject, ToPyObject}; #[test] - fn test_to_str_utf8() { + fn test_to_cow_utf8() { Python::with_gil(|py| { let s = "ascii 🐈"; - let obj: PyObject = PyString::new(py, s).into(); - let py_string: &PyString = obj.downcast(py).unwrap(); - assert_eq!(s, py_string.to_str().unwrap()); + let py_string = PyString::new_bound(py, s); + assert_eq!(s, py_string.to_cow().unwrap()); }) } #[test] - fn test_to_str_surrogate() { + fn test_to_cow_surrogate() { Python::with_gil(|py| { - let obj: PyObject = py.eval(r"'\ud800'", None, None).unwrap().into(); - let py_string: &PyString = obj.downcast(py).unwrap(); - assert!(py_string.to_str().is_err()); + let py_string = py + .eval_bound(r"'\ud800'", None, None) + .unwrap() + .downcast_into::() + .unwrap(); + assert!(py_string.to_cow().is_err()); }) } #[test] - fn test_to_str_unicode() { + fn test_to_cow_unicode() { Python::with_gil(|py| { let s = "ε“ˆε“ˆπŸˆ"; - let obj: PyObject = PyString::new(py, s).into(); - let py_string: &PyString = obj.downcast(py).unwrap(); - assert_eq!(s, py_string.to_str().unwrap()); + let py_string = PyString::new_bound(py, s); + assert_eq!(s, py_string.to_cow().unwrap()); }) } @@ -548,7 +542,7 @@ mod tests { #[test] fn test_encode_utf8_surrogate() { Python::with_gil(|py| { - let obj: PyObject = py.eval(r"'\ud800'", None, None).unwrap().into(); + let obj: PyObject = py.eval_bound(r"'\ud800'", None, None).unwrap().into(); assert!(obj .bind(py) .downcast::() @@ -561,11 +555,12 @@ mod tests { #[test] fn test_to_string_lossy() { Python::with_gil(|py| { - let obj: PyObject = py - .eval(r"'🐈 Hello \ud800World'", None, None) + let py_string = py + .eval_bound(r"'🐈 Hello \ud800World'", None, None) .unwrap() - .into(); - let py_string: &PyString = obj.downcast(py).unwrap(); + .downcast_into::() + .unwrap(); + assert_eq!(py_string.to_string_lossy(), "🐈 Hello οΏ½οΏ½οΏ½World"); }) } @@ -574,7 +569,7 @@ mod tests { fn test_debug_string() { Python::with_gil(|py| { let v = "Hello\n".to_object(py); - let s: &PyString = v.downcast(py).unwrap(); + let s = v.downcast_bound::(py).unwrap(); assert_eq!(format!("{:?}", s), "'Hello\\n'"); }) } @@ -583,7 +578,7 @@ mod tests { fn test_display_string() { Python::with_gil(|py| { let v = "Hello\n".to_object(py); - let s: &PyString = v.downcast(py).unwrap(); + let s = v.downcast_bound::(py).unwrap(); assert_eq!(format!("{}", s), "Hello\n"); }) } @@ -592,7 +587,7 @@ mod tests { #[cfg(not(Py_LIMITED_API))] fn test_string_data_ucs1() { Python::with_gil(|py| { - let s = PyString::new(py, "hello, world"); + let s = PyString::new_bound(py, "hello, world"); let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs1(b"hello, world")); @@ -615,11 +610,13 @@ mod tests { ) }; assert!(!ptr.is_null()); - let s: &PyString = unsafe { py.from_owned_ptr(ptr) }; + let s = unsafe { ptr.assume_owned(py).downcast_into_unchecked::() }; let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs1(b"f\xfe")); let err = data.to_string(py).unwrap_err(); - assert!(err.get_type(py).is(py.get_type::())); + assert!(err + .get_type_bound(py) + .is(&py.get_type_bound::())); assert!(err .to_string() .contains("'utf-8' codec can't decode byte 0xfe in position 1")); @@ -631,7 +628,7 @@ mod tests { #[cfg(not(Py_LIMITED_API))] fn test_string_data_ucs2() { Python::with_gil(|py| { - let s = py.eval("'foo\\ud800'", None, None).unwrap(); + let s = py.eval_bound("'foo\\ud800'", None, None).unwrap(); let py_string = s.downcast::().unwrap(); let data = unsafe { py_string.data().unwrap() }; @@ -657,11 +654,13 @@ mod tests { ) }; assert!(!ptr.is_null()); - let s: &PyString = unsafe { py.from_owned_ptr(ptr) }; + let s = unsafe { ptr.assume_owned(py).downcast_into_unchecked::() }; let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs2(&[0xff22, 0xd800])); let err = data.to_string(py).unwrap_err(); - assert!(err.get_type(py).is(py.get_type::())); + assert!(err + .get_type_bound(py) + .is(&py.get_type_bound::())); assert!(err .to_string() .contains("'utf-16' codec can't decode bytes in position 0-3")); @@ -674,7 +673,7 @@ mod tests { fn test_string_data_ucs4() { Python::with_gil(|py| { let s = "ε“ˆε“ˆπŸˆ"; - let py_string = PyString::new(py, s); + let py_string = PyString::new_bound(py, s); let data = unsafe { py_string.data().unwrap() }; assert_eq!(data, PyStringData::Ucs4(&[21704, 21704, 128008])); @@ -696,11 +695,13 @@ mod tests { ) }; assert!(!ptr.is_null()); - let s: &PyString = unsafe { py.from_owned_ptr(ptr) }; + let s = unsafe { ptr.assume_owned(py).downcast_into_unchecked::() }; let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs4(&[0x20000, 0xd800])); let err = data.to_string(py).unwrap_err(); - assert!(err.get_type(py).is(py.get_type::())); + assert!(err + .get_type_bound(py) + .is(&py.get_type_bound::())); assert!(err .to_string() .contains("'utf-32' codec can't decode bytes in position 0-7")); @@ -711,16 +712,16 @@ mod tests { #[test] fn test_intern_string() { Python::with_gil(|py| { - let py_string1 = PyString::intern(py, "foo"); - assert_eq!(py_string1.to_str().unwrap(), "foo"); + let py_string1 = PyString::intern_bound(py, "foo"); + assert_eq!(py_string1.to_cow().unwrap(), "foo"); - let py_string2 = PyString::intern(py, "foo"); - assert_eq!(py_string2.to_str().unwrap(), "foo"); + let py_string2 = PyString::intern_bound(py, "foo"); + assert_eq!(py_string2.to_cow().unwrap(), "foo"); assert_eq!(py_string1.as_ptr(), py_string2.as_ptr()); - let py_string3 = PyString::intern(py, "bar"); - assert_eq!(py_string3.to_str().unwrap(), "bar"); + let py_string3 = PyString::intern_bound(py, "bar"); + assert_eq!(py_string3.to_cow().unwrap(), "bar"); assert_ne!(py_string1.as_ptr(), py_string3.as_ptr()); }); @@ -730,7 +731,7 @@ mod tests { fn test_py_to_str_utf8() { Python::with_gil(|py| { let s = "ascii 🐈"; - let py_string: Py = PyString::new(py, s).into_py(py); + let py_string: Py = PyString::new_bound(py, s).into_py(py); #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] assert_eq!(s, py_string.to_str(py).unwrap()); @@ -742,8 +743,11 @@ mod tests { #[test] fn test_py_to_str_surrogate() { Python::with_gil(|py| { - let py_string: Py = - py.eval(r"'\ud800'", None, None).unwrap().extract().unwrap(); + let py_string: Py = py + .eval_bound(r"'\ud800'", None, None) + .unwrap() + .extract() + .unwrap(); #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] assert!(py_string.to_str(py).is_err()); @@ -756,7 +760,7 @@ mod tests { fn test_py_to_string_lossy() { Python::with_gil(|py| { let py_string: Py = py - .eval(r"'🐈 Hello \ud800World'", None, None) + .eval_bound(r"'🐈 Hello \ud800World'", None, None) .unwrap() .extract() .unwrap(); From c2ac9a98e2316e42e59cf8b07f0e24ff244afbb7 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Mon, 22 Apr 2024 10:56:39 +0200 Subject: [PATCH 29/31] fix vectorcall argument handling (#4104) Fixes #4093 - Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython - Remove the assert in PyVectorcall_NARGS and use checked cast --- newsfragments/4104.fixed.md | 1 + pyo3-ffi/src/cpython/abstract_.rs | 16 +++++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) create mode 100644 newsfragments/4104.fixed.md diff --git a/newsfragments/4104.fixed.md b/newsfragments/4104.fixed.md new file mode 100644 index 00000000000..3eff1654b4f --- /dev/null +++ b/newsfragments/4104.fixed.md @@ -0,0 +1 @@ +Changes definitions of `PY_VECTORCALL_ARGUMENTS_OFFSET` and `PyVectorcall_NARGS` to fix a false-positive assertion. diff --git a/pyo3-ffi/src/cpython/abstract_.rs b/pyo3-ffi/src/cpython/abstract_.rs index cf95f6711d4..ad28216cbff 100644 --- a/pyo3-ffi/src/cpython/abstract_.rs +++ b/pyo3-ffi/src/cpython/abstract_.rs @@ -4,8 +4,6 @@ use std::os::raw::{c_char, c_int}; #[cfg(not(Py_3_11))] use crate::Py_buffer; -#[cfg(Py_3_8)] -use crate::pyport::PY_SSIZE_T_MAX; #[cfg(all(Py_3_8, not(any(PyPy, GraalPy))))] use crate::{ vectorcallfunc, PyCallable_Check, PyThreadState, PyThreadState_GET, PyTuple_Check, @@ -42,14 +40,14 @@ extern "C" { } #[cfg(Py_3_8)] -const PY_VECTORCALL_ARGUMENTS_OFFSET: Py_ssize_t = - 1 << (8 * std::mem::size_of::() as Py_ssize_t - 1); +const PY_VECTORCALL_ARGUMENTS_OFFSET: size_t = + 1 << (8 * std::mem::size_of::() as size_t - 1); #[cfg(Py_3_8)] #[inline(always)] pub unsafe fn PyVectorcall_NARGS(n: size_t) -> Py_ssize_t { - assert!(n <= (PY_SSIZE_T_MAX as size_t)); - (n as Py_ssize_t) & !PY_VECTORCALL_ARGUMENTS_OFFSET + let n = n & !PY_VECTORCALL_ARGUMENTS_OFFSET; + n.try_into().expect("cannot fail due to mask") } #[cfg(all(Py_3_8, not(any(PyPy, GraalPy))))] @@ -184,7 +182,7 @@ pub unsafe fn PyObject_CallOneArg(func: *mut PyObject, arg: *mut PyObject) -> *m let args = args_array.as_ptr().offset(1); // For PY_VECTORCALL_ARGUMENTS_OFFSET let tstate = PyThreadState_GET(); let nargsf = 1 | PY_VECTORCALL_ARGUMENTS_OFFSET; - _PyObject_VectorcallTstate(tstate, func, args, nargsf as size_t, std::ptr::null_mut()) + _PyObject_VectorcallTstate(tstate, func, args, nargsf, std::ptr::null_mut()) } extern "C" { @@ -206,7 +204,7 @@ pub unsafe fn PyObject_CallMethodNoArgs( PyObject_VectorcallMethod( name, &self_, - 1 | PY_VECTORCALL_ARGUMENTS_OFFSET as size_t, + 1 | PY_VECTORCALL_ARGUMENTS_OFFSET, std::ptr::null_mut(), ) } @@ -223,7 +221,7 @@ pub unsafe fn PyObject_CallMethodOneArg( PyObject_VectorcallMethod( name, args.as_ptr(), - 2 | PY_VECTORCALL_ARGUMENTS_OFFSET as size_t, + 2 | PY_VECTORCALL_ARGUMENTS_OFFSET, std::ptr::null_mut(), ) } From 5d2f5b5702319150d41258de77f589119134ee74 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Tue, 23 Apr 2024 07:48:27 +0200 Subject: [PATCH 30/31] feature gate deprecated APIs for `PyDict` (#4108) --- src/conversion.rs | 1 + src/instance.rs | 6 +-- src/types/dict.rs | 86 ++++++++++++++++++++----------------------- src/types/iterator.rs | 9 +++-- src/types/mapping.rs | 1 + 5 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/conversion.rs b/src/conversion.rs index dfa53eac83e..ced209abade 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -674,6 +674,7 @@ mod test_no_clone {} #[cfg(test)] mod tests { + #[cfg(feature = "gil-refs")] #[allow(deprecated)] mod deprecated { use super::super::PyTryFrom; diff --git a/src/instance.rs b/src/instance.rs index b2715abe2b9..02b3fc76241 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2034,7 +2034,7 @@ mod tests { #[test] fn test_call_for_non_existing_method() { Python::with_gil(|py| { - let obj: PyObject = PyDict::new(py).into(); + let obj: PyObject = PyDict::new_bound(py).into(); assert!(obj.call_method0(py, "asdf").is_err()); assert!(obj .call_method(py, "nonexistent_method", (1,), None) @@ -2047,7 +2047,7 @@ mod tests { #[test] fn py_from_dict() { let dict: Py = Python::with_gil(|py| { - let native = PyDict::new(py); + let native = PyDict::new_bound(py); Py::from(native) }); @@ -2057,7 +2057,7 @@ mod tests { #[test] fn pyobject_from_py() { Python::with_gil(|py| { - let dict: Py = PyDict::new(py).into(); + let dict: Py = PyDict::new_bound(py).unbind(); let cnt = dict.get_refcnt(py); let p: PyObject = dict.into(); assert_eq!(p.get_refcnt(py), cnt); diff --git a/src/types/dict.rs b/src/types/dict.rs index a4ba72a59c0..64b3adcad44 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -57,12 +57,10 @@ pyobject_native_type_core!( impl PyDict { /// Deprecated form of [`new_bound`][PyDict::new_bound]. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`PyDict::new` will be replaced by `PyDict::new_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyDict::new` will be replaced by `PyDict::new_bound` in a future PyO3 version" )] #[inline] pub fn new(py: Python<'_>) -> &PyDict { @@ -75,12 +73,10 @@ impl PyDict { } /// Deprecated form of [`from_sequence_bound`][PyDict::from_sequence_bound]. - #[cfg_attr( - all(not(any(PyPy, GraalPy)), not(feature = "gil-refs")), - deprecated( - since = "0.21.0", - note = "`PyDict::from_sequence` will be replaced by `PyDict::from_sequence_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`PyDict::from_sequence` will be replaced by `PyDict::from_sequence_bound` in a future PyO3 version" )] #[inline] #[cfg(not(any(PyPy, GraalPy)))] @@ -751,12 +747,10 @@ pub(crate) use borrowed_iter::BorrowedDictIter; pub trait IntoPyDict: Sized { /// Converts self into a `PyDict` object pointer. Whether pointer owned or borrowed /// depends on implementation. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "`IntoPyDict::into_py_dict` will be replaced by `IntoPyDict::into_py_dict_bound` in a future PyO3 version" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "`IntoPyDict::into_py_dict` will be replaced by `IntoPyDict::into_py_dict_bound` in a future PyO3 version" )] fn into_py_dict(self, py: Python<'_>) -> &PyDict { Self::into_py_dict_bound(self, py).into_gil_ref() @@ -821,7 +815,6 @@ where } #[cfg(test)] -#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use super::*; #[cfg(not(any(PyPy, GraalPy)))] @@ -853,8 +846,8 @@ mod tests { #[cfg(not(any(PyPy, GraalPy)))] fn test_from_sequence() { Python::with_gil(|py| { - let items = PyList::new(py, &vec![("a", 1), ("b", 2)]); - let dict = PyDict::from_sequence(items).unwrap(); + let items = PyList::new_bound(py, &vec![("a", 1), ("b", 2)]); + let dict = PyDict::from_sequence_bound(&items).unwrap(); assert_eq!( 1, dict.get_item("a") @@ -884,8 +877,8 @@ mod tests { #[cfg(not(any(PyPy, GraalPy)))] fn test_from_sequence_err() { Python::with_gil(|py| { - let items = PyList::new(py, &vec!["a", "b"]); - assert!(PyDict::from_sequence(items).is_err()); + let items = PyList::new_bound(py, &vec!["a", "b"]); + assert!(PyDict::from_sequence_bound(&items).is_err()); }); } @@ -913,11 +906,11 @@ mod tests { Python::with_gil(|py| { let mut v = HashMap::new(); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert_eq!(0, dict.len()); v.insert(7, 32); let ob = v.to_object(py); - let dict2: &PyDict = ob.downcast(py).unwrap(); + let dict2 = ob.downcast_bound::(py).unwrap(); assert_eq!(1, dict2.len()); }); } @@ -928,7 +921,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert!(dict.contains(7i32).unwrap()); assert!(!dict.contains(8i32).unwrap()); }); @@ -940,7 +933,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert_eq!( 32, dict.get_item(7i32) @@ -961,7 +954,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast::(py).unwrap(); assert_eq!( 32, dict.get_item_with_error(7i32) @@ -984,7 +977,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert!(dict.set_item(7i32, 42i32).is_ok()); // change assert!(dict.set_item(8i32, 123i32).is_ok()); // insert assert_eq!( @@ -1010,11 +1003,10 @@ mod tests { fn test_set_item_refcnt() { Python::with_gil(|py| { let cnt; - let obj = py.eval("object()", None, None).unwrap(); + let obj = py.eval_bound("object()", None, None).unwrap(); { - let _pool = unsafe { crate::GILPool::new() }; cnt = obj.get_refcnt(); - let _dict = [(10, obj)].into_py_dict_bound(py); + let _dict = [(10, &obj)].into_py_dict_bound(py); } { assert_eq!(cnt, obj.get_refcnt()); @@ -1028,7 +1020,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert!(dict.set_item(7i32, 42i32).is_ok()); // change assert!(dict.set_item(8i32, 123i32).is_ok()); // insert assert_eq!(32i32, v[&7i32]); // not updated! @@ -1042,7 +1034,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert!(dict.del_item(7i32).is_ok()); assert_eq!(0, dict.len()); assert!(dict.get_item(7i32).unwrap().is_none()); @@ -1055,7 +1047,7 @@ mod tests { let mut v = HashMap::new(); v.insert(7, 32); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); assert!(dict.del_item(7i32).is_ok()); // change assert_eq!(32i32, *v.get(&7i32).unwrap()); // not updated! }); @@ -1069,7 +1061,7 @@ mod tests { v.insert(8, 42); v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. let mut key_sum = 0; let mut value_sum = 0; @@ -1091,7 +1083,7 @@ mod tests { v.insert(8, 42); v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. let mut key_sum = 0; for el in dict.keys() { @@ -1109,7 +1101,7 @@ mod tests { v.insert(8, 42); v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. let mut values_sum = 0; for el in dict.values() { @@ -1127,7 +1119,7 @@ mod tests { v.insert(8, 42); v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); let mut key_sum = 0; let mut value_sum = 0; for (key, value) in dict { @@ -1168,7 +1160,7 @@ mod tests { v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); for (key, value) in dict { dict.set_item(key, value.extract::().unwrap() + 7) @@ -1186,7 +1178,7 @@ mod tests { v.insert(i * 2, i * 2); } let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); for (i, (key, value)) in dict.iter().enumerate() { let key = key.extract::().unwrap(); @@ -1211,7 +1203,7 @@ mod tests { v.insert(i * 2, i * 2); } let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); for (i, (key, value)) in dict.iter().enumerate() { let key = key.extract::().unwrap(); @@ -1235,7 +1227,7 @@ mod tests { v.insert(8, 42); v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); let mut iter = dict.iter(); assert_eq!(iter.size_hint(), (v.len(), Some(v.len()))); @@ -1261,7 +1253,7 @@ mod tests { v.insert(8, 42); v.insert(9, 123); let ob = v.to_object(py); - let dict: &PyDict = ob.downcast(py).unwrap(); + let dict = ob.downcast_bound::(py).unwrap(); let mut key_sum = 0; let mut value_sum = 0; for (key, value) in dict { @@ -1404,7 +1396,7 @@ mod tests { let dict = abc_dict(py); let keys = dict.call_method0("keys").unwrap(); assert!(keys - .is_instance(&py.get_type::().as_borrowed()) + .is_instance(&py.get_type_bound::().as_borrowed()) .unwrap()); }) } @@ -1416,7 +1408,7 @@ mod tests { let dict = abc_dict(py); let values = dict.call_method0("values").unwrap(); assert!(values - .is_instance(&py.get_type::().as_borrowed()) + .is_instance(&py.get_type_bound::().as_borrowed()) .unwrap()); }) } @@ -1428,7 +1420,7 @@ mod tests { let dict = abc_dict(py); let items = dict.call_method0("items").unwrap(); assert!(items - .is_instance(&py.get_type::().as_borrowed()) + .is_instance(&py.get_type_bound::().as_borrowed()) .unwrap()); }) } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index bd01fe81a78..60f36f22de2 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -158,7 +158,7 @@ mod tests { use super::PyIterator; use crate::exceptions::PyTypeError; use crate::gil::GILPool; - use crate::types::{PyDict, PyList}; + use crate::types::{PyAnyMethods, PyDict, PyList}; use crate::{Py, PyAny, Python, ToPyObject}; #[test] @@ -244,10 +244,11 @@ def fibonacci(target): "#; Python::with_gil(|py| { - let context = PyDict::new(py); - py.run(fibonacci_generator, None, Some(context)).unwrap(); + let context = PyDict::new_bound(py); + py.run_bound(fibonacci_generator, None, Some(&context)) + .unwrap(); - let generator = py.eval("fibonacci(5)", None, Some(context)).unwrap(); + let generator = py.eval_bound("fibonacci(5)", None, Some(&context)).unwrap(); for (actual, expected) in generator.iter().unwrap().zip(&[1, 1, 2, 3, 5]) { let actual = actual.unwrap().extract::().unwrap(); assert_eq!(actual, *expected) diff --git a/src/types/mapping.rs b/src/types/mapping.rs index a5a93163bbd..a91dad3679f 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -435,6 +435,7 @@ mod tests { } #[test] + #[cfg(feature = "gil-refs")] #[allow(deprecated)] fn test_mapping_try_from() { use crate::PyTryFrom; From f5fee94afcaf11edceceed45192aabd71aeb9415 Mon Sep 17 00:00:00 2001 From: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com> Date: Tue, 23 Apr 2024 20:01:41 +0200 Subject: [PATCH 31/31] Scope macro imports more tightly (#4088) --- pyo3-macros-backend/src/module.rs | 11 ++++++----- pytests/src/enums.rs | 4 +++- tests/test_no_imports.rs | 5 ++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 3153279a2b8..626cde121e6 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -382,10 +382,7 @@ fn module_initialization(options: PyModuleOptions, ident: &syn::Ident) -> TokenS fn process_functions_in_module(options: &PyModuleOptions, func: &mut syn::ItemFn) -> Result<()> { let ctx = &Ctx::new(&options.krate); let Ctx { pyo3_path } = ctx; - let mut stmts: Vec = vec![syn::parse_quote!( - #[allow(unknown_lints, unused_imports, redundant_imports)] - use #pyo3_path::{PyNativeType, types::PyModuleMethods}; - )]; + let mut stmts: Vec = Vec::new(); for mut stmt in func.block.stmts.drain(..) { if let syn::Stmt::Item(Item::Fn(func)) = &mut stmt { @@ -395,7 +392,11 @@ fn process_functions_in_module(options: &PyModuleOptions, func: &mut syn::ItemFn let name = &func.sig.ident; let statements: Vec = syn::parse_quote! { #wrapped_function - #module_name.as_borrowed().add_function(#pyo3_path::wrap_pyfunction!(#name, #module_name.as_borrowed())?)?; + { + #[allow(unknown_lints, unused_imports, redundant_imports)] + use #pyo3_path::{PyNativeType, types::PyModuleMethods}; + #module_name.as_borrowed().add_function(#pyo3_path::wrap_pyfunction!(#name, #module_name.as_borrowed())?)?; + } }; stmts.extend(statements); } diff --git a/pytests/src/enums.rs b/pytests/src/enums.rs index 4bb269fbbd2..0a1bc49bb63 100644 --- a/pytests/src/enums.rs +++ b/pytests/src/enums.rs @@ -1,5 +1,7 @@ use pyo3::{ - pyclass, pyfunction, pymodule, types::PyModule, wrap_pyfunction_bound, Bound, PyResult, + pyclass, pyfunction, pymodule, + types::{PyModule, PyModuleMethods}, + wrap_pyfunction_bound, Bound, PyResult, }; #[pymodule] diff --git a/tests/test_no_imports.rs b/tests/test_no_imports.rs index 35c978b0da5..022d61e084d 100644 --- a/tests/test_no_imports.rs +++ b/tests/test_no_imports.rs @@ -30,7 +30,10 @@ fn basic_module_bound(m: &pyo3::Bound<'_, pyo3::types::PyModule>) -> pyo3::PyRes 42 } - m.add_function(pyo3::wrap_pyfunction_bound!(basic_function, m)?)?; + pyo3::types::PyModuleMethods::add_function( + m, + pyo3::wrap_pyfunction_bound!(basic_function, m)?, + )?; Ok(()) }