This makes the output considerably easier to use: it now highlights (in
red) matched tabs (instead of just listing the filenames), and adds
line numbers to both the tabs check and the space-less if check outputs.
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.
The check-style exit status wasn't being propagated properly because
the loops were running in a subshell (and so the change the the
`errors` variable wasn't in the active command shell). This fixes it
by running the greps in subshells and the loops in the main shell.
This also avoids the if(/for(/while( style check on
tests/CMakeLists.txt, since it *does* have if() statements with no space
that are producing error messages, but that is (acceptable) CMake style.
This adds a tool that checks style (currently just for tabs instead of
spaces in files under include/tests/docs) and produces a travis-ci build
failure if any problems are found.
Fixes#365. `sysconfig.get_config_var('SO')` already returns the correct
PYTHON_MODULE_EXTENSION, even for debug builds, so there is no need to
add anything else manually.
Installing something outside the project directory from a cmake
invocation is overly intrusive; this changes tests/CMakeLists.txt to
just fail with an informative message instead, and changes the
travis-ci builds to install pytest via pip or apt-get.
ccache on Travis was never configured properly so the setting never
actually did anything. Enabling ccache for real brings other issues:
due to the way the preprocessor is handled, some of the Python header
macros produce bogus compiler warnings (which in turn produce errors
with -Werror). ccache also requires additional configuration on OS X
and docker. It would reduce compile time by ~30 seconds at best, so
it's not worth the trouble.
[skip appveyor]
This build makes sure everything still works without optional
dependencies (numpy/scipy/eigen) and also tests the automatic
discovery functions in CMake (Python version, C++ standard).
[skip appveyor]
- 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.
pytest can capture test output both globally (controlled by the cmd line
flag --capture) or locally (`capsys` and `capfd` fixtures). Enabling both
methods at the same time causes problems on Windows: test output is not
captured sometimes, resulting in test failure. This happens seemingly at
random.
This workaround disables global output capture ("-s", i.e. "--capture=no")
leaving only the local capture fixtures. As a side-effect test output on
AppVeyor CI is a little messy, but this will have to do until a better
solution is found.
Test compilation instructions for Windows were changed to use the
`cmake --build` command line invocation which should be easier than
manually setting up using the CMake GUI and Visual Studio.
Jason Newton
Wenzel Jakob
Jason Rhinelander
Nickolai Belakovski
Dean Moldovan
Ivan Smirnov