[Patches] [ python-Patches-1390657 ] NotImplemented->TypeError in __add__ and __mul__

[SourceForge.net]

Patches item #1390657, was opened at 2005-12-26 16:09
Message generated for change (Comment added) made by arigo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1390657&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Armin Rigo (arigo)
Assigned to: Nobody/Anonymous (nobody)
Summary: NotImplemented->TypeError in __add__ and __mul__

Initial Comment:
This is a fix for bug #1355842, with a comprehensive
test checking that implementing a binary special method
but returning NotImplemented does indeed cause a
TypeError to be raised.  In addition to the case
reported in #1355842 (x+=y) the test found another case
where NotImplemented was just returned (A()*5), a case
where an OverflowError or ValueError was raised instead
of TypeError (A()*large_integer), and a case where an
unexpected AttributeError was raised (5*A()).

The proposed patch also reverts the changes done in svn
revisions 25556-25557 corresponding to bug #516727,
because it makes that fix unnecessary by adopting a
more radical approach.

All the problems are due to hard-to-control
interactions between the various PyTypeObject slots for
addition and multiplication (nb_add vs sq_concat,
nb_multiply vs sq_repeat).  The approach of the present
patch is to not define sq_concat, sq_repeat,
sq_inplace_concat and sq_inplace_repeat slots *at all*
for user-defined types, even if they have __add__ and
__mul__ methods.  This is the only sane solution I
found, specially with respect to the
OverflowError/ValueError problem (caused by trying to
convert the integer to a C int in order to try to call
sq_repeat).  It is also consistent with the behavior of
instances of old-style classes -- the InstanceType did
not define sq_concat and sq_repeat, either.

I'll propose a redefinition of operator.concat() and
operator.repeat() on python-dev.

----------------------------------------------------------------------

>Comment By: Armin Rigo (arigo)
Date: 2005-12-27 16:16

Message:
Logged In: YES 
user_id=4771

Attached a second patch, extending PySequence_Repeat() and
PySequence_Concat() to match more precisely their
documentation, which says that they should be equivalent
to multiplication and addition respectively, at least when
the proper arguments are sequences.

In 2.4 and HEAD, it works with new-style instances defining
__add__ or __mul__, but it does not work with old-style
instances.  If the concat_repeat_cleanup.diff patch is
checked in, then it stops working for new-style instances
too.  So the sequence_operations.diff patch cleans up the
situation by making this work in all cases.

Compatibility note: PySequence_Concat/Repeat() are mostly
never used in the core; they are essentially only used by
operator.concat() and operator.repeat().  Both patches
together should thus not break existing code and could be
considered as a bug fix.  I'd recommend that we check them
in both HEAD and the 2.4 branch.

The patch also addresses PySequence_InPlaceConcat/Repeat()
but without testing them -- they can't be tested from
Python, so they are already completely untested.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1390657&group_id=5470