diff --git a/mypyc/codegen/emit.py b/mypyc/codegen/emit.py index e313c9231564d..55aba6c5e1c18 100644 --- a/mypyc/codegen/emit.py +++ b/mypyc/codegen/emit.py @@ -1398,6 +1398,12 @@ def emit_cpyfunction_instance( self.emit_line(error_stmt) return wrapper_name + def emit_base_tp_function_call( + self, derived_cl: ClassIR, tp_func: str, args: str, *, prefix: str = "" + ) -> None: + type_obj = self.type_struct_name(derived_cl) + self.emit_line(f"{prefix}{type_obj}->tp_base->{tp_func}({args});") + def c_array_initializer(components: list[str], *, indented: bool = False) -> str: """Construct an initializer for a C array variable. diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index 6d8be66672a32..1b6d1217a87d9 100644 --- a/mypyc/codegen/emitclass.py +++ b/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) @@ -868,8 +870,14 @@ def generate_traverse_for_class(cl: ClassIR, func_name: str, emitter: Emitter) - for base in reversed(cl.base_mro): for attr, rtype in base.attributes.items(): emitter.emit_gc_visit(f"self->{emitter.attr(attr)}", rtype) + base_args = "(PyObject *)self, visit, arg" + emitter.emit_line("int rv = 0;") + if cl.builtin_base: + emitter.emit_base_tp_function_call(cl, "tp_traverse", base_args, prefix="rv = ") + emitter.emit_line("if (rv != 0) return rv;") if has_managed_dict(cl, emitter): - emitter.emit_line("PyObject_VisitManagedDict((PyObject *)self, visit, arg);") + emitter.emit_line(f"rv = PyObject_VisitManagedDict({base_args});") + 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 @@ -880,7 +888,7 @@ def generate_traverse_for_class(cl: ClassIR, func_name: str, emitter: Emitter) - f"*((PyObject **)((char *)self + sizeof(PyObject *) + sizeof({struct_name})))", object_rprimitive, ) - emitter.emit_line("return 0;") + emitter.emit_line("return rv;") emitter.emit_line("}") @@ -891,8 +899,11 @@ def generate_clear_for_class(cl: ClassIR, func_name: str, emitter: Emitter) -> N for base in reversed(cl.base_mro): for attr, rtype in base.attributes.items(): emitter.emit_gc_clear(f"self->{emitter.attr(attr)}", rtype) + base_args = "(PyObject *)self" + if cl.builtin_base: + emitter.emit_base_tp_function_call(cl, "tp_clear", base_args) if has_managed_dict(cl, emitter): - emitter.emit_line("PyObject_ClearManagedDict((PyObject *)self);") + emitter.emit_line(f"PyObject_ClearManagedDict({base_args});") elif cl.has_dict: struct_name = cl.struct_name(emitter.names) # __dict__ lives right after the struct and __weakref__ lives right after that @@ -936,6 +947,18 @@ def generate_dealloc_for_class( emitter.emit_line("}") if not cl.is_acyclic: emitter.emit_line("PyObject_GC_UnTrack(self);") + if cl.builtin_base: + emitter.emit_line(f"{clear_func_name}(self);") + # For native subclasses of builtins such as dict, the base deallocator + # is responsible for tearing down base-owned storage and freeing memory. + # Re-track self if base is GC-aware to match cpython's subtype_dealloc. + base = f"{emitter.type_struct_name(cl)}->tp_base" + base_arg = "(PyObject *)self" + emitter.emit_line(f"if (PyType_IS_GC({base})) PyObject_GC_Track({base_arg});") + emitter.emit_base_tp_function_call(cl, "tp_dealloc", base_arg) + emitter.emit_line("done: ;") + emitter.emit_line("}") + return if cl.reuse_freed_instance: emit_reuse_dealloc(cl, emitter) # The trashcan is needed to handle deep recursive deallocations diff --git a/mypyc/irbuild/prepare.py b/mypyc/irbuild/prepare.py index 906a4fe46a2f7..5988c1d1585af 100644 --- a/mypyc/irbuild/prepare.py +++ b/mypyc/irbuild/prepare.py @@ -424,22 +424,6 @@ def prepare_class_def( if attrs.get("acyclic") is True: ir.is_acyclic = True - free_list_len = attrs.get("free_list_len") - if free_list_len is not None: - line = attrs_lines["free_list_len"] - if ir.is_trait: - errors.error('"free_list_len" can\'t be used with traits', path, line) - if ir.allow_interpreted_subclasses: - errors.error( - '"free_list_len" can\'t be used in a class that allows interpreted subclasses', - path, - line, - ) - if free_list_len == 1: - ir.reuse_freed_instance = True - else: - errors.error(f'Unsupported value for "free_list_len": {free_list_len}', path, line) - # Check for subclassing from builtin types for cls in info.mro: # Special case exceptions and dicts @@ -468,6 +452,28 @@ def prepare_class_def( cdef.line, ) + free_list_len = attrs.get("free_list_len") + if free_list_len is not None: + line = attrs_lines["free_list_len"] + if ir.is_trait: + errors.error('"free_list_len" can\'t be used with traits', path, line) + if ir.allow_interpreted_subclasses: + errors.error( + '"free_list_len" can\'t be used in a class that allows interpreted subclasses', + path, + line, + ) + if ir.builtin_base: + errors.error( + '"free_list_len" can\'t be used in a class that inherits from a built-in type', + path, + line, + ) + if free_list_len == 1: + ir.reuse_freed_instance = True + else: + errors.error(f'Unsupported value for "free_list_len": {free_list_len}', path, line) + # Set up the parent class bases = [mapper.type_to_ir[base.type] for base in info.bases if base.type in mapper.type_to_ir] if len(bases) > 1 and any(not c.is_trait for c in bases) and bases[0].is_trait: diff --git a/mypyc/lib-rt/pythoncapi_compat.h b/mypyc/lib-rt/pythoncapi_compat.h index 08b691570d9ae..eb33835ac3f12 100644 --- a/mypyc/lib-rt/pythoncapi_compat.h +++ b/mypyc/lib-rt/pythoncapi_compat.h @@ -922,7 +922,8 @@ PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg) { PyObject **dict = _PyObject_GetDictPtr(obj); if (dict == NULL || *dict == NULL) { - return -1; + // Nothing to do. + return 0; } Py_VISIT(*dict); return 0; diff --git a/mypyc/test-data/fixtures/ir.py b/mypyc/test-data/fixtures/ir.py index f1268ec83601d..2da6244d896b1 100644 --- a/mypyc/test-data/fixtures/ir.py +++ b/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 diff --git a/mypyc/test-data/irbuild-classes.test b/mypyc/test-data/irbuild-classes.test index a4d51a444cbb5..d13bd956e1259 100644 --- a/mypyc/test-data/irbuild-classes.test +++ b/mypyc/test-data/irbuild-classes.test @@ -2138,6 +2138,10 @@ class NonNative: class InterpSub: pass +@mypyc_attr(free_list_len=1) # E: "free_list_len" can't be used in a class that inherits from a built-in type +class InheritsBuiltIn(dict): + pass + [case testAcyclicClassRequiresAcyclicBases] from typing import Generic, TypeVar from mypy_extensions import mypyc_attr, trait diff --git a/mypyc/test-data/run-classes.test b/mypyc/test-data/run-classes.test index 4574d9c6306f0..39172a6385696 100644 --- a/mypyc/test-data/run-classes.test +++ b/mypyc/test-data/run-classes.test @@ -3325,20 +3325,39 @@ 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() +import sys -[out] -deleting DictSubclass... +from native import events, test_dict_subclass_dealloc + +test_dict_subclass_dealloc() + +expected_events: list[str] = [] + +# TODO: Fix when compiling for older python. +# The user-defined __del__ method is currently only invoked when __dict__ is a managed dict +# because calling __del__ in tp_clear on older python crashes. +if sys.version_info >= (3, 12): + expected_events.append("deleting DictSubclass") +expected_events.append("deleting Item") + +assert events == expected_events, events [case testDel] class A: diff --git a/mypyc/test-data/run-dicts.test b/mypyc/test-data/run-dicts.test index 2b75b32c906e0..da507900ff506 100644 --- a/mypyc/test-data/run-dicts.test +++ b/mypyc/test-data/run-dicts.test @@ -368,3 +368,148 @@ class subc(dict[Any, Any]): [file userdefineddict.py] class dict: pass + +[case testDunderDictAccessAfterDel] +from mypy_extensions import mypyc_attr + +@mypyc_attr(allow_interpreted_subclasses=True) +class NormDict(dict[str, str]): + def __init__(self, attr: int = 42) -> None: + super().__init__() + self.attr = attr + +class SubNormDict(NormDict): + def __init__(self, attr: int = 43) -> None: + super().__init__(attr) + +def test_dict_access() -> None: + n = NormDict(1) + d = n.__dict__ + assert d["attr"] == 1 + del n + assert d["attr"] == 1 + +def test_subclass_dict_access() -> None: + s = SubNormDict(1) + d = s.__dict__ + assert d["attr"] == 1 + del s + assert d["attr"] == 1 + +[file driver.py] +from native import NormDict, SubNormDict, test_dict_access, test_subclass_dict_access + + +class InterpretedNormDict(NormDict): + pass + +def test_dict_access_interpreted() -> None: + n = NormDict() + d = n.__dict__ + assert d["attr"] == 42 + del n + assert d["attr"] == 42 + +def test_subclass_dict_access_interpreted() -> None: + s = SubNormDict() + d = s.__dict__ + assert d["attr"] == 43 + del s + assert d["attr"] == 43 + +def test_allow_interpreted_subclass_dict_access() -> None: + s = InterpretedNormDict() + d = s.__dict__ + assert d["attr"] == 42 + del s + assert d["attr"] == 42 + +test_dict_access() +test_dict_access_interpreted() + +test_subclass_dict_access() +test_subclass_dict_access_interpreted() +test_allow_interpreted_subclass_dict_access() + +[fixture fixtures/typing-full.pyi] + +[case testCycleInDictSubclass] +import gc +from mypy_extensions import mypyc_attr + +events: list[str] = [] + +@mypyc_attr(allow_interpreted_subclasses=True) +class CyclicDict(dict): + def __init__(self) -> None: + self["self"] = self + + def __del__(self) -> None: + events.append("deleted") + +class SubCyclicDict(CyclicDict): + def __init__(self) -> None: + super().__init__() + + def __del__(self) -> None: + events.append("sub deleted") + +def test_cyclic_dict_cleanup() -> None: + global events + events = [] + + c = CyclicDict() + del c + gc.collect() + assert events == ["deleted"], events + +def test_sub_cyclic_dict_cleanup() -> None: + global events + events = [] + + c = SubCyclicDict() + del c + gc.collect() + assert events == ["sub deleted"], events + +[file driver.py] +import gc + +import native +from native import CyclicDict, SubCyclicDict, test_cyclic_dict_cleanup, test_sub_cyclic_dict_cleanup + + +class InterpretedCyclicDict(CyclicDict): + def __del__(self) -> None: + native.events.append("interpreted deleted") + +def test_cyclic_dict_cleanup_interpreted() -> None: + native.events = [] + + c = CyclicDict() + del c + gc.collect() + assert native.events == ["deleted"], events + +def test_sub_cyclic_dict_cleanup_interpreted() -> None: + native.events = [] + + c = SubCyclicDict() + del c + gc.collect() + assert native.events == ["sub deleted"], events + +def test_allow_interpreted_subclass_cyclic_dict_cleanup() -> None: + native.events = [] + + c = InterpretedCyclicDict() + del c + gc.collect() + assert native.events == ["interpreted deleted"], events + +test_cyclic_dict_cleanup() +test_sub_cyclic_dict_cleanup() + +test_cyclic_dict_cleanup_interpreted() +test_sub_cyclic_dict_cleanup_interpreted() +test_allow_interpreted_subclass_cyclic_dict_cleanup()