From 9034fb7ef81410d921959a3eec31feff13d98f92 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 28 Jan 2022 09:21:03 -0500 Subject: [PATCH 1/7] Fix optimization bug introduced in #3650 --- include/pybind11/pybind11.h | 11 ++++++----- tests/test_multiple_inheritance.cpp | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index df33d51829..6c7fbee7c0 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1206,13 +1206,14 @@ class generic_type : public object { if (rec.bases.size() > 1 || rec.multiple_inheritance) { mark_parents_nonsimple(tinfo->type); tinfo->simple_ancestors = false; - tinfo->simple_type = false; } else if (rec.bases.size() == 1) { - auto parent_tinfo = get_type_info((PyTypeObject *) rec.bases[0].ptr()); - tinfo->simple_ancestors = parent_tinfo->simple_ancestors; - // a child of a non-simple type can never be a simple type - tinfo->simple_type = parent_tinfo->simple_type; + auto *parent_tinfo = get_type_info((PyTypeObject *) rec.bases[0].ptr()); + assert(parent_tinfo != nullptr); + bool parent_simple_ancestors = parent_tinfo->simple_ancestors; + tinfo->simple_ancestors = parent_simple_ancestors; + // The parent can no longer be a simple type if it has MI and has descendants + parent_tinfo->simple_type = parent_tinfo->simple_type && parent_simple_ancestors; } if (rec.module_local) { diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 44b9876eb4..4689df4e46 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -235,7 +235,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { // - functions are get_{base}_{var}, return {var} struct MVB { MVB() = default; - MVB(const MVB&) = default; + MVB(const MVB &) = default; virtual ~MVB() = default; int b = 1; From b469ba35181a4062eb647d7eca42284df94849d7 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 28 Jan 2022 09:37:02 -0500 Subject: [PATCH 2/7] Add simple Python extension test for MVF --- tests/test_multiple_inheritance.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index 71741b9256..fb80b42488 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -472,3 +472,23 @@ def test_pr3635_diamond_f(): assert o.get_f_e() == 5 assert o.get_f_f() == 6 + + +def test_python_inherit_from_mi(): + class PYMVF(m.MVF): + g = 7 + + def get_g_g(self): + return self.g + + o = PYMVF() + + assert o.b == 1 + assert o.c == 2 + assert o.d0 == 3 + assert o.d1 == 4 + assert o.e == 5 + assert o.f == 6 + assert o.g == 7 + + assert o.get_g_g() == 7 From 486c024d3e46f72872964549e60eb2ccfbf39ed2 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 28 Jan 2022 12:19:46 -0500 Subject: [PATCH 3/7] Improve comments --- include/pybind11/detail/internals.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 462d32474e..f1fa463042 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -200,7 +200,8 @@ struct type_info { void *get_buffer_data = nullptr; void *(*module_local_load)(PyObject *, const type_info *) = nullptr; /* A simple type never occurs as a (direct or indirect) parent - * of a class that makes use of multiple inheritance */ + * of a class that makes use of multiple inheritance. + * The child remains simple unless it becomes a parent of a pybind type. */ bool simple_type : 1; /* True if there is no multiple inheritance in this type's inheritance tree */ bool simple_ancestors : 1; From ba794a52d3d1decb04cba2b340db4847547162e9 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 28 Jan 2022 12:28:50 -0500 Subject: [PATCH 4/7] Clarify comment --- include/pybind11/detail/internals.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index f1fa463042..bf08fe51a8 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -201,7 +201,8 @@ struct type_info { void *(*module_local_load)(PyObject *, const type_info *) = nullptr; /* A simple type never occurs as a (direct or indirect) parent * of a class that makes use of multiple inheritance. - * The child remains simple unless it becomes a parent of a pybind type. */ + * A type can be simple even if it has non-simple ancestors as long as it has no descendants. + */ bool simple_type : 1; /* True if there is no multiple inheritance in this type's inheritance tree */ bool simple_ancestors : 1; From 04d7004bfa62d557ff82f290541325882ef69bdf Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 28 Jan 2022 12:32:31 -0500 Subject: [PATCH 5/7] Clarify another comment --- include/pybind11/pybind11.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6c7fbee7c0..fd60ea29f2 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1212,7 +1212,7 @@ class generic_type : public object { assert(parent_tinfo != nullptr); bool parent_simple_ancestors = parent_tinfo->simple_ancestors; tinfo->simple_ancestors = parent_simple_ancestors; - // The parent can no longer be a simple type if it has MI and has descendants + // The parent can no longer be a simple type if it has MI and has a child parent_tinfo->simple_type = parent_tinfo->simple_type && parent_simple_ancestors; } From 7bd2a0d875c9e076d4388fcf430b3a1a36ab8d93 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 31 Jan 2022 10:31:41 -0500 Subject: [PATCH 6/7] Add test docstring --- tests/test_multiple_inheritance.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index fb80b42488..6d2f03b5c5 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -475,13 +475,15 @@ def test_pr3635_diamond_f(): def test_python_inherit_from_mi(): - class PYMVF(m.MVF): + """Tests extending a Python class from a single inheritor of an MI class""" + + class PyMVF(m.MVF): g = 7 def get_g_g(self): return self.g - o = PYMVF() + o = PyMVF() assert o.b == 1 assert o.c == 2 From 5c442dc8033adf99d9eda4ba54058a863cdef100 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 31 Jan 2022 11:45:35 -0500 Subject: [PATCH 7/7] Fix typo --- tests/test_multiple_inheritance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index 6d2f03b5c5..abdf25d608 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -475,7 +475,7 @@ def test_pr3635_diamond_f(): def test_python_inherit_from_mi(): - """Tests extending a Python class from a single inheritor of an MI class""" + """Tests extending a Python class from a single inheritor of a MI class""" class PyMVF(m.MVF): g = 7