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

Rework exceptions to be native types #1024

Merged
merged 3 commits into from Jul 18, 2020
Merged

Conversation

davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Jul 4, 2020

This is a rework of pyo3's exception types to be more in line with the rest of the Python native types.

I introduce new exception types PyException, PyRuntimeError etc. These must be accessed through either Py<PyBaseException>, Py<PyRuntimeError> or gil-scoped references &PyBaseException , &PyRuntimeError, just like other native types.

To minimise breakage the old types Exception, RuntimeError continue to exist and behave as they did before, but I mark them deprecated with a suggestion to use the new types.

I think this is an important first step of #295 - we should then be able to make something very similar to #[pyclass] for exceptions. It should also enable #682 to be done relatively cleanly by building off these implementations.

I based this PR on the work of #686, which I think can now be closed. As well as Py<PyBaseException>, I was able to implement std::error::Error for Py<T> and &T for all the error types T.

This PR will introduce a couple of minor breaking changes, but I think it's worth it:

  • PyBorrowError and PyBorrowMutError no longer have dedicated Python types and just raise RuntimeError in Python.
    I could undo this change, however the try_borrow() would have to return &PyBorrowError and we'll take a little performance hit.
  • The imported exceptions e.g. socket::herror are now only accessible as native types; existing code using these will break slightly.

If you like this, I'll press ahead with documentation.

TODO:

  • Documentation EDIT: I'm going to write docs after some follow-up PRs, when it's easier to see what the final result looks like.

@davidhewitt davidhewitt marked this pull request as ready for review July 4, 2020 16:02
@kngwyu
Copy link
Member

kngwyu commented Jul 11, 2020

Thank you, looks good except the approach to the breaking change(i.e., Py prefix) 🤔
Is there any simpler solution? (e.g., type Exception = Py<PyException>)

@davidhewitt
Copy link
Member Author

Is there any simpler solution? (e.g., type Exception = Py)

I think any alternative, including the above, would break more user code than this PR already does.

The simplest alternative would be to embrace the breaking change and just remove the old types.

@davidhewitt
Copy link
Member Author

The simplest alternative would be to embrace the breaking change and just remove the old types.

If you think it's okay, I would be very happy to do this. It keeps the pyo3 codebase smaller and I can document these breaking changes immediately in the migration guide.

@kngwyu
Copy link
Member

kngwyu commented Jul 15, 2020

How about

type BaseException = PyBaseException;

?
This is enough for me.

@programmerjake
Copy link
Contributor

How about

type BaseException = PyBaseException;

?
This is enough for me.

sounds good to me if it's marked deprecated

@davidhewitt
Copy link
Member Author

Agreed marking the old functionality as deprecated. I'll try to finish off this PR at the weekend.

nagisa and others added 3 commits July 18, 2020 01:57
This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.
@davidhewitt
Copy link
Member Author

I've updated this PR and rebased.

I was originally intending to write more documentation as part of this PR, but now as I've been thinking about related designs such as impl std::error::Error for PyErr I want to get those into shape before writing new docs.

@davidhewitt davidhewitt force-pushed the py-exception branch 2 times, most recently from 9ba557d to 4ed9748 Compare July 18, 2020 18:06
@davidhewitt davidhewitt merged commit 41b35b8 into PyO3:master Jul 18, 2020
@davidhewitt davidhewitt mentioned this pull request Aug 13, 2020
6 tasks
@davidhewitt davidhewitt deleted the py-exception branch August 10, 2021 07:19
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

Successfully merging this pull request may close these issues.

None yet

4 participants