Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect __module__ attribute for _struct.Struct and perhaps a few others #79512

Open
mr-nfamous mannequin opened this issue Nov 27, 2018 · 2 comments
Open

Incorrect __module__ attribute for _struct.Struct and perhaps a few others #79512

mr-nfamous mannequin opened this issue Nov 27, 2018 · 2 comments
Assignees
Labels
3.10 3.11 3.12 type-bug

Comments

@mr-nfamous
Copy link
Mannequin

@mr-nfamous mr-nfamous mannequin commented Nov 27, 2018

BPO 35331
Nosy @mdickinson, @meadori, @serhiy-storchaka, @mr-nfamous

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:

assignee = None
closed_at = None
created_at = <Date 2018-11-27.18:22:25.306>
labels = ['3.8', 'type-bug', '3.7']
title = 'Incorrect __module__ attribute for _struct.Struct and perhaps a few others'
updated_at = <Date 2019-05-23.12:16:40.753>
user = 'https://github.com/mr-nfamous'

bugs.python.org fields:

activity = <Date 2019-05-23.12:16:40.753>
actor = 'bup'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2018-11-27.18:22:25.306>
creator = 'bup'
dependencies = []
files = []
hgrepos = []
issue_num = 35331
keywords = []
message_count = 1.0
messages = ['330544']
nosy_count = 4.0
nosy_names = ['mark.dickinson', 'meador.inge', 'serhiy.storchaka', 'bup']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue35331'
versions = ['Python 3.7', 'Python 3.8']

@mr-nfamous
Copy link
Mannequin Author

@mr-nfamous mr-nfamous mannequin commented Nov 27, 2018

_struct.Struct not defining a valid __module__ by prefixing its tp_name slot with "_struct" is inconsistent with every other extension type which is available in the corresponding module globals.

From the documentation of the tp_name slot:

Pointer to a NUL-terminated string containing the name of the type. For types that are accessible as module globals, the string should be the full module name, followed by a dot, followed by the type name; for built-in types, it should be just the type name. If the module is a submodule of a package, the full package name is part of the full module name. For example, a type named T defined in module M in subpackage Q in package P should have the tp_name initializer "P.Q.M.T".

For dynamically allocated type objects, this should just be the type name, and the module name explicitly stored in the type dict as the value for key '__module__'.

----

I know that this is also a way to make something unpickleable, but that seems like a poor way to do it and since _struct.Struct was relatively alone in this, I figured it was an oversight.

At the end is the script I made to display all currently alive "builtins" classes that have been "PyType_Ready"ed. For brevity I further manually filtered out obvious cases where a specified module would be inappropriate.

The main point is that I think the new contextvars classes, _struct.Struct, and the weakref classes are missing the "_struct", "_contextvars", and "_weakref" prefixes in their tp_name slots, respectively. Since _contextvars is one of the few extension modules using the multiphase initialization protocol, maybe it should go in their type dicts (although the prefix method still works) instead, although i think the docs were referring to heap allocated types.

if __name__=='__main__':
    import sys, collections
    subclassesof = type.__subclasses__
    def get_types(*names):
        r = {"__builtins__":{'__import__':__import__, 'globals':globals}}
        for name in names:
            exec(f'from {name} import __dict__ as d; globals().update(d)', r)
        return dict.fromkeys(r[k] for k in r if isinstance(r[k],type)).keys()
    def derivative_classes(cls):
        a = b = r = {*subclassesof(cls)}
        while b:
            r, a, b, = r|b, b, set().union(*map(subclassesof, b))
        return r | a    
    classes = derivative_classes(object)
    singles = None, NotImplemented, ...
    od = collections.OrderedDict()
    odtypes = iter(od), od.keys(), od.items(), od.values()
    bltns = {cls for cls in classes if cls.__module__=='builtins'}
    bltns-= get_types('builtins', 'types', '_collections_abc')
    bltns-= {*map(type, odtypes)} | {*map(type, singles)}
    for cls in sorted(bltns, key=vars(type)['__name__'].__get__):
        print(f'# {sys.getrefcount(cls):4} {cls.__name__}')

# all of these are in _contextvars.__dict__ but have their __module__=='builtins':
# 25 Context
# 15 ContextVar
# 12 Token

# from _struct
# 23 Struct # IS in _struct.__dict__
# 11 unpack_iterator # no tp_new so make sense to leave as-is

# These are here because it's a mystery how they were included in the results
# without importing _testcapi:
# 25 hamt
# 8 hamt_array_node
# 8 hamt_bitmap_node
# 8 hamt_collision_node

# no idea what these are:
# 11 items
# 11 values
# 11 keys

# these are all in _weakref.__dict__
# 76 weakcallableproxy
# 76 weakproxy
# 32 weakref

@mr-nfamous mr-nfamous mannequin added 3.7 3.8 labels Nov 27, 2018
@SilentGhost SilentGhost mannequin added the type-bug label Nov 28, 2018
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@serhiy-storchaka serhiy-storchaka self-assigned this Jun 11, 2022
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jun 11, 2022
Classes ReferenceType, ProxyType and CallableProxyType have now correct
atrtributes __module__, __name__ and __qualname__. It makes them pickleable.
@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jun 11, 2022

It was fixed for _struct in 4f384af (bpo-38076) and for _contextvars in 988f1ec (bpo-1635741).

hamt, items, etc are private types.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jun 11, 2022
serhiy-storchaka added a commit that referenced this issue Jun 14, 2022
Classes ReferenceType, ProxyType and CallableProxyType have now correct
atrtributes __module__, __name__ and __qualname__.
It makes them (types, not instances) pickleable.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 14, 2022
…ythonGH-93719)

Classes ReferenceType, ProxyType and CallableProxyType have now correct
atrtributes __module__, __name__ and __qualname__.
It makes them (types, not instances) pickleable.
(cherry picked from commit 8352e32)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington added a commit that referenced this issue Jun 14, 2022
Classes ReferenceType, ProxyType and CallableProxyType have now correct
atrtributes __module__, __name__ and __qualname__.
It makes them (types, not instances) pickleable.
(cherry picked from commit 8352e32)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 3.11 3.12 type-bug
Projects
Status: In Progress
Development

No branches or pull requests

1 participant