Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Doc/whatsnew/3.10.rst
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ Other Language Changes
the :meth:`~object.__int__` method but do not have the
:meth:`~object.__index__` method).
(Contributed by Serhiy Storchaka in :issue:`37999`.)
* If :func:`object.__ipow__` returns :const:`NotImplemented`, the operator will
correctly fall back to :func:`object.__pow__` and :func:`object.__rpow__` as expected.
(Contributed by Alex Shkop in :issue:`38302`.)


New Modules
Expand Down
42 changes: 42 additions & 0 deletions Lib/test/test_descr.py
Original file line number Diff line number Diff line change
Expand Up @@ -3893,6 +3893,48 @@ def __ipow__(self, other):
a = C()
a **= 2

def test_ipow_returns_not_implemented(self):
class A:
def __ipow__(self, other):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we're also lacking a test for when A doesn't define __ipow__. Do you mind adding one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added the test

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for the two fallbacks and ensure the same class is returning NotImplemented and implementing the regular power methods i.e something like:

class A:
    def __ipow__(self, other):
        return NotImplemented

class B(A):
    def __pow__(self, other):
        return 1
class C(A):
    def __rpow__(self, other):
        return 2

The linked bug is only exhibited when a class has an ipow slot but when called it returns NotImplemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understood correctly what you meant, but I updated the test to also test the __pow__ fallback.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, what you have now looks good. What I meant was that to catch the original bug/properly be a regression test, the class has to look like:

class A:
    def __ipow__(self, _):
        return NotImplemented
    def __pow__(self, other):
        ...

if the class looks like

class A:
    def __pow__(self, other):
        ...

then the old logic returns a false here and correctly goes to the non-inplace version of power anyway:

cpython/Objects/abstract.c

Lines 1162 to 1163 in 8e19c8b

if (Py_TYPE(v)->tp_as_number &&
Py_TYPE(v)->tp_as_number->nb_inplace_power != NULL) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, you are right. Thanks!

return NotImplemented

class B(A):
def __rpow__(self, other):
return 1

class C(A):
def __pow__(self, other):
return 2
a = A()
b = B()
c = C()

a **= b
self.assertEqual(a, 1)

c **= b
self.assertEqual(c, 2)

def test_no_ipow(self):
class B:
def __rpow__(self, other):
return 1

a = object()
b = B()
a **= b
self.assertEqual(a, 1)

def test_ipow_exception_text(self):
x = None
with self.assertRaises(TypeError) as cm:
x **= 2
self.assertIn('unsupported operand type(s) for **=', str(cm.exception))

with self.assertRaises(TypeError) as cm:
y = x ** 2
self.assertIn('unsupported operand type(s) for **', str(cm.exception))

def test_mutable_bases(self):
# Testing mutable bases...

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If :func:`object.__ipow__` returns :const:`NotImplemented`, the operator will correctly fall back to :func:`object.__pow__` and :func:`object.__rpow__` as expected.
30 changes: 22 additions & 8 deletions Objects/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -910,8 +910,9 @@ ternary_op(PyObject *v,
if (z == Py_None)
PyErr_Format(
PyExc_TypeError,
"unsupported operand type(s) for ** or pow(): "
"unsupported operand type(s) for %.100s: "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! Really weird that op_name went unused so long. Would you mind adding a test for this like:

        x = None
        with self.assertRaises(TypeError) as cm:
            x **= 2
        self.assertIn('unsupported operand type(s) for **=', str(cm.exception))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added a new test

"'%.100s' and '%.100s'",
op_name,
Py_TYPE(v)->tp_name,
Py_TYPE(w)->tp_name);
else
Expand Down Expand Up @@ -1064,6 +1065,24 @@ binary_iop(PyObject *v, PyObject *w, const int iop_slot, const int op_slot,
return result;
}

static PyObject *
ternary_iop(PyObject *v, PyObject *w, PyObject *z, const int iop_slot, const int op_slot,
const char *op_name)
{
PyNumberMethods *mv = v->ob_type->tp_as_number;
if (mv != NULL) {
ternaryfunc slot = NB_TERNOP(mv, iop_slot);
if (slot) {
PyObject *x = (slot)(v, w, z);
if (x != Py_NotImplemented) {
return x;
}
Py_DECREF(x);
}
}
return ternary_op(v, w, z, op_slot, op_name);
}

#define INPLACE_BINOP(func, iop, op, op_name) \
PyObject * \
func(PyObject *v, PyObject *w) { \
Expand Down Expand Up @@ -1159,13 +1178,8 @@ PyNumber_InPlaceRemainder(PyObject *v, PyObject *w)
PyObject *
PyNumber_InPlacePower(PyObject *v, PyObject *w, PyObject *z)
{
if (Py_TYPE(v)->tp_as_number &&
Py_TYPE(v)->tp_as_number->nb_inplace_power != NULL) {
return ternary_op(v, w, z, NB_SLOT(nb_inplace_power), "**=");
}
else {
return ternary_op(v, w, z, NB_SLOT(nb_power), "**=");
}
return ternary_iop(v, w, z, NB_SLOT(nb_inplace_power),
NB_SLOT(nb_power), "**=");
}


Expand Down