Check scope's __dict__ instead of using hasattr when registering classes and exceptions (#2335)

* Check scope's __dict__ instead of using hasattr when registering classes and exceptions, to allow registering the same name in a derived class scope

* Extend test_base_and_derived_nested_scope test

* Add tests on error being thrown registering duplicate classes

* Circumvent bug with combination of test_class.py::test_register_duplicate_class and test_factory_constructors.py::test_init_factory_alias
This commit is contained in:
Yannick Jadoul 2020-10-09 01:09:56 +02:00 committed by GitHub
parent deba040b6f
commit 71aea49b8b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 77 additions and 2 deletions

View File

@ -1012,7 +1012,7 @@ public:
PYBIND11_OBJECT_DEFAULT(generic_type, object, PyType_Check) PYBIND11_OBJECT_DEFAULT(generic_type, object, PyType_Check)
protected: protected:
void initialize(const type_record &rec) { void initialize(const type_record &rec) {
if (rec.scope && hasattr(rec.scope, rec.name)) if (rec.scope && hasattr(rec.scope, "__dict__") && rec.scope.attr("__dict__").contains(rec.name))
pybind11_fail("generic_type: cannot initialize type \"" + std::string(rec.name) + pybind11_fail("generic_type: cannot initialize type \"" + std::string(rec.name) +
"\": an object with that name is already defined"); "\": an object with that name is already defined");
@ -1930,7 +1930,7 @@ public:
std::string full_name = scope.attr("__name__").cast<std::string>() + std::string full_name = scope.attr("__name__").cast<std::string>() +
std::string(".") + name; std::string(".") + name;
m_ptr = PyErr_NewException(const_cast<char *>(full_name.c_str()), base.ptr(), NULL); m_ptr = PyErr_NewException(const_cast<char *>(full_name.c_str()), base.ptr(), NULL);
if (hasattr(scope, name)) if (hasattr(scope, "__dict__") && scope.attr("__dict__").contains(name))
pybind11_fail("Error during initialization: multiple incompatible " pybind11_fail("Error during initialization: multiple incompatible "
"definitions with name \"" + std::string(name) + "\""); "definitions with name \"" + std::string(name) + "\"");
scope.attr(name) = *this; scope.attr(name) = *this;

View File

@ -406,6 +406,7 @@ TEST_SUBMODULE(class_, m) {
struct IsNonFinalFinal {}; struct IsNonFinalFinal {};
py::class_<IsNonFinalFinal>(m, "IsNonFinalFinal", py::is_final()); py::class_<IsNonFinalFinal>(m, "IsNonFinalFinal", py::is_final());
// test_exception_rvalue_abort
struct PyPrintDestructor { struct PyPrintDestructor {
PyPrintDestructor() = default; PyPrintDestructor() = default;
~PyPrintDestructor() { ~PyPrintDestructor() {
@ -417,6 +418,7 @@ TEST_SUBMODULE(class_, m) {
.def(py::init<>()) .def(py::init<>())
.def("throw_something", &PyPrintDestructor::throw_something); .def("throw_something", &PyPrintDestructor::throw_something);
// test_multiple_instances_with_same_pointer
struct SamePointer {}; struct SamePointer {};
static SamePointer samePointer; static SamePointer samePointer;
py::class_<SamePointer, std::unique_ptr<SamePointer, py::nodelete>>(m, "SamePointer") py::class_<SamePointer, std::unique_ptr<SamePointer, py::nodelete>>(m, "SamePointer")
@ -426,6 +428,44 @@ TEST_SUBMODULE(class_, m) {
struct Empty {}; struct Empty {};
py::class_<Empty>(m, "Empty") py::class_<Empty>(m, "Empty")
.def(py::init<>()); .def(py::init<>());
// test_base_and_derived_nested_scope
struct BaseWithNested {
struct Nested {};
};
struct DerivedWithNested : BaseWithNested {
struct Nested {};
};
py::class_<BaseWithNested> baseWithNested_class(m, "BaseWithNested");
py::class_<DerivedWithNested, BaseWithNested> derivedWithNested_class(m, "DerivedWithNested");
py::class_<BaseWithNested::Nested>(baseWithNested_class, "Nested")
.def_static("get_name", []() { return "BaseWithNested::Nested"; });
py::class_<DerivedWithNested::Nested>(derivedWithNested_class, "Nested")
.def_static("get_name", []() { return "DerivedWithNested::Nested"; });
// test_register_duplicate_class
struct Duplicate {};
struct OtherDuplicate {};
struct DuplicateNested {};
struct OtherDuplicateNested {};
m.def("register_duplicate_class_name", [](py::module_ m) {
py::class_<Duplicate>(m, "Duplicate");
py::class_<OtherDuplicate>(m, "Duplicate");
});
m.def("register_duplicate_class_type", [](py::module_ m) {
py::class_<OtherDuplicate>(m, "OtherDuplicate");
py::class_<OtherDuplicate>(m, "YetAnotherDuplicate");
});
m.def("register_duplicate_nested_class_name", [](py::object gt) {
py::class_<DuplicateNested>(gt, "DuplicateNested");
py::class_<OtherDuplicateNested>(gt, "DuplicateNested");
});
m.def("register_duplicate_nested_class_type", [](py::object gt) {
py::class_<OtherDuplicateNested>(gt, "OtherDuplicateNested");
py::class_<OtherDuplicateNested>(gt, "YetAnotherDuplicateNested");
});
} }
template <int N> class BreaksBase { public: template <int N> class BreaksBase { public:

View File

@ -383,3 +383,38 @@ def test_multiple_instances_with_same_pointer(capture):
# No assert: if this does not trigger the error # No assert: if this does not trigger the error
# pybind11_fail("pybind11_object_dealloc(): Tried to deallocate unregistered instance!"); # pybind11_fail("pybind11_object_dealloc(): Tried to deallocate unregistered instance!");
# and just completes without crashing, we're good. # and just completes without crashing, we're good.
# https://github.com/pybind/pybind11/issues/1624
def test_base_and_derived_nested_scope():
assert issubclass(m.DerivedWithNested, m.BaseWithNested)
assert m.BaseWithNested.Nested != m.DerivedWithNested.Nested
assert m.BaseWithNested.Nested.get_name() == "BaseWithNested::Nested"
assert m.DerivedWithNested.Nested.get_name() == "DerivedWithNested::Nested"
@pytest.mark.skip("See https://github.com/pybind/pybind11/pull/2564")
def test_register_duplicate_class():
import types
module_scope = types.ModuleType("module_scope")
with pytest.raises(RuntimeError) as exc_info:
m.register_duplicate_class_name(module_scope)
expected = ('generic_type: cannot initialize type "Duplicate": '
'an object with that name is already defined')
assert str(exc_info.value) == expected
with pytest.raises(RuntimeError) as exc_info:
m.register_duplicate_class_type(module_scope)
expected = 'generic_type: type "YetAnotherDuplicate" is already registered!'
assert str(exc_info.value) == expected
class ClassScope:
pass
with pytest.raises(RuntimeError) as exc_info:
m.register_duplicate_nested_class_name(ClassScope)
expected = ('generic_type: cannot initialize type "DuplicateNested": '
'an object with that name is already defined')
assert str(exc_info.value) == expected
with pytest.raises(RuntimeError) as exc_info:
m.register_duplicate_nested_class_type(ClassScope)
expected = 'generic_type: type "YetAnotherDuplicateNested" is already registered!'
assert str(exc_info.value) == expected