On Mon, Jun 28, 2010 at 7:32 PM, Antoine Pitrou <report@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?