(original) (raw)

On Mon, Dec 1, 2014 at 4:06 AM, Guido van Rossum <guido@python.org> wrote:
\> On Sun, Nov 30, 2014 at 5:42 PM, Nathaniel Smith <njs@pobox.com> wrote:
\>>
\>> On Mon, Dec 1, 2014 at 1:27 AM, Guido van Rossum <guido@python.org> wrote:
\>> > Nathaniel, did you look at Brett's LazyLoader? It overcomes the subclass
\>> > issue by using a module loader that makes all modules instances of a
\>> > (trivial) Module subclass. I'm sure this approach can be backported as
\>> > far
\>> > as you need to go.
\>>
\>> The problem is that by the time your package's code starts running,
\>> it's too late to install such a loader. Brett's strategy works well
\>> for lazy-loading submodules (e.g., making it so 'import numpy' makes
\>> 'numpy.testing' available, but without the speed hit of importing it
\>> immediately), but it doesn't help if you want to actually hook
\>> attribute access on your top-level package (e.g., making 'numpy.foo'
\>> trigger a DeprecationWarning -- we have a lot of stupid exported
\>> constants that we can never get rid of because our rules say that we
\>> have to deprecate things before removing them).
\>>
\>> Or maybe you're suggesting that we define a trivial heap-allocated
\>> subclass of PyModule\_Type and use that everywhere, as a
\>> quick-and-dirty way to enable \_\_class\_\_ assignment? (E.g., return it
\>> from PyModule\_New?) I considered this before but hesitated b/c it
\>> could potentially break backwards compatibility -- e.g. if code A
\>> creates a PyModule\_Type object directly without going through
\>> PyModule\_New, and then code B checks whether the resulting object is a
\>> module by doing isinstance(x, type(sys)), this will break. (type(sys)
\>> is a pretty common way to get a handle to ModuleType -- in fact both
\>> types.py and importlib use it.) So in my mind I sorta lumped it in
\>> with my Option 2, "minor compatibility break". OTOH maybe anyone who
\>> creates a module object without going through PyModule\_New deserves
\>> whatever they get.
\>
\>
\> Couldn't you install a package loader using some install-time hook?
\>
\> Anyway, I still think that the issues with heap types can be overcome. Hm,
\> didn't you bring that up before here? Was the conclusion that it's
\> impossible?

I've brought it up several times but no-one's really discussed it :-).

I finally attempted a deep dive into typeobject.c today myself. I'm
not at all sure I understand the intricacies correctly here, but I
\*think\* \_\_class\_\_ assignment could be relatively easily extended to
handle non-heap types, and in fact the current restriction to heap
types is actually buggy (IIUC).

object\_set\_class is responsible for checking whether it's okay to take
an object of class "oldto" and convert it to an object of class
"newto". Basically it's goal is just to avoid crashing the interpreter
(as would quickly happen if you e.g. allowed "\[\].\_\_class\_\_ = dict").
Currently the rules (spread across object\_set\_class and
compatible\_for\_assignment) are:

(1) both oldto and newto have to be heap types
(2) they have to have the same tp\_dealloc
(3) they have to have the same tp\_free
(4) if you walk up the ->tp\_base chain for both types until you find
the most-ancestral type that has a compatible struct layout (as
checked by equiv\_structs), then either
(4a) these ancestral types have to be the same, OR
(4b) these ancestral types have to have the same tp\_base, AND they
have to have added the same slots on top of that tp\_base (e.g. if you
have class A(object): pass and class B(object): pass then they'll both
have added a \_\_dict\_\_ slot at the same point in the instance struct,
so that's fine; this is checked in same\_slots\_added).

The only place the code assumes that it is dealing with heap types is
in (4b) -- same\_slots\_added unconditionally casts the ancestral types
to (PyHeapTypeObject\*). AFAICT that's why step (1) is there, to
protect this code. But I don't think the check actually works -- step
(1) checks that the types we're trying to assign are heap types, but
this is no guarantee that the \*ancestral\* types will be heap types.
\[Also, the code for \_\_bases\_\_ assignment appears to also call into
this code with no heap type checks at all.\] E.g., I think if you do

class MyList(list):
\_\_slots\_\_ = ()

class MyDict(dict):
\_\_slots\_\_ = ()

MyList().\_\_class\_\_ = MyDict()

then you'll end up in same\_slots\_added casting PyDict\_Type and
PyList\_Type to PyHeapTypeObjects and then following invalid pointers
into la-la land. (The \_\_slots\_\_ = () is to maintain layout
compatibility with the base types; if you find builtin types that
already have \_\_dict\_\_ and weaklist and HAVE\_GC then this example
should still work even with perfectly empty subclasses.)

Okay, so suppose we move the heap type check (step 1) down into
same\_slots\_added (step 4b), since AFAICT this is actually more correct
anyway. This is almost enough to enable \_\_class\_\_ assignment on
modules, because the cases we care about will go through the (4a)
branch rather than (4b), so the heap type thing is irrelevant.

The remaining problem is the requirement that both types have the same
tp\_dealloc (step 2). ModuleType itself has tp\_dealloc ==
module\_dealloc, while all(?) heap types have tp\_dealloc ==
subtype\_dealloc.

Here again, though, I'm not sure what purpose this
check serves. subtype\_dealloc basically cleans up extra slots, and
then calls the base class tp\_dealloc. So AFAICT it's totally fine if
oldto->tp\_dealloc == module\_dealloc, and newto->tp\_dealloc ==
subtype\_dealloc, so long as newto is a subtype of oldto -- b/c this
means newto->tp\_dealloc will end up calling oldto->tp\_dealloc anyway.

OTOH it's not actually a guarantee of anything useful to see that
oldto->tp\_dealloc == newto->tp\_dealloc == subtype\_dealloc, because
subtype\_dealloc does totally different things depending on the
ancestry tree -- MyList and MyDict above pass the tp\_dealloc check,
even though list.tp\_dealloc and dict.tp\_dealloc are definitely \*not\*
interchangeable.

So I suspect that a more correct way to do this check would be something like

PyTypeObject \*old\_\_real\_deallocer = oldto, \*new\_real\_deallocer = newto;
while (old\_real\_deallocer->tp\_dealloc == subtype\_dealloc)
old\_real\_deallocer = old\_real\_deallocer->tp\_base;
while (new\_real\_deallocer->tp\_dealloc == subtype\_dealloc)
new\_real\_deallocer = new\_real\_deallocer->tp\_base;
if (old\_real\_deallocer->tp\_dealloc != new\_real\_deallocer)
error out;

Module subclasses would pass this check. Alternatively it might make
more sense to add a check in equiv\_structs that
(child\_type->tp\_dealloc == subtype\_dealloc || child\_type->tp\_dealloc
\== parent\_type->tp\_dealloc); I think that would accomplish the same
thing in a somewhat cleaner way.

Obviously this code is really subtle though, so don't trust any of the
above without review from someone who knows typeobject.c better than
me! (Antoine?)