Lib/types.py nit: isinstance != PyType_IsSubtype

[86003435-947b-49e5-a58a-4e75cd09aaad]

BPO	22968
Nosy	@1st1
Files	fix_types_calculate_meta.patch: More correct implementation of types._caclculate_meta fix_types_calculate_meta_v2.patch: v2: More correct implementation of class creation APIs in Lib/types.py fix_types_calculate_meta_v3.patch: v3: Just adds a commit message test_virtual_metaclasses.patch: Tests verifying that type machinery really works this way test_virtual_metaclasses_in_types_module.patch: Tests verifying that types.py matches type machinery test_virtual_metaclasses_in_types_module_v2.patch: Tests verifying that types.py implementation matches native type machinery

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library'] title = 'Lib/types.py nit: isinstance != PyType_IsSubtype' updated_at = user = 'https://bugs.python.org/gmt' ``` bugs.python.org fields: ```python activity = actor = 'serhiy.storchaka' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'gmt' dependencies = [] files = ['37318', '37348', '37349', '37350', '37352', '37356'] hgrepos = [] issue_num = 22968 keywords = ['patch'] message_count = 7.0 messages = ['231871', '231980', '232067', '232074', '232075', '232077', '232104'] nosy_count = 2.0 nosy_names = ['yselivanov', 'gmt'] pr_nums = [] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue22968' versions = ['Python 3.4', 'Python 3.5', 'Python 3.6'] ```

[86003435-947b-49e5-a58a-4e75cd09aaad]

Kinda trivial but...

Something like the enclosed, untested patch would seem to make new_class work a bit more like type_new.

To be explicit, the difference is whether or not to respect virtual subclasses.

So, for example, as implemented, we can implement __subclasscheck on a 'AtypicalMeta' metaclass to create an 'Atypical' class whose __mro is (ATypical, object) -- note, no 'type' -- but for which issubclass(type, ATypical) is true.

If I'm not mistaken, this disguise will suffice sneak our psuedo-metatype past newclass (but not type.\_new__, so we still get the same error message, and the universe does not implode on itself as planned...)

In my case,the only sequela was that the fantasy of my very own type-orthogonal graph of "foo"-style classes in 3.x was first subtly encouraged and then dashed against the Cpython rocks... (just kidding, sort-of).

[86003435-947b-49e5-a58a-4e75cd09aaad]

Also:

In types.prepare_class, here is what currently happens:

we let x be, in order of preference: (a) the explicitly given type if, one is given, else (b) type(bases[0]), if it exists, else (c) type

and then, if isinstance(x, type), we run _calculate_bases.

In python 2, I think something like this really does happen, although, perhaps "isinstance(x, type)" should have been "issubclass(x, type)" to correctly capture how python 2 does it.

In particular, I think we can stick a Callable in there as bases[0], and then any old crazy list of objects as base classes, and it will call our Callable, although if we don't do something about our crazy base classes, it will still break later (something like that... I don't remember exactly, except that the first base is definitely special somehow).

But in python 3, if I'm reading the C code correctly, I don't think the first base class receives any "special" handling, and the cpython-equivalent to _calculate_bases /always/ happens, suggesting that any non-descending-from-type metaclass is expected to have removed itself from the picture before type_new is invoked.

So maybe more minor re-factoring is needed to get it all straightened out. My brain is kind of fried from looking at it, I'll try again later.

[86003435-947b-49e5-a58a-4e75cd09aaad]

perhaps "isinstance(x, type)" should have been "issubclass(x, type)" to correctly capture how python 2 does it.

Actually, scratch that -- its a brain fart.

IIUC newclass was created because, at the time, there was no exposed turnkey method to execute the PEP-3115 protocol... if that's right, is it worth considering, now that \_build_class__ is exposed, a reimplementation like:

def _pass(ns): pass

def new_class(name, bases=(), kwds={}, exec_body=_pass):
    __build_class__(exec_body, name, *bases, **kwds)

? But this is the wrong place to ask.

...

BTW, since my original post I've figured out a bunch of things, or think I have:

• newclass should model \_build_class__

• __build_class__ need not model type_new in terms of what it allows.

I think point one is what was confusing me before.

It is perhaps worth asking, if the differences between typenew and \_build_class__ make sense, given that python 3's type_new is stricter than python 2's? I think I've figured that out too:

A given (metatype, bases) tuple may be sensibly used to implement the PEP-3115 protocol only if either:

A) it can be proved that type_new will return NULL if it is invoked like type_new(metatype, name, bases,...), or,

B) it can be proved that type_new, when invoked like type_new(metatype, name, bases, ...), will use metatype, and not some other (e.g., more derived) type, as the tp_base of the thing it creates, in the event that it does not return NULL.

If the above is falsified, we pretty clearly have a situation where build_type has used the wrong metaclass.

But the fact that __build_class__ is more liberal than type_new is not a problem from the perspective of type_new -- indeed it's a feature, and I'm now satisfied that the above constraint holds.

I've attached a new patch, this time I tested it.

[86003435-947b-49e5-a58a-4e75cd09aaad]

Just added a commit message.

[86003435-947b-49e5-a58a-4e75cd09aaad]

Here are some tests for this... first, some tests that pass right now and demonstrate, I think, correctness of the second batch of tests.

[86003435-947b-49e5-a58a-4e75cd09aaad]

And some analogous tests for types.py, which don't pass without fix_types_calculate_meta_v3.patch.

[86003435-947b-49e5-a58a-4e75cd09aaad]

Fixed a trivial typo in test_virtual_metaclass_in_types_module.patch