GitLab

Minor change that makes this example more compliant with the C++ Core
Guidelines.

This allows a slightly cleaner base type specification of:

    py::class_<Type, Base>("Type")

as an alternative to

    py::class_<Type>("Type", py::base<Base>())

As with the other template parameters, the order relative to the holder
or trampoline types doesn't matter.

This also includes a compile-time assertion failure if attempting to
specify more than one base class (but is easily extendible to support
multiple inheritance, someday, by updating the class_selector::set_bases
function to set multiple bases).

With this change both C++ and Python write to sys.stdout which resolves
the capture issues noted in #351. Therefore, the related workarounds are
removed.

The current pybind11::class_<Type, Holder, Trampoline> fixed template
ordering results in a requirement to repeat the Holder with its default
value (std::unique_ptr<Type>) argument, which is a little bit annoying:
it needs to be specified not because we want to override the default,
but rather because we need to specify the third argument.

This commit removes this limitation by making the class_ template take
the type name plus a parameter pack of options.  It then extracts the
first valid holder type and the first subclass type for holder_type and
trampoline type_alias, respectively.  (If unfound, both fall back to
their current defaults, `std::unique_ptr<type>` and `type`,
respectively).  If any unmatched template arguments are provided, a
static assertion fails.

What this means is that you can specify or omit the arguments in any
order:

    py::class_<A, PyA> c1(m, "A");
    py::class_<B, PyB, std::shared_ptr<B>> c2(m, "B");
    py::class_<C, std::shared_ptr<C>, PyB> c3(m, "C");

It also allows future class attributes (such as base types in the next
commit) to be passed as class template types rather than needing to use
a py::base<> wrapper.

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.

This commit rewrites the examples that look for constructor/destructor
calls to do so via static variable tracking rather than output parsing.

The added ConstructorStats class provides methods to keep track of
constructors and destructors, number of default/copy/move constructors,
and number of copy/move assignments.  It also provides a mechanism for
storing values (e.g. for value construction), and then allows all of
this to be checked at the end of a test by getting the statistics for a
C++ (or python mapping) class.

By not relying on the precise pattern of constructions/destructions,
but rather simply ensuring that every construction is matched with a
destruction on the same object, we ensure that everything that gets
created also gets destroyed as expected.

This replaces all of the various "std::cout << whatever" code in
constructors/destructors with
`print_created(this)`/`print_destroyed(this)`/etc. functions which
provide similar output, but now has a unified format across the
different examples, including a new ### prefix that makes mixed example
output and lifecycle events easier to distinguish.

With this change, relaxed mode is no longer needed, which enables
testing for proper destruction under MSVC, and under any other compiler
that generates code calling extra constructors, or optimizes away any
constructors.  GCC/clang are used as the baseline for move
constructors; the tests are adapted to allow more move constructors to
be evoked (but other types are constructors much have matching counts).

This commit also disables output buffering of tests, as the buffering
sometimes results in C++ output ending up in the middle of python
output (or vice versa), depending on the OS/python version.

Currently pybind11 always translates values returned by Python functions
invoked from C++ code by copying, even when moving is feasible--and,
more importantly, even when moving is required.

The first, and relatively minor, concern is that moving may be
considerably more efficient for some types.  The second problem,
however, is more serious: there's currently no way python code can
return a non-copyable type to C++ code.

I ran into this while trying to add a PYBIND11_OVERLOAD of a virtual
method that returns just such a type: it simply fails to compile because
this:

    overload = ...
    overload(args).template cast<ret_type>();

involves a copy: overload(args) returns an object instance, and the
invoked object::cast() loads the returned value, then returns a copy of
the loaded value.

We can, however, safely move that returned value *if* the object has the
only reference to it (i.e. if ref_count() == 1) and the object is
itself temporary (i.e. if it's an rvalue).

This commit does that by adding an rvalue-qualified object::cast()
method that allows the returned value to be move-constructed out of the
stored instance when feasible.

This basically comes down to three cases:

- For objects that are movable but not copyable, we always try the move,
  with a runtime exception raised if this would involve moving a value
  with multiple references.
- When the type is both movable and non-trivially copyable, the move
  happens only if the invoked object has a ref_count of 1, otherwise the
  object is copied.  (Trivially copyable types are excluded from this
  case because they are typically just collections of primitive types,
  which can be copied just as easily as they can be moved.)
- Non-movable and trivially copy constructible objects are simply
  copied.

This also adds examples to example-virtual-functions that shows both a
non-copyable object and a movable/copyable object in action: the former
raises an exception if returned while holding a reference, the latter
invokes a move constructor if unreferenced, or a copy constructor if
referenced.

Basically this allows code such as:

    class MyClass(Pybind11Class):
        def somemethod(self, whatever):
            mt = MovableType(whatever)
            # ...
            return mt

which allows the MovableType instance to be returned to the C++ code
via its move constructor.

Of course if you attempt to violate this by doing something like:

    self.value = MovableType(whatever)
    return self.value

you get an exception--but right now, the pybind11-side of that code
won't compile at all.

It was already pretty badly intrusive, but it also appears to make MSVC
segfault.  Rather than investigating and fixing it, it's easier to just
remove it.

As discussed in #320.

The adds a documentation block that mentions that the trampoline classes
must provide overrides for both the classes' own virtual methods *and*
any inherited virtual methods.  It also provides a templated solution to
avoiding method duplication.

The example includes a third method (only mentioned in the "see also"
section of the documentation addition), using multiple inheritance.
While this approach works, and avoids code generation in deep
hierarchies, it is intrusive by requiring that the wrapped classes use
virtual inheritance, which itself is more instrusive if any of the
virtual base classes need anything other than default constructors.  As
per the discussion in #320, it is kept as an example, but not suggested
in the documentation.

This renames example files from `exampleN` to `example-description`.

Specifically, the following renaming is applied:

example1 -> example-methods-and-attributes
example2 -> example-python-types
example3 -> example-operator-overloading
example4 -> example-constants-and-functions
example5 -> example-callbacks (*)
example6 -> example-sequence-and-iterators
example7 -> example-buffers
example8 -> example-custom-ref-counting
example9 -> example-modules
example10 -> example-numpy-vectorize
example11 -> example-arg-keywords-and-defaults
example12 -> example-virtual-functions
example13 -> example-keep-alive
example14 -> example-opaque-types
example15 -> example-pickling
example16 -> example-inheritance
example17 -> example-stl-binders
example18 -> example-eval
example19 -> example-custom-exceptions

* the inheritance parts of example5 are moved into example-inheritance
(previously example16), and the remainder is left as example-callbacks.

This commit also renames the internal variables ("Example1",
"Example2", "Example4", etc.) into non-numeric names ("ExampleMandA",
"ExamplePythonTypes", "ExampleWithEnum", etc.) to correspond to the
file renaming.

The order of tests is preserved, but this can easily be changed if
there is some more natural ordering by updating the list in
examples/CMakeLists.txt.

Sergey Lyskov pointed out that the trampoline mechanism used to override
virtual methods from within Python caused unnecessary overheads when
instantiating the original (i.e. non-extended) class.

This commit removes this inefficiency, but some syntax changes were
needed to achieve this. Projects using this features will need to make a
few changes:

In particular, the example below shows the old syntax to instantiate a
class with a trampoline:

class_<TrampolineClass>("MyClass")
    .alias<MyClass>()
    ....

This is what should be used now:

class_<MyClass, std::unique_ptr<MyClass, TrampolineClass>("MyClass")
    ....

Importantly, the trampoline class is now specified as the *third*
argument to the class_ template, and the alias<..>() call is gone. The
second argument with the unique pointer is simply the default holder
type used by pybind11.