Skip to content

split PyFunctionArgument to specialize Option #5002

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

Merged
merged 3 commits into from
Mar 28, 2025
Merged

Conversation

Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Mar 23, 2025

Adds const generic parameter to PyFunctionArgument to allow specialization. This allows Option wrapped extraction of some types that can't implement FromPyObject(Bound) because they require a holder to borrow from, most notably &T for T: PyClass.

  • requires modifying the argument types, to elide lifetimes
  • only works for "outest" Option, inner Options will use the FromPyObject impl

ToDo:

  • add some tests

Closes #4965

Adds const generic parameter to `PyFunctionArgument` to allow specialization.
This allows `Option` wrapped extraction of some types that can't implement
`FromPyObject(Bound)` because they require a `holder` to borrow from, most
notably `&T` for `T: PyClass`.

Closes PyO3#4965
@davidhewitt
Copy link
Member

davidhewitt commented Mar 28, 2025

Given that this is significantly more self-contained than a full implementation of #5003 would be, and this all changes after #4390 anyway, I pushed a test and newsfragment here. Will merge and get on with prepping a release. Thanks!

}

impl TypeExt for syn::Type {
fn elide_lifetimes(mut self) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should add a FIXME somewhere here, and say that we want to remove this again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to leave it as-is, we can see how brittle it is in practice (or maybe we end up removing this again when it's unused).

@davidhewitt davidhewitt marked this pull request as ready for review March 28, 2025 18:47
@davidhewitt davidhewitt added this pull request to the merge queue Mar 28, 2025
@davidhewitt
Copy link
Member

Oops forgot to click merge 🤦

Merged via the queue into PyO3:main with commit d85a02d Mar 28, 2025
49 checks passed
@Icxolu Icxolu deleted the fix/4965 branch March 28, 2025 21:36
Icxolu added a commit to Icxolu/pyo3 that referenced this pull request Apr 1, 2025
Icxolu added a commit to Icxolu/pyo3 that referenced this pull request Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.24: Option<&str> not a valid function argument anymore?
2 participants