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

Add 'except' values for various function types #1980

Merged
merged 1 commit into from Nov 6, 2017

Conversation

jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Nov 5, 2017

No description provided.

@scoder
Copy link
Contributor

scoder commented Nov 5, 2017

Hmm, is this documented anywhere? I mean, it looks reasonable, but I wouldn't be so sure that there are no exceptions from these rules. Should we maybe use except?, just for safety? The difference isn't big in practice.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 5, 2017

The fields in the PyTypeObject* structure do not explicitly document exception values. See for example tp_as_mapping->mp_length which does not mention exception handling. It does say that it has the same signature as PyMapping_Length() and PyMapping_Length does document that -1 signals an exception.

If you look in the sources for PyMapping_Length, you will see that it just calls tp_as_mapping->mp_length without any special exception handling: exceptions are just passed through.

Not sure whether you consider that sufficient or not...

@scoder
Copy link
Contributor

scoder commented Nov 5, 2017

I'm more concerned about people using these typedef-ed names outside of the PyTypeObject struct, if only because "they are there". I mean, the chances of people using them for something that does not do any error handling at all probably aren't very high, but there is always a risk of breaking someone's code out there.

The problem is really that you are changing the typedefs here, and not the PyTypeObject struct.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 6, 2017

there is always a risk of breaking someone's code out there.

Suppose we add an except value to these types, that would break compilation for those people (since the except value is part of the function signature, right?). So they will need to change their code anyway.

I mean, the chances of people using them for something that does not do any error handling at all probably aren't very high

Indeed, that doesn't seem very likely. First of all, you already need to know that those function types exist at all.

@scoder scoder added this to the 0.28 milestone Nov 6, 2017
@scoder
Copy link
Contributor

scoder commented Nov 6, 2017

You got me convinced that it fixes more than it might break. Thanks for your contribution(s), I appreciate it.

@scoder scoder merged commit ddf2486 into cython:master Nov 6, 2017
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

2 participants