Skip to content

Commit

Permalink
Support inheriting from slots classes with gc=False
Browse files Browse the repository at this point in the history
Previously we prevented setting `gc=False` when inheriting from any
non-Struct type. In reality this is safe to set when inheriting from any
non-c-extension class lacking a `__dict__`.
  • Loading branch information
jcrist committed Jan 21, 2024
1 parent 38c3330 commit b17b23e
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
12 changes: 7 additions & 5 deletions msgspec/_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -5456,7 +5456,7 @@ typedef struct {
bool already_has_dict;
int cache_hash;
Py_ssize_t hash_offset;
bool has_non_struct_bases;
bool has_non_slots_bases;
} StructMetaInfo;

static int
Expand Down Expand Up @@ -5505,7 +5505,9 @@ structmeta_collect_base(StructMetaInfo *info, MsgspecState *mod, PyObject *base)
}

if (Py_TYPE(base) != &StructMetaType) {
info->has_non_struct_bases = true;
if (((PyTypeObject *)base)->tp_dictoffset) {
info->has_non_slots_bases = true;
}
/* XXX: in Python 3.8 Generic defines __new__, but we can ignore it.
* This can be removed when Python 3.8 support is dropped */
if (base == mod->typing_generic) return 0;
Expand Down Expand Up @@ -6221,7 +6223,7 @@ StructMeta_new_inner(
.already_has_dict = false,
.cache_hash = arg_cache_hash,
.hash_offset = 0,
.has_non_struct_bases = false,
.has_non_slots_bases = false,
};

info.defaults_lk = PyDict_New();
Expand Down Expand Up @@ -6273,10 +6275,10 @@ StructMeta_new_inner(
}

if (info.gc == OPT_FALSE) {
if (info.has_non_struct_bases) {
if (info.has_non_slots_bases) {
PyErr_SetString(
PyExc_ValueError,
"Cannot set gc=False when inheriting from non-struct types"
"Cannot set gc=False when inheriting from non-struct types with a __dict__"
);
goto cleanup;
}
Expand Down
18 changes: 14 additions & 4 deletions tests/test_struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -1049,18 +1049,28 @@ class Test(Struct, gc=has_gc):
assert not gc.is_tracked(copy.copy(Test(1, ())))
assert gc.is_tracked(copy.copy(Test(1, []))) == has_gc

def test_struct_gc_false_forbids_mixins(self):
class Mixin:
def test_struct_gc_false_cannot_inherit_from_non_slots_classes(self):
class Base:
pass

with pytest.raises(
ValueError,
match="Cannot set gc=False when inheriting from non-struct types",
match="Cannot set gc=False when inheriting from non-struct types with a __dict__",
):

class Test(Struct, Mixin, gc=False):
class Test(Struct, Base, gc=False):
pass

def test_struct_gc_false_can_inherit_from_slots_class_mixin(self):
class Base:
__slots__ = ()

class Test(Struct, Base, gc=False):
x: int

t = Test(1)
assert not gc.is_tracked(t)

@pytest.mark.parametrize("case", ["base-dict", "base-nogc", "nobase"])
def test_struct_gc_false_forbids_dict_true(self, case):
if case == "base-dict":
Expand Down

0 comments on commit b17b23e

Please sign in to comment.