docs: fix error condition in custom type caster example #5416
Conversation
| @@ -64,7 +64,7 @@ type is explicitly allowed. | |||
| value.long_value = PyLong_AsLong(tmp); | |||
| Py_DECREF(tmp); | |||
| /* Ensure return code was OK (to avoid out-of-range errors etc) */ | |||
| return !(value.long_value == -1 && !PyErr_Occurred()); | |||
| return !(value.long_value == -1 && PyErr_Occurred()); | |||
There was a problem hiding this comment.
The error needs to be cleared, too.
A better version of custom.rst file is buried in here:
The ADL approach under #3862 didn't work out, so the parts referring to that need to be removed.
Looking at it with a fresh eye, I think there is still a bug in the cast() implementation: missing error handling again.
Let me see if I can rescue custom.rst and tests/test_docs_advanced_cast_custom.cpp,py from the old PR. Maybe this weekend.
There was a problem hiding this comment.
Or please let me know if you want to help with the rescuing.
There was a problem hiding this comment.
Well, I'd be happy to give a shot at fully sanitizing the custom type caster example. I can add the other missing error checks to this PR, also including the associated modifications of the test.
There was a problem hiding this comment.
Awesome, please tag me here when this PR is ready for re-review. It'll be great if you could adopt the doc changes from 3862 as well (excluding the ADL-related bits).
|
This PR should be resolved by #5450 since the example was completely changed. |
Description
I found a mistake in the documentation for custom type_casters, which I believe hasn't been covered by an open issue or PR yet. The
loadfunction of a type caster struct is supposed to returntrueupon success. However, the condition!(value.long_value == -1 && !PyErr_Occurred())whose value is returned, evaluates to false if no error occurred and the Python float value is
-1. The negation of the result ofPyErr_Occurred()needs to be removed.Suggested changelog entry: