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

Improve diagnostic for invalid function passed to from_py_with #4763

Open
davidhewitt opened this issue Dec 3, 2024 · 2 comments
Open

Improve diagnostic for invalid function passed to from_py_with #4763

davidhewitt opened this issue Dec 3, 2024 · 2 comments

Comments

@davidhewitt
Copy link
Member

Just looking at #4748, and the compile error on invalid from_py_with function.

I wonder if we can somehow wrap the function up in a trait and then use diagnostic::on_unimplemented to give a more user-friendly error message for that case? 🤔

@mejrs
Copy link
Member

mejrs commented Dec 4, 2024

The macro currently emits something like

        let ret = function(::pyo3::impl_::extract_argument::from_py_with(
            unsafe {
                ::pyo3::impl_::extract_argument::unwrap_required_argument(output[0usize].as_deref())
            },
            "bytes",
            from_py_with_0 as fn(_) -> _,
        )?);

If you use a type annotation rather than an as cast, like

        let from_py_with_0: fn(_) -> _ = from_py_with_0;
        let ret = function(::pyo3::impl_::extract_argument::from_py_with(
            unsafe {
                ::pyo3::impl_::extract_argument::unwrap_required_argument(output[0usize].as_deref())
            },
            "bytes",
            from_py_with_0,
        )?);

then you get an error that at least says

    = note: expected fn pointer `fn(&pyo3::Bound<'_, PyAny>) -> Result<_, PyErr>`
               found fn pointer `fn(&pyo3::Bound<'_, PyBytes>) -> Vec<u8>`

I wonder if we can somehow wrap the function up in a trait and then use diagnostic::on_unimplemented

I'm not a fan of this approach, it seems like a better fix for this sort of thing is to make changes in the compiler itself to make this kind of error message better.

@davidhewitt
Copy link
Member Author

👍 true, given it's a diagnostic it would be good enough to get an improvement into latest stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants