[mypyc] Generate more type methods for types with managed dicts

Author: p-sawicki

mypyc/codegen/emitclass.py

@@ -262,7 +262,8 @@ def generate_class(cl: ClassIR, module: str, emitter: Emitter) -> None:
     if not cl.builtin_base:
         fields["tp_new"] = new_name
 
-    if generate_full:
+    managed_dict = has_managed_dict(cl, emitter)
+    if generate_full or managed_dict:
         fields["tp_dealloc"] = f"(destructor){name_prefix}_dealloc"
         if not cl.is_acyclic:
             fields["tp_traverse"] = f"(traverseproc){name_prefix}_traverse"
@@ -335,6 +336,14 @@ def emit_line() -> None:
     else:
         fields["tp_basicsize"] = base_size
 
+    if generate_full or managed_dict:
+        if not cl.is_acyclic:
+            generate_traverse_for_class(cl, traverse_name, emitter)
+            emit_line()
+        generate_clear_for_class(cl, clear_name, emitter)
+        emit_line()
+        generate_dealloc_for_class(cl, dealloc_name, clear_name, bool(del_method), emitter)
+        emit_line()
     if generate_full:
         assert cl.setup is not None
         emitter.emit_line(native_function_header(cl.setup, emitter) + ";")
@@ -345,13 +354,6 @@ def emit_line() -> None:
         init_fn = cl.get_method("__init__")
         generate_new_for_class(cl, new_name, vtable_name, setup_name, init_fn, emitter)
         emit_line()
-        if not cl.is_acyclic:
-            generate_traverse_for_class(cl, traverse_name, emitter)
-            emit_line()
-        generate_clear_for_class(cl, clear_name, emitter)
-        emit_line()
-        generate_dealloc_for_class(cl, dealloc_name, clear_name, bool(del_method), emitter)
-        emit_line()
 
         if cl.allow_interpreted_subclasses:
             shadow_vtable_name: str | None = generate_vtables(
@@ -380,7 +382,7 @@ def emit_line() -> None:
     emit_line()
 
     flags = ["Py_TPFLAGS_DEFAULT", "Py_TPFLAGS_HEAPTYPE", "Py_TPFLAGS_BASETYPE"]
-    if generate_full and not cl.is_acyclic:
+    if (generate_full or managed_dict) and not cl.is_acyclic:
         flags.append("Py_TPFLAGS_HAVE_GC")
     if cl.has_method("__call__"):
         fields["tp_vectorcall_offset"] = "offsetof({}, vectorcall)".format(
@@ -391,7 +393,7 @@ def emit_line() -> None:
             # This is just a placeholder to please CPython. It will be
             # overridden during setup.
             fields["tp_call"] = "PyVectorcall_Call"
-    if has_managed_dict(cl, emitter):
+    if managed_dict:
         flags.append("Py_TPFLAGS_MANAGED_DICT")
     fields["tp_flags"] = " | ".join(flags)
 
@@ -869,7 +871,8 @@ def generate_traverse_for_class(cl: ClassIR, func_name: str, emitter: Emitter) -
         for attr, rtype in base.attributes.items():
             emitter.emit_gc_visit(f"self->{emitter.attr(attr)}", rtype)
     if has_managed_dict(cl, emitter):
-        emitter.emit_line("PyObject_VisitManagedDict((PyObject *)self, visit, arg);")
+        emitter.emit_line("int rv = PyObject_VisitManagedDict((PyObject *)self, visit, arg);")
+        emitter.emit_line("if (rv < 0) return rv;")
     elif cl.has_dict:
         struct_name = cl.struct_name(emitter.names)
         # __dict__ lives right after the struct and __weakref__ lives right after that
@@ -934,6 +937,12 @@ def generate_dealloc_for_class(
         emitter.emit_line("if (res < 0) {")
         emitter.emit_line("goto done;")
         emitter.emit_line("}")
+    if cl.builtin_base:
+        # For native subclasses of builtins such as dict, the base deallocator
+        # is responsible for tearing down base-owned storage and freeing memory.
+        emitter.emit_line(f"{clear_func_name}(self);")
+        emitter.emit_line("Py_TYPE(self)->tp_base->tp_dealloc((PyObject *)self);")
+        emitter.emit_line("goto done;")
     if not cl.is_acyclic:
         emitter.emit_line("PyObject_GC_UnTrack(self);")
     if cl.reuse_freed_instance:

mypyc/test-data/fixtures/ir.py

@@ -40,6 +40,7 @@ def __pow__(self, other: T_contra, modulo: _M) -> T_co: ...
 
 class object:
     __class__: type
+    __dict__: dict[str, Any]
     def __new__(cls) -> Self: pass
     def __init__(self) -> None: pass
     def __init_subclass__(cls, **kwargs: object) -> None: pass

mypyc/test-data/run-classes.test

@@ -3325,20 +3325,31 @@ def test_function():
     assert(isinstance(d.fitem, ForwardDefinedClass))
     assert(isinstance(d.fitems, ForwardDefinedClass))
 
-[case testDelForDictSubclass-xfail]
-# The crash in issue mypy#19175 is fixed.
-# But, for classes that derive from built-in Python classes, user-defined __del__ method is not
-# being invoked.
+[case testDelForDictSubclass]
+events: list[str] = []
+
+class Item:
+    def __del__(self) -> None:
+        events.append("deleting Item")
+
 class DictSubclass(dict):
-    def __del__(self):
-        print("deleting DictSubclass...")
+    def __del__(self) -> None:
+        events.append("deleting DictSubclass")
+
+def test_dict_subclass_dealloc() -> None:
+    d = DictSubclass()
+    d["item"] = Item()
+    del d
 
 [file driver.py]
 import native
-native.DictSubclass()
+native.test_dict_subclass_dealloc()
+assert native.events == [
+    "deleting DictSubclass",
+    "deleting Item",
+]
 
 [out]
-deleting DictSubclass...
 
 [case testDel]
 class A:

mypyc/test-data/run-dicts.test

@@ -368,3 +368,31 @@ class subc(dict[Any, Any]):
 [file userdefineddict.py]
 class dict:
     pass
+
+[case testDunderDictAccessAfterDel]
+class NormDict(dict[str, str]):
+    def __init__(self, attr: int = 42) -> None:
+        super().__init__()
+        self.attr = attr
+
+def test_dict_access() -> None:
+    n = NormDict(1)
+    d = n.__dict__
+    assert d["attr"] == 1
+    del n
+    assert d["attr"] == 1
+
+[file driver.py]
+from native import NormDict, test_dict_access
+
+def test_dict_access_interpreted() -> None:
+    n = NormDict()
+    d = n.__dict__
+    assert d["attr"] == 42
+    del n
+    assert d["attr"] == 42
+
+test_dict_access()
+test_dict_access_interpreted()
+
+[fixture fixtures/typing-full.pyi]