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 error diagnostic for exception-spec mismatch #5547

Merged
merged 3 commits into from Jul 21, 2023

Conversation

da-woods
Copy link
Contributor

It seems like the most common Cython 3 snag is the change in function exception behaviour. This PR adds some extra diagnostics to help people navigate this change, and a general mechanism for adding this kind of diagnostics.

It needs some tests, but I wasn't to see what tests fail to tell me where to put them.

It seems like the most common Cython 3 snag is the change in
function exception behaviour. This PR adds some extra diagnostics
to help people navigate this change, and a general mechanism
for adding this kind of diagnostics.

It needs some tests, but I wasn't to see what tests fail
to tell me where to put them.
Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

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

I'm very much in favour of this. It's a common problem and we should provide as much help as possible to users who want to adapt or migrate.

copied_src_type.exception_value = self.base_type.exception_value
if self.base_type.pointer_assignable_from_resolved_type(copied_src_type):
# the only reason we can't assign is because of exception incompatibility
msg = "Exception specifications are incompatible."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure "exception specification" is something we use in the documentation, so it might be difficult to look up. I think we usually talk about "exception values", maybe "exception return values", although that's more constrained than "specification".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use it a few times in the docs (and at least of of those times was written by someone who wasn't me)

https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#language-basics

cdef functions that are not extern are implicitly declared with a suitable exception specification for the return type

https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html

There is an easy-to-encounter performance pitfall here with nogil functions with an implicit exception specification of except *.

https://cython.readthedocs.io/en/latest/src/userguide/nogil.html

Be aware that a function with an except * exception specification (typically functions returning void) will be expensive to call because Cython will need to temporarily reacquire the GIL after every call to check the exception state. Most other exception specifications are cheap to handle in a nogil block since the GIL is only acquired if an exception is actually thrown.

In most cases this is efficient since Cython is able to use the function’s exception specification to check for an error

So I'm not feeling too bad about it.

However; I'll replace it with "exception values"

@da-woods da-woods added this to the 3.0 milestone Jul 20, 2023
@da-woods da-woods merged commit 572b640 into cython:master Jul 21, 2023
73 of 75 checks passed
@da-woods da-woods deleted the improve-func-ptr-error-diagnostics branch July 21, 2023 07:00
@da-woods da-woods modified the milestones: 3.0, 3.0.1 Jul 25, 2023
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.

None yet

2 participants