classification
Title: object.__new__ argument calling autodetection faulty
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Ringding, Trundle, aronacher, benjamin.peterson, doko, jdemeyer, larry, ned.deily, ppperry, prologic, python-dev, scoder, sebastinas, serhiy.storchaka, thansen, vstinner
Priority: normal Keywords:

Created on 2009-02-19 20:34 by aronacher, last changed 2018-08-10 02:07 by ppperry.

Files
File name Uploaded Description Edit
update_one_slot.patch Trundle, 2009-03-24 20:25
update_one_slot2-3.x.patch serhiy.storchaka, 2016-11-17 21:28 review
update_one_slot2-2.7.patch serhiy.storchaka, 2016-11-17 21:28 review
UnsupportedOperation-bases-order.patch serhiy.storchaka, 2016-12-07 10:51 review
Messages (27)
msg82497 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-02-19 20:34
In 2.6 a deprecation warning was added if `object.__new__` was called
with arguments.  Per se this is fine, but the detection seems to be faulty.

The following code shows the problem:

>>> class A(object):
...     def __new__(self):
...         raise TypeError('i do not exist')
... 
>>> class B(A):
...     __new__ = object.__new__
...     def __init__(self, x):
...         self.x = x
... 
>>> B(1)
__main__:1: DeprecationWarning: object.__new__() takes no parameters
<__main__.B object at 0x88dd0>

In the `B` case `__new__` is not overridden (in the sense that it
differs from object.__new__) but `__init__` is.  Which is the default
behaviour.  Nonetheless a warning is raised.

I used the pattern with the "__new__ switch" to achieve a
cStringIO.StringIO behavior that supports typechecks:  IterIO() returns
either a IterI or IterO object, both instances of IterIO so that
typechecks can be performed.

Real-world use case here:
http://dev.pocoo.org/projects/werkzeug/browser/werkzeug/contrib/iterio.py
msg82505 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-02-19 22:48
The problem seems to be caused by tp_new being slot_tp_new which then
invokes whatever __new__ in the class dict is.

I'm not so sure what would be the solution to this.  One could of course
check if tp_new is either object_new or slot_tp_new and in the latter
case check if the class dict's __new__ item is object_new...
msg84112 - (view) Author: Andreas Stührk (Trundle) * Date: 2009-03-24 20:25
I think the real problem here is `update_one_slot` and not `object_new`. It
is impossible to set "__new__" to a PyCFunction inside Python code, which
may be a feature, but is in fact very irritating.

For example the following snippet:

>>> class Dict(dict): __new__ = object.__new__
...
>>> Dict.__new__ is object.__new__
True
>>> Dict()
{}

I would rather expect this behaviour (or at least that Dict.__new__ is not
object.__new__):

>>> Dict.__new__ is object.__new__
True
>>> Dict()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: object.__new__(Dict) is not safe, use dict.__new__()

The attached patch leads to that behaviour, which also fixes the argument
calling autodetection of `object.__new__`.
msg85828 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-09 21:46
I'm sorry, I don't have any opinion on this.
msg86111 - (view) Author: Andreas Stührk (Trundle) * Date: 2009-04-18 08:56
The problem is that `type_setattro()` sets the new "__new__" attribute
in the type's dict (through `PyObject_GenericSetAttr()`), but the
corresponding slot will never be updated if the new "__new__" is a
PyCFunction.

The affected code in `update_one_slot()` was added by Guido van Rossum
in r28090, so maybe he would like to comment on that.
msg86702 - (view) Author: Andreas Stührk (Trundle) * Date: 2009-04-27 21:55
See also issue #1694663.
msg145289 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2011-10-10 00:03
I think it needs tests.
msg281064 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-17 21:28
Here are updated patches with tests for 3.x and 2.7.
msg282224 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-02 06:43
New changeset a37cc3d926ec by Serhiy Storchaka in branch '2.7':
Issue #5322: Fixed setting __new__ to a PyCFunction inside Python code.
https://hg.python.org/cpython/rev/a37cc3d926ec
msg282225 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-02 06:44
Will commit to 3.5-3.7 after releasing 3.6.0.
msg282605 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-07 09:28
New changeset 1f31bf3f76f5 by Serhiy Storchaka in branch '3.5':
Issue #5322: Fixed setting __new__ to a PyCFunction inside Python code.
https://hg.python.org/cpython/rev/1f31bf3f76f5

New changeset 747de8acb7e4 by Serhiy Storchaka in branch '3.6':
Issue #5322: Fixed setting __new__ to a PyCFunction inside Python code.
https://hg.python.org/cpython/rev/747de8acb7e4

New changeset 9605c558ab58 by Serhiy Storchaka in branch 'default':
Issue #5322: Fixed setting __new__ to a PyCFunction inside Python code.
https://hg.python.org/cpython/rev/9605c558ab58
msg282612 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-12-07 10:37
test_file started to crash after the change "Issue #5322: Fixed setting __new__ to a PyCFunction inside Python code." :-/ (so all buildbots became red.)

Can someone fix it or revert it? (Sorry, I don't have the bandwith right to investigate the crash.)
msg282615 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-07 10:51
The test is fixed if change order of base classes of UnsupportedOperation. This is rather a workaround, we should find more general fix.
msg282617 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-07 11:32
New changeset 4a610bc8577b by Serhiy Storchaka in branch '3.5':
Change order of io.UnsupportedOperation base classes.
https://hg.python.org/cpython/rev/4a610bc8577b
msg282852 - (view) Author: Tobias Hansen (thansen) Date: 2016-12-10 13:51
This change breaks backward compatibility in Python 2.7. This is the example that also broke in #25731. In that case the change was reverted. See https://bugs.python.org/issue25731#msg262922

$ cat foo.pxd 
cdef class B:
    cdef object b
$ cat foo.pyx 
cdef class A:
    pass

cdef class B:
    def __init__(self, b):
        self.b = b
$ cat bar.py
from foo import A, B

class C(A, B):
    def __init__(self):
        B.__init__(self, 1)

C()
$ cython foo.pyx && gcc -I/usr/include/python2.7 -Wall -shared -fPIC -o foo.so foo.c
$ python -c 'import bar'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "bar.py", line 7, in <module>
    C()
TypeError: foo.A.__new__(C) is not safe, use foo.B.__new__()
msg282942 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2016-12-11 20:15
Here is more minimal breaking example. This clearly shows that this patch breaks backwards compatibility.

```
$ cat obj.pyx
cdef class OBJ(object):
    pass

$ ipython
Python 2.7.13rc1 (default, Dec 11 2016, 14:21:24) 
Type "copyright", "credits" or "license" for more information.

IPython 5.1.0 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: import pyximport; pyximport.install()
Out[1]: (None, <pyximport.pyximport.PyxImporter at 0x7f8e8c585910>)

In [2]: import obj

In [3]: class X(obj.OBJ, dict):
   ...:     pass
   ...: 

In [4]: X()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-a7d4f7b89654> in <module>()
----> 1 X()

TypeError: obj.OBJ.__new__(X) is not safe, use dict.__new__()
```
msg282969 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-12 07:59
Does changing the order of base classes help or there is an unavoidable conflict?
msg282974 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2016-12-12 09:03
https://trac.sagemath.org/ticket/22037 reports about another regression.
msg282976 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2016-12-12 09:42
@serhiy.storchaka: yes, changing the order of the base classes fixes the issue with __new__. Also manually assigning __new__ works, like

class C(A, B):
    __new__ = B.__new__

What is broken by this patch is only the auto-detection of which __new__ (really, which tp_new) should be called.

@doko: not "another regression", it's exactly the one that we are talking about.
msg282979 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-12 10:52
Thank you Jeroen. It looks to me that all problems can be resolved by reordering base classes and making Cython not generating trivial __new__. But that is possible only in new Python version. In maintained versions we should keep the old behavior for backward compatibility even if it contradicts normal rules for method resolution and the behavior of Python classes. We should find other solution for making explicit __new__ assigning working.
msg282999 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2016-12-12 13:53
Wouldn't it be possible to fix assignment of __new__ without breaking backwards compatibility (and then apply the same patch for all Python versions)? I have a feeling that breaking the auto-detection of tp_new is a new bug introduced by this patch and not a fundamental feature needed to fix assignment of __new__.
msg283170 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-14 07:33
New changeset 5315db3171b0 by Benjamin Peterson in branch '2.7':
revert a37cc3d926ec (#5322)
https://hg.python.org/cpython/rev/5315db3171b0
msg283178 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-12-14 09:17
Since this change seems to break the backward compatibility, is it safe to apply it to Python 3.5.x and Python 3.6.x? The bug was reported in 2009, 7 years ago. Can the fix wait for Python 3.7?

test_file contains code which worked well before the change and started to crash after the change. If it occurs for an application, I expect users to be unhappy of getting such "behaviour change" in a minor release, no?

--

Is it possible to prevent the crash of test_file without modifying its code (without the change 4a610bc8577b "Change order of io.UnsupportedOperation base classes")? Sorry, I didnd't follow this issue.
msg283209 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-14 17:57
New changeset f89ef18f9824 by Serhiy Storchaka in branch '2.7':
Issue #5322: Restored tests for __new__.
https://hg.python.org/cpython/rev/f89ef18f9824

New changeset 06e4b9f2e4b0 by Serhiy Storchaka in branch '3.5':
Revert changeset 1f31bf3f76f5 (issue5322) except tests.
https://hg.python.org/cpython/rev/06e4b9f2e4b0
msg283243 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-12-15 06:07
BTW, at least for #25731, I think the right approach in the MI case is to synthesize a __new__ on the subclass that calls the solid base __new__.
msg283245 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-15 06:21
Yes, it was what the patch did by setting tp_new to slot_tp_new. The problem is that the same code is used for inherited __new__ and assigned in class body. It is hard to distinguish between these cases.

In any case I think that Cython shouldn't generate trivial __new__. This will help to change the order of __new__ resolution at least in 3.7.
msg284637 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2017-01-04 14:04
It worries me that nothing in the Python docs nor in any PEP describes how tp_new is inherited. In my opinion, ​this patch makes a significant change which should be subject to a PEP. However, neither the old nor new behaviour is described anywhere. This also makes it harder to argue which behaviour is correct.
History
Date User Action Args
2018-08-10 02:07:42ppperrysetnosy: + ppperry
2018-08-08 21:13:24ppperrysettitle: Python 2.6 object.__new__ argument calling autodetection faulty -> object.__new__ argument calling autodetection faulty
2017-01-04 14:04:23jdemeyersetmessages: + msg284637
2016-12-15 06:21:03serhiy.storchakasetnosy: + scoder
messages: + msg283245
2016-12-15 06:07:36benjamin.petersonsetmessages: + msg283243
2016-12-14 17:58:15serhiy.storchakasetpriority: release blocker -> normal
2016-12-14 17:57:20python-devsetmessages: + msg283209
2016-12-14 09:17:29vstinnersetmessages: + msg283178
2016-12-14 07:33:48python-devsetmessages: + msg283170
2016-12-12 15:08:48gvanrossumsetnosy: - gvanrossum
2016-12-12 13:53:09jdemeyersetmessages: + msg282999
2016-12-12 10:52:32serhiy.storchakasetpriority: normal -> release blocker
nosy: + ned.deily, larry
messages: + msg282979

2016-12-12 09:42:34jdemeyersetmessages: + msg282976
2016-12-12 09:03:32dokosetnosy: + doko
messages: + msg282974
2016-12-12 08:00:28serhiy.storchakasetversions: + Python 2.7
2016-12-12 08:00:17serhiy.storchakasetkeywords: - patch
stage: patch review ->
2016-12-12 07:59:43serhiy.storchakasetmessages: + msg282969
2016-12-11 20:17:51pitrousetnosy: - pitrou
2016-12-11 20:15:48jdemeyersetnosy: + jdemeyer
messages: + msg282942
2016-12-10 13:51:17thansensetnosy: + thansen
messages: + msg282852
2016-12-07 11:32:27python-devsetmessages: + msg282617
2016-12-07 10:51:19serhiy.storchakasetfiles: + UnsupportedOperation-bases-order.patch

messages: + msg282615
2016-12-07 10:37:17serhiy.storchakalinkissue28884 dependencies
2016-12-07 10:37:03vstinnersetnosy: + vstinner
messages: + msg282612
2016-12-07 09:28:14python-devsetmessages: + msg282605
2016-12-02 06:44:36serhiy.storchakasetmessages: + msg282225
versions: - Python 2.7
2016-12-02 06:43:06python-devsetnosy: + python-dev
messages: + msg282224
2016-11-30 07:26:37serhiy.storchakasetassignee: serhiy.storchaka
2016-11-17 21:28:37serhiy.storchakasetfiles: + update_one_slot2-2.7.patch
2016-11-17 21:28:19serhiy.storchakasetfiles: + update_one_slot2-3.x.patch

components: + Interpreter Core
versions: + Python 2.7, Python 3.5, Python 3.6, Python 3.7, - Python 2.6
nosy: + serhiy.storchaka

messages: + msg281064
stage: patch review
2011-10-10 00:03:54benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg145289
2009-05-06 10:53:32Ringdingsetnosy: + Ringding
2009-04-27 21:55:44Trundlesetmessages: + msg86702
2009-04-26 19:16:47Trundlesetnosy: + gvanrossum
2009-04-18 08:56:09Trundlesetmessages: + msg86111
2009-04-09 21:46:45pitrousetassignee: pitrou -> (no value)
messages: + msg85828
2009-04-09 20:08:01georg.brandlsetassignee: pitrou

nosy: + pitrou
2009-03-24 22:19:35sebastinassetnosy: + sebastinas
2009-03-24 20:25:09Trundlesetfiles: + update_one_slot.patch

nosy: + Trundle
messages: + msg84112

keywords: + patch
2009-02-26 19:25:21prologicsetnosy: + prologic
2009-02-19 22:48:04aronachersetmessages: + msg82505
2009-02-19 20:34:23aronachercreate