Issue1775078

classification
Title: Fix binary operations on str/unicode/lists/tuples
Type: Severity: normal
Components: None Versions:
Milestone:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: pjenvey
Priority: normal Keywords: patch

Created on 2007-08-16.00:28:46 by pjenvey, last changed 2007-08-31.21:48:33 by pjenvey.

Files
File name Uploaded Description Edit Remove
subclass-binop_r3416.diff pjenvey, 2007-08-16.00:28:46 patch against trunk r3416
Messages
msg2804 (view) Author: Philip Jenvey (pjenvey) Date: 2007-08-16.00:28:46
These 4 classes have binary op bugs against subclasses that implement their own custom __add__/__radd__/__mul__ etc methods (see the example below)

PyObject._binop_rule calls into the __xxx__/__rxxx__ methods when at least one of the objects involved isn't a built in type. It expects those methods to return Py.NotImplemented when the binary operation doesn't succeed -- signaling that _binop_rule can try a different means (e.g. if __add__ fails, try the other object's __radd__)

The str/unicode/list/tuple methods weren't marked as binary in their .expose files; fixing this ensures that  Py.NotImplemented is automatically returned to _binop_rule when these methods return null.

Some of these methods were taking it upon themselves to incorrectly throw an exception when the bin op failed; doing so short circuits _binop_rule, preventing it from trying different means. These have also been fixed

The methods that did correctly return null would end up (without being defined as a binary op in the expose file) returning a null back into the Python object space, causing evilness such as:

Jython 2.3a0 on java1.5.0_07
Type "copyright", "credits" or "license" for more information.
>>> class Hi(object): 
...     def __radd__(self, o):
...             return '%r + Hi()' % o
... 
>>> a = [1, 2] + Hi()
>>> a
Traceback (innermost last):
  File "<console>", line 1, in ?
NameError: a

This patch also corrects the TypeError messages used when certain binary ops fail; they've been changed to match Python 2.5.1's.

I also added a subclass_binop test to test_descr.py to ensure the return values for + and * ops, and their TypeError messages

List's __iadd__/__imul__ don't have to be defined as binary ops in the .expose file because they don't ever utilize _binop_rule
msg2805 (view) Author: Philip Jenvey (pjenvey) Date: 2007-08-31.01:32:44
There's a problem with this change, returning null instead of throwing TypeErrors in the binop methods breaks calling them directly from python, which is expected to work from some tests:

>>> 'abc'.__mul__('') 
NotImplemented

_binop_rule may need to be changed to catch TypeErrors (maybe any python exception) and handle it as if NotImplemented was returned, instead
msg2806 (view) Author: Philip Jenvey (pjenvey) Date: 2007-08-31.19:52:31
Actually, returning NotImplemented here is inline with pypy

PyPy 1.0.0 in StdObjSpace on top of Python 2.5.1 (startuptime: 11.98 secs)
>>>> 'abc'.__mul__('')
NotImplemented

The test that fails with this patch applied:

        self.checkraises(TypeError, 'abc', '__mul__', '')

pypy modifies this to:

        self.checkopraises(TypeError, operator.mul, 'abc', '')

which we'd pass
msg2807 (view) Author: Philip Jenvey (pjenvey) Date: 2007-08-31.21:46:44
applied in r3461
msg2808 (view) Author: Philip Jenvey (pjenvey) Date: 2007-08-31.21:48:33
r3461 also includes that pypy string_test modification I previously mentioned
History
Date User Action Args
2007-08-16 00:28:46pjenveycreate