Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Useless conversion error when using PyResult<PyObject> in pyo3 #14272

Open
edlng opened this issue Feb 21, 2025 · 7 comments
Open

Useless conversion error when using PyResult<PyObject> in pyo3 #14272

edlng opened this issue Feb 21, 2025 · 7 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@edlng
Copy link

edlng commented Feb 21, 2025

Summary

In rustc v1.84 we didn't have this issue but after upgrading to 1.85, we started getting this error. Downgrading back to 1.84 seemed to fix the issue.

Reproducer

For example I tried this dummy code:

  #[pyfunction]
  fn sum_as_string(py: Python) -> PyResult<PyObject> {
      let py_dict = PyDict::new_bound(py);

      Ok(py_dict.into_py(py))
  }

I expected to see no warnings or errors

Instead, this happened:

error: useless conversion to the same type: `pyo3::PyErr`
   --> src/lib.rs:160:45
    |
160 |     fn sum_as_string(py: Python) -> PyResult<PyObject> {
    |                                             ^ help: consider removing
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion

Version


Additional Labels

No response

@edlng edlng added the C-bug Category: Clippy is not doing the correct thing label Feb 21, 2025
@LilyFoote
Copy link

I'm hitting this too and I've used https://github.com/rust-lang/cargo-bisect-rustc to bisect to rust-lang/rust@19e75f4.

cargo bisect-rustc --start=1.84.0 --end=1.85.0 -c clippy -- clippy -- -F clippy::useless_conversion

I'm not immediately sure how to bisect further, but I'll spend a bit more time this evening to see if I can work it out.

@LilyFoote
Copy link

I think I managed to bisect to 2ddfc92, but now it's passing and I don't know why...

@LilyFoote
Copy link

Assuming that's the right commit, I now suspect this requires a fix in PyO3's #[pyfunction] and #[pymethods] proc macros, though I'm a little surprised we didn't pick up this before 1.85 was released.

@LilyFoote
Copy link

Actually, this does appear to be fixed on v0.23.5 of PyO3 - I guess my code was using an older version. @edlng I suspect this is also the case for you.

@samueltardieu
Copy link
Contributor

Assuming that's the right commit, I now suspect this requires a fix in PyO3's #[pyfunction] and #[pymethods] proc macros, though I'm a little surprised we didn't pick up this before 1.85 was released.

As the author of the commit you identified, which indeed lints more cases of useless conversions than it did before, I also had a look and could not reproduce it, and suspected a proc macro problem as well. I haven't looked at the pyfunction or pymethods proc macros, but sometimes macro writers use incorrect spans during expansion (for example through quote::quote_spanned!()) which prevent tools such as Clippy from noticing that the linted code comes from expansion of an external macro. This can lead to inappropriate diagnostics.

If you can exhibit a minimal compilable reproducer that I can use, I'll look at it, unless you consider the problem fixed already, in which case this issue can be closed.

@LilyFoote
Copy link

I think there might be more to do in PyO3, but right now I don't think there's anything wrong in Clippy.

@edlng
Copy link
Author

edlng commented Feb 27, 2025

Thanks! I'll take a look again in the morning and close the issue once I confirm it is fixed.

LilyFoote added a commit to PyO3/pyo3 that referenced this issue Feb 27, 2025
Using `#[allow(clippy::useless_conversion)]` still causes failures if
the PyO3 user uses `#[forbid(clippy::useless_conversion)]`. Instead we
can ensure the `into` call is not within a `quote_spanned!` block,
allowing clippy to know this is macro code.

rust-lang/rust-clippy#14272 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests

3 participants