<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 1, 2014 at 1:38 PM, Nathaniel Smith <span dir="ltr"><<a href="mailto:njs@pobox.com" target="_blank">njs@pobox.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Dec 1, 2014 at 4:06 AM, Guido van Rossum <<a href="mailto:guido@python.org">guido@python.org</a>> wrote:<br>
> On Sun, Nov 30, 2014 at 5:42 PM, Nathaniel Smith <<a href="mailto:njs@pobox.com">njs@pobox.com</a>> wrote:<br>
>> On Mon, Dec 1, 2014 at 1:27 AM, Guido van Rossum <<a href="mailto:guido@python.org">guido@python.org</a>> wrote:<br>
>> > Nathaniel, did you look at Brett's LazyLoader? It overcomes the subclass<br>
>> > issue by using a module loader that makes all modules instances of a<br>
>> > (trivial) Module subclass. I'm sure this approach can be backported as<br>
>> > far<br>
>> > as you need to go.<br>
>> The problem is that by the time your package's code starts running,<br>
>> it's too late to install such a loader. Brett's strategy works well<br>
>> for lazy-loading submodules (e.g., making it so 'import numpy' makes<br>
>> 'numpy.testing' available, but without the speed hit of importing it<br>
>> immediately), but it doesn't help if you want to actually hook<br>
>> attribute access on your top-level package (e.g., making 'numpy.foo'<br>
>> trigger a DeprecationWarning -- we have a lot of stupid exported<br>
>> constants that we can never get rid of because our rules say that we<br>
>> have to deprecate things before removing them).<br>
>> Or maybe you're suggesting that we define a trivial heap-allocated<br>
>> subclass of PyModule_Type and use that everywhere, as a<br>
>> quick-and-dirty way to enable __class__ assignment? (E.g., return it<br>
>> from PyModule_New?) I considered this before but hesitated b/c it<br>
>> could potentially break backwards compatibility -- e.g. if code A<br>
>> creates a PyModule_Type object directly without going through<br>
>> PyModule_New, and then code B checks whether the resulting object is a<br>
>> module by doing isinstance(x, type(sys)), this will break. (type(sys)<br>
>> is a pretty common way to get a handle to ModuleType -- in fact both<br>
>> types.py and importlib use it.) So in my mind I sorta lumped it in<br>
>> with my Option 2, "minor compatibility break". OTOH maybe anyone who<br>
>> creates a module object without going through PyModule_New deserves<br>
>> whatever they get.<br>
> Couldn't you install a package loader using some install-time hook?<br>
> Anyway, I still think that the issues with heap types can be overcome. Hm,<br>
> didn't you bring that up before here? Was the conclusion that it's<br>
> impossible?<br>
</div></div>I've brought it up several times but no-one's really discussed it :-).<br></blockquote><div><br></div><div>That's because nobody dares to touch it. (Myself included -- I increased the size of typeobject.c from ~50 to ~5000 lines in a single intense editing session more than a decade ago, and since then it's been basically unmaintainable. :-(<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I finally attempted a deep dive into typeobject.c today myself. I'm<br>
not at all sure I understand the intricacies correctly here, but I<br>
*think* __class__ assignment could be relatively easily extended to<br>
handle non-heap types, and in fact the current restriction to heap<br>
types is actually buggy (IIUC).<br>
object_set_class is responsible for checking whether it's okay to take<br>
an object of class "oldto" and convert it to an object of class<br>
"newto". Basically it's goal is just to avoid crashing the interpreter<br>
(as would quickly happen if you e.g. allowed "[].__class__ = dict").<br>
Currently the rules (spread across object_set_class and<br>
compatible_for_assignment) are:<br>
(1) both oldto and newto have to be heap types<br>
(2) they have to have the same tp_dealloc<br>
(3) they have to have the same tp_free<br>
(4) if you walk up the ->tp_base chain for both types until you find<br>
the most-ancestral type that has a compatible struct layout (as<br>
checked by equiv_structs), then either<br>
   (4a) these ancestral types have to be the same, OR<br>
   (4b) these ancestral types have to have the same tp_base, AND they<br>
have to have added the same slots on top of that tp_base (e.g. if you<br>
have class A(object): pass and class B(object): pass then they'll both<br>
have added a __dict__ slot at the same point in the instance struct,<br>
so that's fine; this is checked in same_slots_added).<br>
The only place the code assumes that it is dealing with heap types is<br>
in (4b) -- same_slots_added unconditionally casts the ancestral types<br>
to (PyHeapTypeObject*). AFAICT that's why step (1) is there, to<br>
protect this code. But I don't think the check actually works -- step<br>
(1) checks that the types we're trying to assign are heap types, but<br>
this is no guarantee that the *ancestral* types will be heap types.<br>
[Also, the code for __bases__ assignment appears to also call into<br>
this code with no heap type checks at all.] E.g., I think if you do<br>
class MyList(list):<br>
    __slots__ = ()<br>
class MyDict(dict):<br>
    __slots__ = ()<br>
MyList().__class__ = MyDict()<br>
then you'll end up in same_slots_added casting PyDict_Type and<br>
PyList_Type to PyHeapTypeObjects and then following invalid pointers<br>
into la-la land. (The __slots__ = () is to maintain layout<br>
compatibility with the base types; if you find builtin types that<br>
already have __dict__ and weaklist and HAVE_GC then this example<br>
should still work even with perfectly empty subclasses.)<br></blockquote><div><br></div><div>Have you filed this as a bug? I believe nobody has discovered this problem before. I've confirmed it as far back as 2.5 (I don't have anything older installed).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Okay, so suppose we move the heap type check (step 1) down into<br>
same_slots_added (step 4b), since AFAICT this is actually more correct<br>
anyway. This is almost enough to enable __class__ assignment on<br>
modules, because the cases we care about will go through the (4a)<br>
branch rather than (4b), so the heap type thing is irrelevant.<br>
The remaining problem is the requirement that both types have the same<br>
tp_dealloc (step 2). ModuleType itself has tp_dealloc ==<br>
module_dealloc, while all(?) heap types have tp_dealloc ==<br>
subtype_dealloc.</blockquote><div><br></div><div>Yeah, I can't see a way that type_new() can create a type whose tp_dealloc isn't subtype_dealloc.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Here again, though, I'm not sure what purpose this<br>
check serves. subtype_dealloc basically cleans up extra slots, and<br>
then calls the base class tp_dealloc. So AFAICT it's totally fine if<br>
oldto->tp_dealloc == module_dealloc, and newto->tp_dealloc ==<br>
subtype_dealloc, so long as newto is a subtype of oldto -- b/c this<br>
means newto->tp_dealloc will end up calling oldto->tp_dealloc anyway.<br></blockquote><div><br></div><div>I guess the simple check is an upper bound (or whatever that's called -- my math-speak is rusty ;-) for the necessary-and-sufficient check that you're describing.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
OTOH it's not actually a guarantee of anything useful to see that<br>
oldto->tp_dealloc == newto->tp_dealloc == subtype_dealloc, because<br>
subtype_dealloc does totally different things depending on the<br>
ancestry tree -- MyList and MyDict above pass the tp_dealloc check,<br>
even though list.tp_dealloc and dict.tp_dealloc are definitely *not*<br>
So I suspect that a more correct way to do this check would be something like<br>
PyTypeObject *old__real_deallocer = oldto, *new_real_deallocer = newto;<br>
while (old_real_deallocer->tp_dealloc == subtype_dealloc)<br>
    old_real_deallocer = old_real_deallocer->tp_base;<br>
while (new_real_deallocer->tp_dealloc == subtype_dealloc)<br>
    new_real_deallocer = new_real_deallocer->tp_base;<br>
if (old_real_deallocer->tp_dealloc != new_real_deallocer)<br>
    error out;<br></blockquote><div><br></div><div>I'm not set up to disagree with you on this any more...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Module subclasses would pass this check. Alternatively it might make<br>
more sense to add a check in equiv_structs that<br>
(child_type->tp_dealloc == subtype_dealloc || child_type->tp_dealloc<br>
== parent_type->tp_dealloc); I think that would accomplish the same<br>
thing in a somewhat cleaner way.<br>
Obviously this code is really subtle though, so don't trust any of the<br>
above without review from someone who knows typeobject.c better than<br>
me! (Antoine?)<br clear="all"></blockquote><div><br></div><div>Or Benjamin? <br></div></div><br>-- <br><div class="gmail_signature">--Guido van Rossum (<a href="http://python.org/~guido">python.org/~guido</a>)</div>