GrahamDumpleton / wrapt

A Python module for decorators, wrappers and monkey patching.
BSD 2-Clause "Simplified" License
2.06k stars 231 forks source link

SystemError raised instead of TypeError with in place update operator #110

Closed DRMacIver closed 3 years ago

DRMacIver commented 7 years ago

The following code raises a TypeError:

x = []
x |= x
Traceback (most recent call last):
  File "err.py", line 4, in <module>
    x |= x
TypeError: unsupported operand type(s) for |=: 'list' and 'list'

But the following code raises a SystemError:

import wrapt

x = wrapt.ObjectProxy([])
x |= x

This fails with:

TypeError: unsupported operand type(s) for |=: 'list' and 'list'

The above exception was the direct cause of the following exception:

SystemError: PyEval_EvalFrameEx returned a result with an error set

I tested this with the current develop running native extensions on Python 3.6.1.

GrahamDumpleton commented 7 years ago

Not sure why, this only showed as problem with Python 3.5 and 3.6. And obviously if only using the C extension.

Anyway, fixed in https://github.com/GrahamDumpleton/wrapt/commit/e0d82df6dbdd3d602896fc528942ffa1902e2d5e of develop branch. Had missed propagating back error condition properly for just that one inplace operator.

Don't know how you are finding these, but efforts much appreciated.

DRMacIver commented 7 years ago

Don't know how you are finding these, but efforts much appreciated.

No one single method, honestly!

The prompt for this is that I have a possible need for a transparent object proxy in hypothesis, so I wrote some code that wrapped all provided arguments in object proxies and ran the test suite, debugging failures from which lead me to #106. #108 was by inspection. #109 was because I vaguely remembered complex numbers being an issue from the test suite so went to see how you were handling them and went "Ah ha, I bet this doesn't work". This one I wrote some tests using Hypothesis to assert expression equivalence between values and their proxies, which found this one (but also found a bug in Hypothesis. I'd be happy to submit the tests as a PR once I fix that, but they didn't find anything else very interesting after a while running)

GrahamDumpleton commented 3 years ago

Closing this old issue as assumed all fixed by more recent release of wrapt.