-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
richcompare suppresses all errors raised from descriptors of comparison special methods #131151
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
Comments
I'm asking fellow core devs that may have an idea why it was designed like this @JelleZijlstra @iritkatriel @encukou @vstinner |
Yes, it should probably suppress AttributeError only; I suspect it was simply written this way because it's easier. I think we can probably afford to change this, but in 3.14 only. Also, we should make sure to do this consistently for all special methods with fallbacks. |
So far I think that there are sort of several categories of special methods, at least in terms of internal details, and I think it would be a larger effort to try to get similar "fallback" behavior from raising
Honestly, I would actually advocate against fallbacks from raising AttributeError from a descriptor (in principle). It's not really possible to know where an AttributeError came from, or whether it was intended to trigger a fallback. Just my opinion, but I think returning To me it would be a compromise to not have other errors cause fallback. For some context, the embarrassing way I discovered this was a virtually infinite recursion error in a descriptor I made for |
This may have been addressed by #131588, but have not dug too deep into the changes. |
Bug report
Bug description:
This is reporting possibly incorrect suppression of all errors generated by descriptors for the comparison special methods
__eq__
,__lt__
, etc. (and possibly some other special methods). This only affects errors during the__get__
stage, while errors from the actual call propagate as expected.I say possibly incorrect because I can see this being one of those sensitive underbellies that could affect existing behavior for some libraries. However, this seems inconsistent relative to some other special methods like
__contains__
,__bool__
, etc., and since it suppresses all errors like SystemExit, MemoryError, KeyboardInterrupt, etc. it can hide underlying problems or behaviors that are hard to diagnose.Minimal example
Minimal demonstration of the issue below is a descriptor which always raises an exception during
__get__
. The result of comparison through==
returns False instead of raising the error. The type of error raised from__get__
is not important, it could have been fromexit()
or anything else down the stack. However, other special methods like__bool__
do raise the exception, as does directly accessing__eq__
as a method.Suspected cause and proposed resolution
The source of this behavior appears to be in
Objects/typeobject.c
functionslot_tp_richcompare
. Currently if an error occurs while getting the comparison method from a descriptor the error is cleared andNotImplemented
is returned. Looking at some of the other special methods it seems like there is some inconsistency, some check forPyErr_Occurred
and propagate the exception (EGslot_nb_bool
), while others clear the error and default to some other action (E.G.slot_tp_repr
).A couple experimental resolutions were attempted to look at the consequence. The first was to treat errors similar to
slot_nb_bool
, propagating them instead of suppressing.However, this caused a failed test which seems to be checking that raising
AttributeError
should lead to suppression and default comparison. It is not clear to me if that actually should be the expected behavior, and the comment in the test seems to indicate that it may actually be testing for some internal regression:But, as a compromise the following alternative checking specifically for
AttributeError
passes all tests enabled for my current platform. Depending on feedback here I am willing to contribute a pull request once I have a better idea as to what is expected/desired in these cases.CPython versions tested on:
3.14, CPython main branch
Operating systems tested on:
Linux
Linked PRs
The text was updated successfully, but these errors were encountered: