Bug fix: adding back !is_alias<Class>(ptr) that were accidentally omitted. (#2958)

* Bug fix: adding back `!is_alias<Class>(ptr)` that were accidentally omitted.

* Introducing PYBIND11_SH_AVL, PYBIND11_SH_DEF macros. Applying PYBIND11_SH_DEF to test_factory_constructors.py to complete test coverage.

* Using PYBIND11_SH_DEF in test_methods_and_attributes.cpp, for more complete test coverage.

* Using PYBIND11_SH_DEF in test_multiple_inheritance.cpp, for more complete test coverage.

* Cleaning up test_classh_mock.cpp.

* Better explanations for PYBIND11_SH_AVL, PYBIND11_SH_DEF.

* Disabling 3.10-dev builds.
This commit is contained in:
Ralf W. Grosse-Kunstleve 2021-04-19 10:54:37 -07:00 committed by GitHub
parent 0032083113
commit 99de498b26
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 177 additions and 97 deletions

View File

@ -23,7 +23,8 @@ jobs:
- 3.5 - 3.5
- 3.6 - 3.6
- 3.9 - 3.9
- 3.10-dev # Broken b/o https://github.com/pytest-dev/pytest/issues/8539
# - 3.10-dev
- pypy2 - pypy2
- pypy3 - pypy3

View File

@ -101,28 +101,48 @@ holder:
py::classh<Foo>(m, "Foo"); py::classh<Foo>(m, "Foo");
} }
There are three small differences compared to classic pybind11: There are three small differences compared to Classic pybind11:
- ``#include <pybind11/smart_holder.h>`` is used instead of - ``#include <pybind11/smart_holder.h>`` is used instead of
``#include <pybind11/pybind11.h>``. ``#include <pybind11/pybind11.h>``.
- ``py::classh`` is used instead of ``py::class_``.
- The ``PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo)`` macro is needed. - The ``PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo)`` macro is needed.
To the 2nd bullet point, ``py::classh<Foo>`` is simply a shortcut for - ``py::classh`` is used instead of ``py::class_``.
``py::class_<Foo, py::smart_holder>``. The shortcut makes it possible to
switch to using ``py::smart_holder`` without messing up the indentation of
existing code. However, when migrating code that uses ``py::class_<Foo,
std::shared_ptr<Foo>>``, currently ``std::shared_ptr<Foo>`` needs to be
removed manually when switching to ``py::classh`` (#HelpAppreciated this
could probably be avoided with a little bit of template metaprogramming).
To the 3rd bullet point, the macro also needs to appear in other translation To the 2nd bullet point, the ``PYBIND11_SMART_HOLDER_TYPE_CASTERS`` macro
units with pybind11 bindings that involve Python⇄C++ conversions for needs to appear in all translation units with pybind11 bindings that involve
`Foo`. This is the biggest inconvenience of the Conservative mode. Practically, Python⇄C++ conversions for `Foo`. This is the biggest inconvenience of the
at a larger scale it is best to work with a pair of `.h` and `.cpp` files Conservative mode. Practically, at a larger scale it is best to work with a
for the bindings code, with the macros in the `.h` files. pair of `.h` and `.cpp` files for the bindings code, with the macros in the
`.h` files.
To the 3rd bullet point, ``py::classh<Foo>`` is simply a shortcut for
``py::class_<Foo, py::smart_holder>``. The shortcut makes it possible to
switch to using ``py::smart_holder`` without disturbing the indentation of
existing code.
When migrating code that uses ``py::class_<Bar, std::shared_ptr<Bar>>``
there are two alternatives. The first one is to use ``py::classh<Bar>``:
.. code-block:: diff
- py::class_<Bar, std::shared_ptr<Bar>>(m, "Bar");
+ py::classh<Bar>(m, "Bar");
This is clean and simple, but makes it difficult to fall back to Classic
mode if needed. The second alternative is to replace ``std::shared_ptr<Bar>``
with ``PYBIND11_SH_AVL(Bar)``:
.. code-block:: diff
- py::class_<Bar, std::shared_ptr<Bar>>(m, "Bar");
+ py::class_<Bar, PYBIND11_SH_AVL(Bar)>(m, "Bar");
The ``PYBIND11_SH_AVL`` macro substitutes ``py::smart_holder``
in Conservative mode, or ``std::shared_ptr<Bar>`` in Classic mode.
See tests/test_classh_mock.cpp for an example. Note that the macro is also
designed to not disturb the indentation of existing code.
Progressive mode Progressive mode
@ -132,47 +152,63 @@ To work in Progressive mode:
- Add ``-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT`` to the compilation commands. - Add ``-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT`` to the compilation commands.
- Remove any ``std::shared_ptr<...>`` holders from existing ``py::class_`` - Remove or replace (see below) ``std::shared_ptr<...>`` holders.
instantiations.
- Only if custom smart-pointers are used: the - Only if custom smart-pointers are used: the
`PYBIND11_TYPE_CASTER_BASE_HOLDER` macro is needed [`example `PYBIND11_TYPE_CASTER_BASE_HOLDER` macro is needed (see
<https://github.com/pybind/pybind11/blob/2f624af1ac8571d603df2d70cb54fc7e2e3a356a/tests/test_multiple_inheritance.cpp#L72>`_]. tests/test_smart_ptr.cpp for examples).
Overall this is probably easier to work with than the Conservative mode, but Overall this is probably easier to work with than the Conservative mode, but
- the macro inconvenience is shifted from ``py::smart_holder`` to custom - the macro inconvenience is shifted from ``py::smart_holder`` to custom
smart-pointers (but probably much more rarely needed). smart-pointer holders (which are probably much more rare).
- it will not interoperate with other extensions built against master or - it will not interoperate with other extensions built against master or
stable, or extensions built in Conservative mode (see the cross-module stable, or extensions built in Conservative mode (see the cross-module
compatibility section below). compatibility section below).
When migrating code that uses ``py::class_<Bar, std::shared_ptr<Bar>>`` there
are the same alternatives as for the Conservative mode (see previous section).
An additional alternative is to use the ``PYBIND11_SH_DEF(...)`` macro:
Transition from Conservative to Progressive mode .. code-block:: diff
------------------------------------------------
- py::class_<Bar, std::shared_ptr<Bar>>(m, "Bar");
+ py::class_<Bar, PYBIND11_SH_DEF(Bar)>(m, "Bar");
The ``PYBIND11_SH_DEF`` macro substitutes ``py::smart_holder`` only in
Progressive mode, or ``std::shared_ptr<Bar>`` in Classic or Conservative
mode. See tests/test_classh_mock.cpp for an example. Note that the
``PYBIND11_SMART_HOLDER_TYPE_CASTERS`` macro is never needed in combination
with the ``PYBIND11_SH_DEF`` macro, which is an advantage compared to the
``PYBIND11_SH_AVL`` macro. Please review tests/test_classh_mock.cpp for a
concise overview of all available options.
Transition from Classic to Progressive mode
-------------------------------------------
This still has to be tried out more in practice, but in small-scale situations This still has to be tried out more in practice, but in small-scale situations
it may be feasible to switch directly to Progressive mode in a break-fix it may be feasible to switch directly to Progressive mode in a break-fix
fashion. In large-scale situations it seems more likely that an incremental fashion. In large-scale situations it seems more likely that an incremental
approach is needed, which could mean incrementally converting ``py::class_`` approach is needed, which could mean incrementally converting ``py::class_``
to ``py::classh`` including addition of the macros, then flip the switch, to ``py::classh`` and using the family of related macros, then flip the switch
and convert ``py::classh`` back to ``py:class_`` combined with removal of the to Progressive mode, and convert ``py::classh`` back to ``py:class_`` combined
macros if desired (at that point it will work equivalently either way). It with removal of the macros if desired (at that point it will work equivalently
may be smart to delay the final cleanup step until all third-party projects either way). It may be smart to delay the final cleanup step until all
of interest have made the switch, because then the code will continue to third-party projects of interest have made the switch, because then the code
work in either mode. will continue to work in all modes.
Using py::classh but with fallback to classic pybind11 Using py::smart_holder but with fallback to Classic pybind11
------------------------------------------------------ ------------------------------------------------------------
This could be viewed as super-conservative mode, for situations in which For situations in which compatibility with Classic pybind11
compatibility with classic pybind11 (without smart_holder) is needed for (without smart_holder) is needed for some period of time, fallback
some period of time. The main idea is to enable use of ``py::classh`` to Classic mode can be enabled by copying the ``BOILERPLATE`` code
and the associated ``PYBIND11_SMART_HOLDER_TYPE_CASTERS`` macro while block from tests/test_classh_mock.cpp. This code block provides mock
still being able to build the same code with classic pybind11. Please see implementations of ``py::classh`` and the family of related macros
tests/test_classh_mock.cpp for an example. (e.g. ``PYBIND11_SMART_HOLDER_TYPE_CASTERS``).
Classic / Conservative / Progressive cross-module compatibility Classic / Conservative / Progressive cross-module compatibility
@ -268,7 +304,7 @@ inherit from ``py::trampoline_self_life_support``, for example:
... ...
}; };
This is the only difference compared to classic pybind11. A fairly This is the only difference compared to Classic pybind11. A fairly
minimal but complete example is tests/test_class_sh_trampoline_unique_ptr.cpp. minimal but complete example is tests/test_class_sh_trampoline_unique_ptr.cpp.
@ -277,7 +313,7 @@ Ideas for the long-term
The macros are clearly an inconvenience in many situations. Highly The macros are clearly an inconvenience in many situations. Highly
speculative: to avoid the need for the macros, a potential approach would speculative: to avoid the need for the macros, a potential approach would
be to combine the classic implementation (``type_caster_base``) with be to combine the Classic implementation (``type_caster_base``) with
the ``smart_holder_type_caster``, but this will probably be very messy and the ``smart_holder_type_caster``, but this will probably be very messy and
not great as a long-term solution. The ``type_caster_base`` code is very not great as a long-term solution. The ``type_caster_base`` code is very
complex already. A more maintainable approach long-term could be to work complex already. A more maintainable approach long-term could be to work

View File

@ -179,7 +179,7 @@ template <
void construct(value_and_holder &v_h, std::unique_ptr<Cpp<Class>, D> &&unq_ptr, bool need_alias) { void construct(value_and_holder &v_h, std::unique_ptr<Cpp<Class>, D> &&unq_ptr, bool need_alias) {
auto *ptr = unq_ptr.get(); auto *ptr = unq_ptr.get();
no_nullptr(ptr); no_nullptr(ptr);
if (Class::has_alias && need_alias) if (Class::has_alias && need_alias && !is_alias<Class>(ptr))
throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee " throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee "
"is not an alias instance"); "is not an alias instance");
auto smhldr auto smhldr
@ -209,7 +209,7 @@ template <
void construct(value_and_holder &v_h, std::shared_ptr<Cpp<Class>> &&shd_ptr, bool need_alias) { void construct(value_and_holder &v_h, std::shared_ptr<Cpp<Class>> &&shd_ptr, bool need_alias) {
auto *ptr = shd_ptr.get(); auto *ptr = shd_ptr.get();
no_nullptr(ptr); no_nullptr(ptr);
if (Class::has_alias && need_alias) if (Class::has_alias && need_alias && !is_alias<Class>(ptr))
throw type_error("pybind11::init(): construction failed: returned std::shared_ptr pointee " throw type_error("pybind11::init(): construction failed: returned std::shared_ptr pointee "
"is not an alias instance"); "is not an alias instance");
auto smhldr = type_caster<Cpp<Class>>::template smart_holder_from_shared_ptr(shd_ptr); auto smhldr = type_caster<Cpp<Class>>::template smart_holder_from_shared_ptr(shd_ptr);

View File

@ -1217,12 +1217,30 @@ template <typename T>
using default_holder_type = std::unique_ptr<T>; using default_holder_type = std::unique_ptr<T>;
# ifndef PYBIND11_SH_AVL
# define PYBIND11_SH_AVL(...) std::shared_ptr<__VA_ARGS__> // "Smart_Holder if AVaiLable"
// -------- std::shared_ptr(...) -- same length by design, to not disturb the indentation
// of existing code.
# endif
# define PYBIND11_SH_DEF(...) std::shared_ptr<__VA_ARGS__> // "Smart_Holder if DEFault"
// -------- std::shared_ptr(...) -- same length by design, to not disturb the indentation
// of existing code.
# define PYBIND11_TYPE_CASTER_BASE_HOLDER(T, ...) # define PYBIND11_TYPE_CASTER_BASE_HOLDER(T, ...)
#else #else
using default_holder_type = smart_holder; using default_holder_type = smart_holder;
# ifndef PYBIND11_SH_AVL
# define PYBIND11_SH_AVL(...) ::pybind11::smart_holder // "Smart_Holder if AVaiLable"
// -------- std::shared_ptr(...) -- same length by design, to not disturb the indentation
// of existing code.
# endif
# define PYBIND11_SH_DEF(...) ::pybind11::smart_holder // "Smart_Holder if DEFault"
// This define could be hidden away inside detail/smart_holder_type_casters.h, but is kept here // This define could be hidden away inside detail/smart_holder_type_casters.h, but is kept here
// for clarity. // for clarity.
# define PYBIND11_TYPE_CASTER_BASE_HOLDER(T, ...) \ # define PYBIND11_TYPE_CASTER_BASE_HOLDER(T, ...) \

View File

@ -8,6 +8,12 @@
#include "detail/smart_holder_type_casters.h" #include "detail/smart_holder_type_casters.h"
#include "pybind11.h" #include "pybind11.h"
#undef PYBIND11_SH_AVL // Undoing #define in pybind11.h
#define PYBIND11_SH_AVL(...) ::pybind11::smart_holder // "Smart_Holder if AVaiLable"
// ---- std::shared_ptr(...) -- same length by design, to not disturb the indentation
// of existing code.
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
// Supports easier switching between py::class_<T> and py::class_<T, py::smart_holder>: // Supports easier switching between py::class_<T> and py::class_<T, py::smart_holder>:

View File

@ -14,6 +14,12 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
template <typename type_, typename... options> template <typename type_, typename... options>
using classh = class_<type_, options...>; using classh = class_<type_, options...>;
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
# ifndef PYBIND11_SH_AVL
# define PYBIND11_SH_AVL(...) std::shared_ptr<__VA_ARGS__> // "Smart_Holder if AVaiLable"
# endif
# ifndef PYBIND11_SH_DEF
# define PYBIND11_SH_DEF(...) std::shared_ptr<__VA_ARGS__> // "Smart_Holder if DEFault"
# endif
# ifndef PYBIND11_SMART_HOLDER_TYPE_CASTERS # ifndef PYBIND11_SMART_HOLDER_TYPE_CASTERS
# define PYBIND11_SMART_HOLDER_TYPE_CASTERS(...) # define PYBIND11_SMART_HOLDER_TYPE_CASTERS(...)
# endif # endif
@ -24,23 +30,42 @@ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
// BOILERPLATE END // BOILERPLATE END
namespace { namespace {
struct Foo0 {}; struct FooUc {};
struct Foo1 {}; struct FooUp {};
struct Foo2 {}; struct FooSa {};
struct FooSc {};
struct FooSp {};
} // namespace } // namespace
PYBIND11_TYPE_CASTER_BASE_HOLDER(Foo1, std::shared_ptr<Foo1>) PYBIND11_SMART_HOLDER_TYPE_CASTERS(FooUp)
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo2) PYBIND11_SMART_HOLDER_TYPE_CASTERS(FooSp)
PYBIND11_TYPE_CASTER_BASE_HOLDER(FooSa, std::shared_ptr<FooSa>)
TEST_SUBMODULE(classh_mock, m) { TEST_SUBMODULE(classh_mock, m) {
// Uses std::unique_ptr<Foo0> as holder in conservative mode, py::smart_holder in progressive // Please see README_smart_holder.rst, in particular section
// mode (if available). // Classic / Conservative / Progressive cross-module compatibility
py::class_<Foo0>(m, "Foo0").def(py::init<>());
// Always uses std::shared_ptr<Foo1> as holder. // Uses std::unique_ptr<FooUc> as holder in Classic or Conservative mode, py::smart_holder in
py::class_<Foo1, std::shared_ptr<Foo1>>(m, "Foo1").def(py::init<>()); // Progressive mode.
py::class_<FooUc>(m, "FooUc").def(py::init<>());
// Uses py::smart_holder if available, or std::unique_ptr<Foo2> if only pybind11 classic is // Uses std::unique_ptr<FooUp> as holder in Classic mode, py::smart_holder in Conservative or
// available. // Progressive mode.
py::classh<Foo2>(m, "Foo2").def(py::init<>()); py::classh<FooUp>(m, "FooUp").def(py::init<>());
// Always uses std::shared_ptr<FooSa> as holder.
py::class_<FooSa, std::shared_ptr<FooSa>>(m, "FooSa").def(py::init<>());
// Uses std::shared_ptr<FooSc> as holder in Classic or Conservative mode, py::smart_holder in
// Progressive mode.
py::class_<FooSc, PYBIND11_SH_DEF(FooSc)>(m, "FooSc").def(py::init<>());
// -------------- std::shared_ptr<FooSc> -- same length by design, to not disturb the
// indentation of existing code.
// Uses std::shared_ptr<FooSp> as holder in Classic mode, py::smart_holder in Conservative or
// Progressive mode.
py::class_<FooSp, PYBIND11_SH_AVL(FooSp)>(m, "FooSp").def(py::init<>());
// -------------- std::shared_ptr<FooSp> -- same length by design, to not disturb the
// indentation of existing code.
} }

View File

@ -6,6 +6,8 @@ from pybind11_tests import classh_mock as m
def test_foobar(): def test_foobar():
# Not really testing anything in particular. The main purpose of this test is to ensure the # Not really testing anything in particular. The main purpose of this test is to ensure the
# suggested BOILERPLATE code block in test_classh_mock.cpp is correct. # suggested BOILERPLATE code block in test_classh_mock.cpp is correct.
assert m.Foo0() assert m.FooUc()
assert m.Foo1() assert m.FooUp()
assert m.Foo2() assert m.FooSa()
assert m.FooSc()
assert m.FooSp()

View File

@ -139,11 +139,6 @@ public:
static std::shared_ptr<TestFactory3> construct3(int a) { return std::shared_ptr<TestFactory3>(new TestFactory3(a)); } static std::shared_ptr<TestFactory3> construct3(int a) { return std::shared_ptr<TestFactory3>(new TestFactory3(a)); }
}; };
PYBIND11_TYPE_CASTER_BASE_HOLDER(TestFactory3, std::shared_ptr<TestFactory3>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(TestFactory4, std::shared_ptr<TestFactory4>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(TestFactory5, std::shared_ptr<TestFactory5>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(TestFactory7, std::shared_ptr<TestFactory7>)
TEST_SUBMODULE(factory_constructors, m) { TEST_SUBMODULE(factory_constructors, m) {
// Define various trivial types to allow simpler overload resolution: // Define various trivial types to allow simpler overload resolution:
@ -188,7 +183,7 @@ TEST_SUBMODULE(factory_constructors, m) {
auto c4a = [c](pointer_tag, TF4_tag, int a) { (void) c; return new TestFactory4(a);}; auto c4a = [c](pointer_tag, TF4_tag, int a) { (void) c; return new TestFactory4(a);};
// test_init_factory_basic, test_init_factory_casting // test_init_factory_basic, test_init_factory_casting
py::class_<TestFactory3, std::shared_ptr<TestFactory3>> pyTestFactory3(m, "TestFactory3"); py::class_<TestFactory3, PYBIND11_SH_DEF(TestFactory3)> pyTestFactory3(m, "TestFactory3");
pyTestFactory3 pyTestFactory3
.def(py::init([](pointer_tag, int v) { return TestFactoryHelper::construct3(v); })) .def(py::init([](pointer_tag, int v) { return TestFactoryHelper::construct3(v); }))
.def(py::init([](shared_ptr_tag) { return TestFactoryHelper::construct3(); })); .def(py::init([](shared_ptr_tag) { return TestFactoryHelper::construct3(); }));
@ -212,12 +207,12 @@ TEST_SUBMODULE(factory_constructors, m) {
; ;
// test_init_factory_casting // test_init_factory_casting
py::class_<TestFactory4, TestFactory3, std::shared_ptr<TestFactory4>>(m, "TestFactory4") py::class_<TestFactory4, TestFactory3, PYBIND11_SH_DEF(TestFactory4)>(m, "TestFactory4")
.def(py::init(c4a)) // pointer .def(py::init(c4a)) // pointer
; ;
// Doesn't need to be registered, but registering makes getting ConstructorStats easier: // Doesn't need to be registered, but registering makes getting ConstructorStats easier:
py::class_<TestFactory5, TestFactory3, std::shared_ptr<TestFactory5>>(m, "TestFactory5"); py::class_<TestFactory5, TestFactory3, PYBIND11_SH_DEF(TestFactory5)>(m, "TestFactory5");
// test_init_factory_alias // test_init_factory_alias
// Alias testing // Alias testing
@ -238,7 +233,7 @@ TEST_SUBMODULE(factory_constructors, m) {
// test_init_factory_dual // test_init_factory_dual
// Separate alias constructor testing // Separate alias constructor testing
py::class_<TestFactory7, PyTF7, std::shared_ptr<TestFactory7>>(m, "TestFactory7") py::class_<TestFactory7, PyTF7, PYBIND11_SH_DEF(TestFactory7)>(m, "TestFactory7")
.def(py::init( .def(py::init(
[](int i) { return TestFactory7(i); }, [](int i) { return TestFactory7(i); },
[](int i) { return PyTF7(i); })) [](int i) { return PyTF7(i); }))

View File

@ -280,10 +280,11 @@ def test_init_factory_dual():
assert not g1.has_alias() assert not g1.has_alias()
with pytest.raises(TypeError) as excinfo: with pytest.raises(TypeError) as excinfo:
PythFactory7(tag.shared_ptr, tag.invalid_base, 14) PythFactory7(tag.shared_ptr, tag.invalid_base, 14)
assert ( assert str(excinfo.value) in (
str(excinfo.value) "pybind11::init(): construction failed: returned holder-wrapped instance is not an "
== "pybind11::init(): construction failed: returned holder-wrapped instance is not an " "alias instance",
"alias instance" "pybind11::init(): construction failed: returned std::shared_ptr pointee is not an "
"alias instance",
) )
assert [i.alive() for i in cstats] == [13, 7] assert [i.alive() for i in cstats] == [13, 7]

View File

@ -113,7 +113,7 @@ class NoneTester { public: int answer = 42; };
int none1(const NoneTester &obj) { return obj.answer; } int none1(const NoneTester &obj) { return obj.answer; }
int none2(NoneTester *obj) { return obj ? obj->answer : -1; } int none2(NoneTester *obj) { return obj ? obj->answer : -1; }
int none3(const std::shared_ptr<NoneTester> &obj) { return obj ? obj->answer : -1; } int none3(const std::shared_ptr<NoneTester> &obj) { return obj ? obj->answer : -1; }
int none4(const std::shared_ptr<NoneTester> *obj) { return obj && *obj ? (*obj)->answer : -1; } int none4(std::shared_ptr<NoneTester> *obj) { return obj && *obj ? (*obj)->answer : -1; }
int none5(std::shared_ptr<NoneTester> obj) { return obj ? obj->answer : -1; } int none5(std::shared_ptr<NoneTester> obj) { return obj ? obj->answer : -1; }
struct StrIssue { struct StrIssue {
@ -148,8 +148,6 @@ struct RefQualified {
int constRefQualified(int other) const & { return value + other; } int constRefQualified(int other) const & { return value + other; }
}; };
PYBIND11_TYPE_CASTER_BASE_HOLDER(NoneTester, std::shared_ptr<NoneTester>)
TEST_SUBMODULE(methods_and_attributes, m) { TEST_SUBMODULE(methods_and_attributes, m) {
// test_methods_and_attributes // test_methods_and_attributes
py::class_<ExampleMandA> emna(m, "ExampleMandA"); py::class_<ExampleMandA> emna(m, "ExampleMandA");
@ -327,19 +325,23 @@ TEST_SUBMODULE(methods_and_attributes, m) {
// [workaround(intel)] ICC 20/21 breaks with py::arg().stuff, using py::arg{}.stuff works. // [workaround(intel)] ICC 20/21 breaks with py::arg().stuff, using py::arg{}.stuff works.
// test_accepts_none // test_accepts_none
py::class_<NoneTester, std::shared_ptr<NoneTester>>(m, "NoneTester") py::class_<NoneTester, PYBIND11_SH_DEF(NoneTester)>(m, "NoneTester")
.def(py::init<>()); .def(py::init<>());
m.def("no_none1", &none1, py::arg{}.none(false)); m.def("no_none1", &none1, py::arg{}.none(false));
m.def("no_none2", &none2, py::arg{}.none(false)); m.def("no_none2", &none2, py::arg{}.none(false));
m.def("no_none3", &none3, py::arg{}.none(false)); m.def("no_none3", &none3, py::arg{}.none(false));
m.def("no_none4", &none4, py::arg{}.none(false));
m.def("no_none5", &none5, py::arg{}.none(false)); m.def("no_none5", &none5, py::arg{}.none(false));
m.def("ok_none1", &none1); m.def("ok_none1", &none1);
m.def("ok_none2", &none2, py::arg{}.none(true)); m.def("ok_none2", &none2, py::arg{}.none(true));
m.def("ok_none3", &none3); m.def("ok_none3", &none3);
m.def("ok_none4", &none4, py::arg{}.none(true));
m.def("ok_none5", &none5); m.def("ok_none5", &none5);
#ifndef PYBIND11_USE_SMART_HOLDER_AS_DEFAULT
// smart_holder_type_caster does not support conversion to `const shared_ptr<T> *`.
m.def("no_none4", &none4, py::arg{}.none(false));
m.def("ok_none4", &none4, py::arg{}.none(true));
#endif
m.def("no_none_kwarg", &none2, "a"_a.none(false)); m.def("no_none_kwarg", &none2, "a"_a.none(false));
m.def("no_none_kwarg_kw_only", &none2, py::kw_only(), "a"_a.none(false)); m.def("no_none_kwarg_kw_only", &none2, py::kw_only(), "a"_a.none(false));

View File

@ -374,12 +374,10 @@ def test_accepts_none(msg):
assert m.no_none1(a) == 42 assert m.no_none1(a) == 42
assert m.no_none2(a) == 42 assert m.no_none2(a) == 42
assert m.no_none3(a) == 42 assert m.no_none3(a) == 42
assert m.no_none4(a) == 42
assert m.no_none5(a) == 42 assert m.no_none5(a) == 42
assert m.ok_none1(a) == 42 assert m.ok_none1(a) == 42
assert m.ok_none2(a) == 42 assert m.ok_none2(a) == 42
assert m.ok_none3(a) == 42 assert m.ok_none3(a) == 42
assert m.ok_none4(a) == 42
assert m.ok_none5(a) == 42 assert m.ok_none5(a) == 42
with pytest.raises(TypeError) as excinfo: with pytest.raises(TypeError) as excinfo:
@ -391,9 +389,6 @@ def test_accepts_none(msg):
with pytest.raises(TypeError) as excinfo: with pytest.raises(TypeError) as excinfo:
m.no_none3(None) m.no_none3(None)
assert "incompatible function arguments" in str(excinfo.value) assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.no_none4(None)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo: with pytest.raises(TypeError) as excinfo:
m.no_none5(None) m.no_none5(None)
assert "incompatible function arguments" in str(excinfo.value) assert "incompatible function arguments" in str(excinfo.value)
@ -414,7 +409,6 @@ def test_accepts_none(msg):
# The rest take the argument as pointer or holder, and accept None: # The rest take the argument as pointer or holder, and accept None:
assert m.ok_none2(None) == -1 assert m.ok_none2(None) == -1
assert m.ok_none3(None) == -1 assert m.ok_none3(None) == -1
assert m.ok_none4(None) == -1
assert m.ok_none5(None) == -1 assert m.ok_none5(None) == -1
with pytest.raises(TypeError) as excinfo: with pytest.raises(TypeError) as excinfo:
@ -430,6 +424,14 @@ def test_accepts_none(msg):
m.no_none_kwarg_kw_only(a=None) m.no_none_kwarg_kw_only(a=None)
assert "incompatible function arguments" in str(excinfo.value) assert "incompatible function arguments" in str(excinfo.value)
if hasattr(m, "no_none4"):
assert m.no_none4(a) == 42
assert m.ok_none4(a) == 42
with pytest.raises(TypeError) as excinfo:
m.no_none4(None)
assert "incompatible function arguments" in str(excinfo.value)
assert m.ok_none4(None) == -1
def test_str_issue(msg): def test_str_issue(msg):
"""#283: __str__ called on uninitialized instance when constructor arguments invalid""" """#283: __str__ called on uninitialized instance when constructor arguments invalid"""

View File

@ -69,14 +69,6 @@ struct I801D : I801C {}; // Indirect MI
} // namespace } // namespace
PYBIND11_TYPE_CASTER_BASE_HOLDER(Base1a, std::shared_ptr<Base1a>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(Base2a, std::shared_ptr<Base2a>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(Base12a, std::shared_ptr<Base12a>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(I801B1, std::shared_ptr<I801B1>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(I801B2, std::shared_ptr<I801B2>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(I801C, std::shared_ptr<I801C>)
PYBIND11_TYPE_CASTER_BASE_HOLDER(I801D, std::shared_ptr<I801D>)
TEST_SUBMODULE(multiple_inheritance, m) { TEST_SUBMODULE(multiple_inheritance, m) {
// Please do not interleave `struct` and `class` definitions with bindings code, // Please do not interleave `struct` and `class` definitions with bindings code,
// but implement `struct`s and `class`es in the anonymous namespace above. // but implement `struct`s and `class`es in the anonymous namespace above.
@ -136,16 +128,16 @@ TEST_SUBMODULE(multiple_inheritance, m) {
// test_multiple_inheritance_virtbase // test_multiple_inheritance_virtbase
// Test the case where not all base classes are specified, and where pybind11 requires the // Test the case where not all base classes are specified, and where pybind11 requires the
// py::multiple_inheritance flag to perform proper casting between types. // py::multiple_inheritance flag to perform proper casting between types.
py::class_<Base1a, std::shared_ptr<Base1a>>(m, "Base1a") py::class_<Base1a, PYBIND11_SH_DEF(Base1a)>(m, "Base1a")
.def(py::init<int>()) .def(py::init<int>())
.def("foo", &Base1a::foo); .def("foo", &Base1a::foo);
py::class_<Base2a, std::shared_ptr<Base2a>>(m, "Base2a") py::class_<Base2a, PYBIND11_SH_DEF(Base2a)>(m, "Base2a")
.def(py::init<int>()) .def(py::init<int>())
.def("bar", &Base2a::bar); .def("bar", &Base2a::bar);
py::class_<Base12a, /* Base1 missing */ Base2a, py::class_<Base12a, /* Base1 missing */ Base2a,
std::shared_ptr<Base12a>>(m, "Base12a", py::multiple_inheritance()) PYBIND11_SH_DEF(Base12a)>(m, "Base12a", py::multiple_inheritance())
.def(py::init<int, int>()); .def(py::init<int, int>());
m.def("bar_base2a", [](Base2a *b) { return b->bar(); }); m.def("bar_base2a", [](Base2a *b) { return b->bar(); });
@ -158,10 +150,10 @@ TEST_SUBMODULE(multiple_inheritance, m) {
struct I801B3 { int c = 3; virtual ~I801B3() = default; }; struct I801B3 { int c = 3; virtual ~I801B3() = default; };
struct I801E : I801B3, I801D {}; struct I801E : I801B3, I801D {};
py::class_<I801B1, std::shared_ptr<I801B1>>(m, "I801B1").def(py::init<>()).def_readonly("a", &I801B1::a); py::class_<I801B1, PYBIND11_SH_DEF(I801B1)>(m, "I801B1").def(py::init<>()).def_readonly("a", &I801B1::a);
py::class_<I801B2, std::shared_ptr<I801B2>>(m, "I801B2").def(py::init<>()).def_readonly("b", &I801B2::b); py::class_<I801B2, PYBIND11_SH_DEF(I801B2)>(m, "I801B2").def(py::init<>()).def_readonly("b", &I801B2::b);
py::class_<I801C, I801B1, I801B2, std::shared_ptr<I801C>>(m, "I801C").def(py::init<>()); py::class_<I801C, I801B1, I801B2, PYBIND11_SH_DEF(I801C)>(m, "I801C").def(py::init<>());
py::class_<I801D, I801C, std::shared_ptr<I801D>>(m, "I801D").def(py::init<>()); py::class_<I801D, I801C, PYBIND11_SH_DEF(I801D)>(m, "I801D").def(py::init<>());
// Two separate issues here: first, we want to recognize a pointer to a base type as being a // Two separate issues here: first, we want to recognize a pointer to a base type as being a
// known instance even when the pointer value is unequal (i.e. due to a non-first // known instance even when the pointer value is unequal (i.e. due to a non-first