Adding or removing tests is a little bit cumbersome currently: the test
needs to be added to CMakeLists.txt, the init function needs to be
predeclared in pybind11_tests.cpp, then called in the plugin
initialization. While this isn't a big deal for tests that are being
committed, it's more of a hassle when working on some new feature or
test code for which I temporarily only care about building and linking
the test being worked on rather than the entire test suite.
This commit changes tests to self-register their initialization by
having each test initialize a local object (which stores the
initialization function in a static variable). This makes changing the
set of tests being build easy: one only needs to add or comment out
test names in tests/CMakeLists.txt.
A couple other minor changes that go along with this:
- test_eigen.cpp is now included in the test list, then removed if eigen
isn't available. This lets you disable the eigen tests by commenting
it out, just like all the other tests, but keeps the build working
without eigen eigen isn't available. (Also, if it's commented out, we
don't even bother looking for and reporting the building with/without
eigen status message).
- pytest is now invoked with all the built test names (with .cpp changed
to .py) so that it doesn't try to run tests that weren't built.
Problem
=======
The template trampoline pattern documented in PR #322 has a problem with
virtual method overloads in intermediate classes in the inheritance
chain between the trampoline class and the base class.
For example, consider the following inheritance structure, where `B` is
the actual class, `PyB<B>` is the trampoline class, and `PyA<B>` is an
intermediate class adding A's methods into the trampoline:
PyB<B> -> PyA<B> -> B -> A
Suppose PyA<B> has a method `some_method()` with a PYBIND11_OVERLOAD in
it to overload the virtual `A::some_method()`. If a Python class `C` is
defined that inherits from the pybind11-registered `B` and tries to
provide an overriding `some_method()`, the PYBIND11_OVERLOADs declared
in PyA<B> fails to find this overloaded method, and thus never invoke it
(or, if pure virtual and not overridden in PyB<B>, raises an exception).
This happens because the base (internal) `PYBIND11_OVERLOAD_INT` macro
simply calls `get_overload(this, name)`; `get_overload()` then uses the
inferred type of `this` to do a type lookup in `registered_types_cpp`.
This is where it fails: `this` will be a `PyA<B> *`, but `PyA<B>` is
neither the base type (`B`) nor the trampoline type (`PyB<B>`). As a
result, the overload fails and we get a failed overload lookup.
The fix
=======
The fix is relatively simple: we can cast `this` passed to
`get_overload()` to a `const B *`, which lets get_overload look up the
correct class. Since trampoline classes should be derived from `B`
classes anyway, this cast should be perfectly safe.
This does require adding the class name as an argument to the
PYBIND11_OVERLOAD_INT macro, but leaves the public macro signatures
unchanged.
- ICPC can't handle the NCVirt trampoline which returns a non-copyable
type, which is likely due to a constexpr/SFINAE issue. This disables
the test on that compiler so that at least the rest can be tested.
Use simple asserts and pytest's powerful introspection to make testing
simpler. This merges the old .py/.ref file pairs into simple .py files
where the expected values are right next to the code being tested.
This commit does not touch the C++ part of the code and replicates the
Python tests exactly like the old .ref-file-based approach.
