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

Return an error state from HPyField_Store #460

Open
mattip opened this issue Nov 30, 2023 · 4 comments
Open

Return an error state from HPyField_Store #460

mattip opened this issue Nov 30, 2023 · 4 comments

Comments

@mattip
Copy link
Contributor

mattip commented Nov 30, 2023

Currently HPyField_Store has a void return value. On PyPy, the function can fail if the type has no tp_traverse.

@steve-s
Copy link
Contributor

steve-s commented Dec 1, 2023

It's about the type of the "owner" or the field value or both?

For the owner: it must be defined in the extension that calls HPyField_Store, so the user should know and it can be viewed as a bug from the same category as casting to a different struct or such, it feels to me that one could justify making it just a debug mode check instead. But that does not hold for the value, it can be of an arbitrary type, even coming from another extension...

@fangerer
Copy link
Contributor

fangerer commented Dec 1, 2023

I remember that we discussed if HPyField_Store should be an operation that may fail. Unfortunately, I cannot find any notes about that neither in the dev call minutes nor in the issues/PRs.
However, as far as I remember, we agreed that if HPyField_Store would fail, this would be a fatal error like a segmentation fault.

I'm curious: why does it fail on PyPy if tp_traverse is not there? Is it due to a sanity chech?
Anyway, I suggest that you do something like HPy_FatalError.

@mattip
Copy link
Contributor Author

mattip commented Dec 1, 2023

Yes, it is a sanity check. The Visit code in PyPy marks the memory as "still used" so it will not be collected. If there is no tp_traverse, we will get the problems that led to this comment and the subsequent discussion. While I can do a HPy_FatalError, I would prefer to give a clean exception instead by returning an error value from the function.

@mattip mattip changed the title Retrun an error state from HPyField_Store Return an error state from HPyField_Store Dec 1, 2023
@mattip
Copy link
Contributor Author

mattip commented Dec 1, 2023

Cython takes this approach: rather than segfault it will fail to import a c-extension that does not match its expectations. Accepting a segfault as a proper way to notify users about errors seems to be a last-result technique that should be avoided if possible.

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

No branches or pull requests

3 participants