Alexander Belopolsky <belopol...@users.sourceforge.net> added the comment:

On Mon, Jun 28, 2010 at 7:32 PM, Antoine Pitrou <rep...@bugs.python.org> wrote:
>> My patch attempts to emulate 2.x PyInstance_NewRaw with a call to
>> tp_alloc.
>
> This is certainly the wrong thing to do. You could at least try
> PyBaseObject_Type.tp_new (see object_new() in typeobject.c), but
> even this is not necessarily right (for example if the
> class is derived from an extension type defining its own tp_new).

Well, this is not so obviously wrong as you make it sound.  If you look closer 
at object_new(), you will see that in sane situations it reduces to 
type->tp_alloc(type, 0) call.  Remember, this is called in very special 
circumstances: when unpickling old style class instances from a 2.x generated 
pickle.  No arguments are available for tp_new call, so excess_args check does 
not apply.  The other check is for an ABC, but the situation where an old-style 
class becomes an ABC in 2.x to 3.x transition is hardly sane.  If such sanity 
check is necessary, I would rather check the flag explicitly in _pickle.c 
rather than having to generate dummy args and kwds to satisfy tp_new signature.

>
> So, IMO, the right thing to do would be to choose the first base
> type that isn't a Python-defined class and use its tp_new:
>
>    staticbase = subtype;
>    while (staticbase && (staticbase->tp_flags & Py_TPFLAGS_HEAPTYPE))
>        staticbase = staticbase->tp_base;
>
> (these two lines are from tp_new_wrapper() in typeobject.c)
>

This looks like overengineering to me.  I think 3.x should simply refuse to 
unpickle old style class instances into anything that is not subclassed in 
python directly from object.  It would be helpful at this point if you could 
provide a test case where tp_alloc logic does not work.  Regardless of what 
algorithm we ultimately choose, I would like to include such test in regrtest.

PS: Note that in my patch I've eliminated one control path that was calling 
instantiate(..) erroneously, but I have not reviewed the code carefully enough 
to eliminate the possibility that instantiate(..) would be called to 
instantiate new-style class instances.

PPS: What is you opinion on the pickle.py trick of monkey patching __class__ on 
an instance of an empty class?  Do you think this can be fooled?

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue5180>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to