PyAPI: check registered classes don't depend on registered classes

When registering a class, warn if it's base-classes or sub-classes
are already registered as this is bad practice.

Currently the check only runs when the `--debug-python` argument is used
to avoid overhead on startup.
This commit is contained in:
Campbell Barton
2025-02-13 12:42:42 +11:00
parent d0ac35eff4
commit e569417798

View File

@@ -9734,6 +9734,75 @@ static void bpy_class_free(void *pyob_ptr)
PyGILState_Release(gilstate);
}
/**
* \return the first base-class which is already registered or null.
*/
static PyTypeObject *bpy_class_check_any_bases_registered(PyTypeObject *cls)
{
if (PyObject *bases = cls->tp_bases) {
const int bases_num = PyTuple_GET_SIZE(bases);
for (int i = 0; i < bases_num; i++) {
PyTypeObject *base_cls = (PyTypeObject *)PyTuple_GET_ITEM(bases, i);
BLI_assert(PyType_Check(base_cls));
if (base_cls->tp_dict) {
if (BPy_StructRNA *py_srna = (BPy_StructRNA *)PyDict_GetItem(base_cls->tp_dict,
bpy_intern_str_bl_rna))
{
if (const StructRNA *srna = static_cast<const StructRNA *>(py_srna->ptr->data)) {
if (srna->flag & STRUCT_RUNTIME) {
return base_cls;
}
}
}
}
if (PyTypeObject *base_cls_test = bpy_class_check_any_bases_registered(base_cls)) {
return base_cls_test;
}
}
}
return nullptr;
}
/**
* \return the first sub-class which is already registered or null.
*/
static PyTypeObject *bpy_class_check_any_subclasses_registered(PyTypeObject *cls)
{
PyObject *subclasses = static_cast<PyObject *>(cls->tp_subclasses);
if (subclasses) {
BLI_assert(PyDict_CheckExact(subclasses));
PyObject *key = nullptr;
Py_ssize_t pos = 0;
PyObject *value = nullptr;
while (PyDict_Next(subclasses, &pos, &key, &value)) {
BLI_assert(PyWeakref_CheckRef(value));
PyObject *value_ref = PyWeakref_GET_OBJECT(value);
if (value_ref == Py_None) {
continue;
}
PyTypeObject *sub_cls = reinterpret_cast<PyTypeObject *>(value_ref);
if (sub_cls->tp_dict) {
if (BPy_StructRNA *py_srna = reinterpret_cast<BPy_StructRNA *>(
PyDict_GetItem(sub_cls->tp_dict, bpy_intern_str_bl_rna)))
{
if (const StructRNA *srna = static_cast<const StructRNA *>(py_srna->ptr->data)) {
if (srna->flag & STRUCT_RUNTIME) {
return sub_cls;
}
}
}
}
if (PyTypeObject *sub_cls_test = bpy_class_check_any_subclasses_registered(sub_cls)) {
return sub_cls_test;
}
}
}
return nullptr;
}
void pyrna_alloc_types()
{
/* NOTE: This isn't essential to run on startup, since sub-types will lazy initialize.
@@ -9856,6 +9925,34 @@ static PyObject *pyrna_register_class(PyObject * /*self*/, PyObject *py_class)
return nullptr;
}
if (G.debug & G_DEBUG_PYTHON) {
/* Warn if a class being registered uses an already registered base-class or sub-class,
* both checks are needed otherwise the order of registering could suppress the warning.
*
* NOTE(@ideasman42) This is mainly to ensure good practice.
* Mix-in classes are preferred when sharing functionality is needed,
* otherwise changes to an Operator for example could unintentionally
* break another operator that sub-classes it. */
if (PyTypeObject *base_cls_test = bpy_class_check_any_bases_registered(
(PyTypeObject *)py_class))
{
fprintf(stderr,
"%s warning, %.200s: references and already registered base-class %.200s\n",
error_prefix,
((PyTypeObject *)py_class)->tp_name,
base_cls_test->tp_name);
}
if (PyTypeObject *sub_cls_test = bpy_class_check_any_subclasses_registered(
(PyTypeObject *)py_class))
{
fprintf(stderr,
"%s warning, %.200s: references and already registered sub-class %.200s\n",
error_prefix,
((PyTypeObject *)py_class)->tp_name,
sub_cls_test->tp_name);
}
}
if (!pyrna_write_check()) {
PyErr_Format(PyExc_RuntimeError,
"%s can't run in readonly state '%.200s'",