Description

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'

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']

Activity

1. mr-nfamous commented on Nov 27, 2018

   

   mr-nfamousmannequin

   on Nov 27, 2018

   MannequinAuthor

   More actions

   _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
2. mr-nfamous

   added

   3.7 (EOL)end of lifeend of life

   3.8 (EOL)end of lifeend of life

   on Nov 27, 2018
3. SilentGhost

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Nov 28, 2018
4. ezio-melotti

   transferred this issue fromon Apr 10, 2022
5. serhiy-storchaka

   added this to Weak reference issueson Jun 9, 2022
6. serhiy-storchaka

   self-assigned this

   on Jun 11, 2022
7. serhiy-storchaka

   added

   3.11only security fixesonly security fixes

   3.10 (EOL)end of lifeend of life

   3.12only security fixesonly security fixes

   and removed

   3.8 (EOL)end of lifeend of life

   3.7 (EOL)end of lifeend of life

   on Jun 11, 2022
8. serhiy-storchaka

   moved this to In Progress in Weak reference issueson Jun 11, 2022
9. serhiy-storchaka

   added a commit that references this issue on Jun 11, 2022

   pythongh-79512: Fixed names and __module__ value of weakref classes

   9c04de8
10. serhiy-storchaka commented on Jun 11, 2022

    

    serhiy-storchaka

    on Jun 11, 2022

    Member

    More actions

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

    hamt, items, etc are private types.
11. miss-islington

    added 2 commits that reference this issue on Jun 14, 2022

    pythongh-79512: Fixed names and __module__ value of weakref classes (p…

    b0bf575

    gh-79512: Fixed names and __module__ value of weakref classes (GH-93719)

    29c7e81
12. serhiy-storchaka

    added 2 commits that reference this issue on Jun 21, 2022

    [3.10] pythongh-79512: Fixed names and __module__ value of weakref cl…

    4456b3c

    [3.10] gh-79512: Fixed names and __module__ value of weakref classes (G…

    54250e7
13. serhiy-storchaka

    closed this as completedon Jun 21, 2022
14. Repository owner moved this from In Progress to Done in Weak reference issueson Jun 21, 2022
15. serhiy-storchaka

    added a commit that references this issue on Jun 26, 2022

    Revert 30935 defer bpo45162 to 312 (#94285)

    1ffde3e
16. mscuthbert commented on Aug 9, 2022

    

    mscuthbert

    on Aug 9, 2022

    More actions

    Hi all -- Thanks for the fix. I wish though that this were done on a major version upgrade, because I did not think that Class.__name__ was considered private (at least it's documented in the data model) and it ended up making 3.10.6 a backwards incompatible version in some places where .name was being used instead of isinstance() checking (long old code from the time when isinstance() checking was very slow, but the code still worked)

Issue actions