From 278cea66b07fbe7cc366099486cec77758b9abce Mon Sep 17 00:00:00 2001 From: mokashang Date: Sun, 14 Jun 2026 14:28:10 -0700 Subject: [PATCH 1/4] fix: don't collide regular-method wrappers with #[getter]/#[setter]/#[deleter] (fixes #5974) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, `#[pymethods]` generated the same internal associated function name for two distinct kinds of method: - a getter for `x` (`#[getter] fn x`) becomes `__pymethod_get_x__` (the `get_` prefix is from `impl_py_getter_def`). - a regular method called `get_x` becomes `__pymethod_get_x__` (the whole name comes from `impl_py_method_def`, which uses `__pymethod_{python_name}__`). These clash with `error[E0592]: duplicate definitions with name `__pymethod_get_x__``. The same pattern hits `set_X` against `#[setter]` and `delete_X` against `#[deleter]`, and it survives `#[pyo3(name = "get_y")]` because the regular-method wrapper is keyed off `spec.python_name`. This forces user-facing Python APIs to choose between common idioms that should coexist — a `value` property *and* a separate `get_value` method for parameterised lookup, which is a common pattern when adapting legacy code. Fix the regular-method wrapper to use a `__pymethod_method_*__` infix, which can no longer collide with `__pymethod_get_*__`, `__pymethod_set_*__`, or `__pymethod_delete_*__` regardless of the chosen Python name. The maintainer-suggested direction in #5974 was to add a distinct prefix that is not a substring of the property prefixes; `method` reads more naturally than `standard` and has the same effect. Add a regression test in `tests/test_getter_setter.rs` covering all three collision shapes (regular method, `#[pyo3(name = ...)]` rename, and the setter variant). Update the existing UI test that was asserting on the old wrapper name. --- pyo3-macros-backend/src/pymethod.rs | 5 +- tests/test_getter_setter.rs | 79 ++++++++++++++++++++ tests/ui/invalid_pymethods_duplicates.rs | 2 +- tests/ui/invalid_pymethods_duplicates.stderr | 6 +- 4 files changed, 87 insertions(+), 5 deletions(-) diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index c529c40f4ff..76267ecb081 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -372,7 +372,10 @@ pub fn impl_py_method_def( ctx: &Ctx, ) -> Result { let Ctx { pyo3_path, .. } = ctx; - let wrapper_ident = format_ident!("__pymethod_{}__", spec.python_name); + // The `method_` infix keeps the generated identifier from colliding with + // the wrappers for `#[getter]`/`#[setter]`/`#[deleter]`, which use the + // `get_`/`set_`/`delete_` prefixes (see #5974). + let wrapper_ident = format_ident!("__pymethod_method_{}__", spec.python_name); let calling_convention = CallingConvention::from_signature(&spec.signature); let associated_method = spec.get_wrapper_function( &wrapper_ident, diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index 582cfd89c5f..e61b450c126 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -317,3 +317,82 @@ fn test_optional_setter() { ); }) } + +// Regression test for #5974: previously, the wrapper generated for a regular +// method called `get_x` collided with the wrapper generated for a `#[getter]` +// of `x`, because both produced an associated function named +// `__pymethod_get_x__`. The same problem applied to setters/deleters and to +// `#[pyo3(name = ...)]` renames that happened to start with `get_`/`set_`/ +// `delete_`. The fix is to use a distinct infix for regular methods. +#[test] +fn property_and_regular_method_can_share_name_prefix() { + #[pyclass] + struct Object { + x: u32, + y: u32, + z: u32, + } + + #[pymethods] + impl Object { + #[getter] + fn x(&self) -> u32 { + self.x + } + + // Was previously a compile error: wrapper collided with the `x` getter. + fn get_x(&self) -> u32 { + self.x + 1 + } + + #[getter] + fn y(&self) -> u32 { + self.y + } + + // Was previously a compile error too: `#[pyo3(name = ...)]` is also + // routed through `python_name`. + #[pyo3(name = "get_y")] + fn y_get(&self) -> u32 { + self.y + 2 + } + + #[setter] + fn z(&mut self, value: u32) { + self.z = value; + } + + // Same collision pattern as above, but for setters. + fn set_z(&mut self, value: u32) { + self.z = value + 1; + } + } + + Python::attach(|py| { + let instance = Py::new( + py, + Object { + x: 10, + y: 20, + z: 30, + }, + ) + .unwrap(); + py_run!( + py, + instance, + "assert instance.x == 10 and instance.get_x() == 11" + ); + py_run!( + py, + instance, + "assert instance.y == 20 and instance.get_y() == 22" + ); + // `z` has only a setter; we exercise both the setter and the regular + // method by routing reads through the borrowed Rust value. + py_run!(py, instance, "instance.z = 5"); + assert_eq!(instance.borrow(py).z, 5); + py_run!(py, instance, "instance.set_z(5)"); + assert_eq!(instance.borrow(py).z, 6); + }) +} diff --git a/tests/ui/invalid_pymethods_duplicates.rs b/tests/ui/invalid_pymethods_duplicates.rs index 018e2b80c12..5c842dfea53 100644 --- a/tests/ui/invalid_pymethods_duplicates.rs +++ b/tests/ui/invalid_pymethods_duplicates.rs @@ -25,7 +25,7 @@ impl TwoNew { struct DuplicateMethod {} #[pymethods] -//~^ ERROR: duplicate definitions with name `__pymethod_func__` +//~^ ERROR: duplicate definitions with name `__pymethod_method_func__` impl DuplicateMethod { #[pyo3(name = "func")] fn func_a(&self) {} diff --git a/tests/ui/invalid_pymethods_duplicates.stderr b/tests/ui/invalid_pymethods_duplicates.stderr index 6856ee97863..a8d0817ccf3 100644 --- a/tests/ui/invalid_pymethods_duplicates.stderr +++ b/tests/ui/invalid_pymethods_duplicates.stderr @@ -20,14 +20,14 @@ error[E0592]: duplicate definitions with name `__pymethod___new____` | = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) -error[E0592]: duplicate definitions with name `__pymethod_func__` +error[E0592]: duplicate definitions with name `__pymethod_method_func__` --> tests/ui/invalid_pymethods_duplicates.rs:27:1 | 27 | #[pymethods] | ^^^^^^^^^^^^ | | - | duplicate definitions for `__pymethod_func__` - | other definition for `__pymethod_func__` + | duplicate definitions for `__pymethod_method_func__` + | other definition for `__pymethod_method_func__` | = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) From 4229343db59ce3e84480e85b418c00c326216253 Mon Sep 17 00:00:00 2001 From: mokashang Date: Sun, 14 Jun 2026 14:29:24 -0700 Subject: [PATCH 2/4] newsfragment: 6135.fixed.md --- newsfragments/6135.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/6135.fixed.md diff --git a/newsfragments/6135.fixed.md b/newsfragments/6135.fixed.md new file mode 100644 index 00000000000..c308ceda5dd --- /dev/null +++ b/newsfragments/6135.fixed.md @@ -0,0 +1 @@ +Fix `#[pymethods]` so a regular method whose Python name starts with `get_`, `set_`, or `delete_` no longer collides with a property of the trimmed name (e.g. `fn get_x` alongside `#[getter] fn x`). From b07540f1f97862892e6e8b571d1a3b8188da3d88 Mon Sep 17 00:00:00 2001 From: mokashang Date: Mon, 15 Jun 2026 09:06:28 -0700 Subject: [PATCH 3/4] review: drop redundant explanatory comment in pymethod.rs The regression test in test_getter_setter.rs already documents the #5974 collision pattern in detail, so the in-source comment was duplicating that explanation. --- pyo3-macros-backend/src/pymethod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 76267ecb081..f4c2574177a 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -372,9 +372,6 @@ pub fn impl_py_method_def( ctx: &Ctx, ) -> Result { let Ctx { pyo3_path, .. } = ctx; - // The `method_` infix keeps the generated identifier from colliding with - // the wrappers for `#[getter]`/`#[setter]`/`#[deleter]`, which use the - // `get_`/`set_`/`delete_` prefixes (see #5974). let wrapper_ident = format_ident!("__pymethod_method_{}__", spec.python_name); let calling_convention = CallingConvention::from_signature(&spec.signature); let associated_method = spec.get_wrapper_function( From 1bbdc5f7cc1d3cd85fa7bbb25bc079bdd57edbd0 Mon Sep 17 00:00:00 2001 From: mokashang Date: Tue, 16 Jun 2026 09:06:21 -0700 Subject: [PATCH 4/4] test: refresh invalid_pyclass_generic UI test for renamed wrapper The wrapper identifier for regular #[pymethods] methods was renamed to __pymethod_method_{name}__ to avoid colliding with getter/setter wrappers (this PR's fix for #5974). One UI test snapshot still referenced the old __pymethod_{name}__ symbol both in its //~ ERROR annotation and in the .stderr file, so the trybuild check failed on the python3.14t job. Update the inline directive and snapshot to match the new symbol. --- tests/ui/invalid_pyclass_generic.rs | 2 +- tests/ui/invalid_pyclass_generic.stderr | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ui/invalid_pyclass_generic.rs b/tests/ui/invalid_pyclass_generic.rs index 23524bb1237..f79fc7c147b 100644 --- a/tests/ui/invalid_pyclass_generic.rs +++ b/tests/ui/invalid_pyclass_generic.rs @@ -2,7 +2,7 @@ use pyo3::prelude::*; use pyo3::types::PyType; #[pyclass(generic)] -//~^ ERROR: duplicate definitions with name `__pymethod___class_getitem____` +//~^ ERROR: duplicate definitions with name `__pymethod_method___class_getitem____` //~| ERROR: duplicate definitions with name `__class_getitem__` //~| ERROR: multiple applicable items in scope //~| ERROR: multiple applicable items in scope diff --git a/tests/ui/invalid_pyclass_generic.stderr b/tests/ui/invalid_pyclass_generic.stderr index 1a99c4444d4..1d6b38ede87 100644 --- a/tests/ui/invalid_pyclass_generic.stderr +++ b/tests/ui/invalid_pyclass_generic.stderr @@ -1,11 +1,11 @@ -error[E0592]: duplicate definitions with name `__pymethod___class_getitem____` +error[E0592]: duplicate definitions with name `__pymethod_method___class_getitem____` --> tests/ui/invalid_pyclass_generic.rs:4:1 | 4 | #[pyclass(generic)] - | ^^^^^^^^^^^^^^^^^^^ duplicate definitions for `__pymethod___class_getitem____` + | ^^^^^^^^^^^^^^^^^^^ duplicate definitions for `__pymethod_method___class_getitem____` ... 12 | #[pymethods] - | ------------ other definition for `__pymethod___class_getitem____` + | ------------ other definition for `__pymethod_method___class_getitem____` | = note: this error originates in the macro `::pyo3::impl_::pymethods::maybe_define_fastcall_function_with_keywords` which comes from the expansion of the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -29,7 +29,7 @@ error[E0034]: multiple applicable items in scope --> tests/ui/invalid_pyclass_generic.rs:4:1 | 4 | #[pyclass(generic)] - | ^^^^^^^^^^^^^^^^^^^ multiple `__pymethod___class_getitem____` found + | ^^^^^^^^^^^^^^^^^^^ multiple `__pymethod_method___class_getitem____` found | note: candidate #1 is defined in an impl for the type `ClassRedefinesClassGetItem` --> tests/ui/invalid_pyclass_generic.rs:4:1 @@ -92,7 +92,7 @@ error[E0034]: multiple applicable items in scope --> tests/ui/invalid_pyclass_generic.rs:20:12 | 20 | pub fn __class_getitem__( - | ^^^^^^^^^^^^^^^^^ multiple `__pymethod___class_getitem____` found + | ^^^^^^^^^^^^^^^^^ multiple `__pymethod_method___class_getitem____` found | note: candidate #1 is defined in an impl for the type `ClassRedefinesClassGetItem` --> tests/ui/invalid_pyclass_generic.rs:4:1