Various C++17 fixes #555

jagerman · 2016-12-14T03:16:22Z

This PR consists of a few commits to make pybind11 compile under -std=c++17 (or -std=c++1z for clang), and adds a gcc7 build (using a recent snapshot from debian experimental) with build failures permitted.

(The commit removing convert doesn't have anything to do with C++17, but I noticed it when moving the function call from a lambda to a static class method).

-Wint-in-bool-context triggers many warnings when compiling eigen code, so disable it locally in eigen.h.

wjakob · 2016-12-14T03:42:48Z

Regarding the "Work around GCC 7 ICE" commit: I liked the version with the inline lambda much better :(. Shouldn't we wait until these compilers are actually reliable rather than changing the code to allow them to compile pybind11?

A somewhat related rant: I suspect that Clang is (at least for now) a better reference to use as a standards compliant c++17 compiler. My personal sample point is that I've reported a ton of frontend & backend bugs to LLVM & GCC this last summer. Fast forward to now: all of the LLVM issues are resolved, while almost all of the GCC ones are still unaddressed and some of them have zero discussion.

wjakob · 2016-12-14T03:46:17Z

Good catch about the noexcept function signatures, that's a very nice addition.

jagerman · 2016-12-14T04:08:18Z

Any idea why the DOCS failure is happening?

wjakob · 2016-12-14T04:13:21Z

$ flake8
./tests/test_constants_and_functions.py:26:1: E302 expected 2 blank lines, found 1
def test_exception_specifiers():

jagerman · 2016-12-14T04:21:27Z

Oh! I was looking at the

/home/travis/build/pybind/pybind11/docs/reference.rst:60: WARNING: cpp:class targets a function (handle::handle).

/home/travis/build/pybind/pybind11/docs/reference.rst:158: WARNING: cpp:class targets a function (object::object).

/home/travis/build/pybind/pybind11/docs/reference.rst:163: WARNING: cpp:class targets a function (object::object).

warnings and didn't notice the failure later.

jagerman · 2016-12-14T05:23:40Z

So I've looked into switching this to a clang-4.0 snapshot. Currently, gcc-7's c++17 support seems better; current clang 4.0 snapshot compiling under -std=c++1z will neither deduce the NoExceptions parameter inside noexcept(), nor let noexcept methods/functions match the templated versions without a noexcept.

So basically, to make current clang++-4 work, we'd have to provide 7 cpp_function constructors [basically: (const method, non-const method, function) x (noexcept(true), noexcept(false)) + lambda].

But even that wouldn't be enough: current clang with current libstdc++ also hits https://llvm.org/bugs/show_bug.cgi?id=30949 on anything that includes (indirectly) <locale>, which includes both <sstream> and <complex> which pybind includes.

So I think we can revisit it later, but for the moment, g++ seems the saner choice.

I'll investigate the lambda failure to see if there isn't some other less drastic change I can make there to avoid the problem.

Current g++ 7 snapshot fails to compile pybind under -std=c++17 with: ``` $ make [ 3%] Building CXX object tests/CMakeFiles/pybind11_tests.dir/pybind11_tests.cpp.o In file included from /home/jagerman/src/pybind11/tests/pybind11_tests.h:2:0, from /home/jagerman/src/pybind11/tests/pybind11_tests.cpp:10: /home/jagerman/src/pybind11/include/pybind11/pybind11.h: In instantiation of 'pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...)::<lambda(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle)> [with Func = pybind11::cpp_function::cpp_function(Return (Class::*)(Arg ...), const Extra& ...) [with Return = int; Class = ConstructorStats; Arg = {}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]::<lambda(ConstructorStats*)>; Return = int; Args = {ConstructorStats*}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]': /home/jagerman/src/pybind11/include/pybind11/pybind11.h:120:22: required from 'struct pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...) [with Func = pybind11::cpp_function::cpp_function(Return (Class::*)(Arg ...), const Extra& ...) [with Return = int; Class = ConstructorStats; Arg = {}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]::<lambda(ConstructorStats*)>; Return = int; Args = {ConstructorStats*}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]::<lambda(struct pybind11::detail::function_record*, class pybind11::handle, class pybind11::handle, class pybind11::handle)>' /home/jagerman/src/pybind11/include/pybind11/pybind11.h:120:19: required from 'void pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...) [with Func = pybind11::cpp_function::cpp_function(Return (Class::*)(Arg ...), const Extra& ...) [with Return = int; Class = ConstructorStats; Arg = {}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]::<lambda(ConstructorStats*)>; Return = int; Args = {ConstructorStats*}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]' /home/jagerman/src/pybind11/include/pybind11/pybind11.h:62:9: required from 'pybind11::cpp_function::cpp_function(Return (Class::*)(Arg ...), const Extra& ...) [with Return = int; Class = ConstructorStats; Arg = {}; Extra = {pybind11::name, pybind11::is_method, pybind11::sibling}]' /home/jagerman/src/pybind11/include/pybind11/pybind11.h:984:22: required from 'pybind11::class_<type_, options>& pybind11::class_<type_, options>::def(const char*, Func&&, const Extra& ...) [with Func = int (ConstructorStats::*)(); Extra = {}; type_ = ConstructorStats; options = {}]' /home/jagerman/src/pybind11/tests/pybind11_tests.cpp:24:47: required from here /home/jagerman/src/pybind11/include/pybind11/pybind11.h:147:9: sorry, unimplemented: unexpected AST of kind cleanup_stmt }; ^ /home/jagerman/src/pybind11/include/pybind11/pybind11.h:147:9: internal compiler error: in potential_constant_expression_1, at cp/constexpr.c:5593 0x84c52a potential_constant_expression_1 ../../src/gcc/cp/constexpr.c:5593 0x84c3c0 potential_constant_expression_1 ../../src/gcc/cp/constexpr.c:5154 0x645511 finish_function(int) ../../src/gcc/cp/decl.c:15527 0x66e80b instantiate_decl(tree_node*, int, bool) ../../src/gcc/cp/pt.c:22558 0x6b61e2 instantiate_class_template_1 ../../src/gcc/cp/pt.c:10444 0x6b61e2 instantiate_class_template(tree_node*) ../../src/gcc/cp/pt.c:10514 0x75a676 complete_type(tree_node*) ../../src/gcc/cp/typeck.c:133 0x67d5a4 tsubst_copy_and_build(tree_node*, tree_node*, int, tree_node*, bool, bool) ../../src/gcc/cp/pt.c:17516 0x67ca19 tsubst_copy_and_build(tree_node*, tree_node*, int, tree_node*, bool, bool) ../../src/gcc/cp/pt.c:16655 0x672cce tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool) ../../src/gcc/cp/pt.c:16140 0x6713dc tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool) ../../src/gcc/cp/pt.c:15408 0x671915 tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool) ../../src/gcc/cp/pt.c:15394 0x671fc0 tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool) ../../src/gcc/cp/pt.c:15618 0x66e97f tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool) ../../src/gcc/cp/pt.c:15379 0x66e97f instantiate_decl(tree_node*, int, bool) ../../src/gcc/cp/pt.c:22536 0x6ba0cb instantiate_pending_templates(int) ../../src/gcc/cp/pt.c:22653 0x6fd7f8 c_parse_final_cleanups() ../../src/gcc/cp/decl2.c:4512 ``` which looks a lot like https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77545. The error seems to be that it gets confused about the `std::tuple<...> value` in argument_loader: it is apparently not being initialized properly. Adding a default constructor with an explicit default-initialization of `value` works around the problem.

Since the argument loader split off from the tuple converter, it is never called with a `convert` argument set to anything but true. This removes the argument entirely, passing a literal `true` from within `argument_loader` to the individual value casters.

gcc 7 has both std::experimental::optional and std::optional, but this breaks the test compilation as we are trying to use the same `opt_int` type alias for both.

When compiling in C++17 mode the noexcept specifier is part of the function type. This causes a failure in pybind11 because, by omitting a noexcept specifier when deducing function return and argument types, we are implicitly making `noexcept(false)` part of the type. This means that functions with `noexcept` fail to match the function templates in cpp_function (and other places), and we get compilation failure (we end up trying to fit it into the lambda function version, which fails since a function pointer has no `operator()`). We can, however, deduce the true/false `B` in noexcept(B), so we don't need to add a whole other set of overloads, but need to deduce the extra argument when under C++17. That will *not* work under pre-C++17, however. This commit adds two macros to fix the problem: under C++17 (with the appropriate feature macro set) they provide an extra `bool NoExceptions` template argument and provide the `noexcept(NoExceptions)` deduced specifier. Under pre-C++17 they expand to nothing. This is needed to compile pybind11 with gcc7 under -std=c++17.

Add a build using g++-7 snapshot from debian experimental. This build is set to allow failures without triggering an overall build failure (since this is an experimental compiler with experimental support for a future C++ standard).

jagerman · 2016-12-14T06:48:57Z

I found a much smaller workaround to the gcc problem.

wjakob · 2016-12-14T19:25:51Z

I'd be happy to merge this if there is a nicer workaround for the gcc issue -- do you want to push what you found?

jagerman · 2016-12-14T19:34:55Z

I already did (I rebased and replaced the commit in the PR): f8df582

wjakob · 2016-12-14T19:41:09Z

Got it, thanks! That was a much shorter workaround indeed!

noexcept deduction, added in PR pybind#555, doesn't work with clang's -std=c++1z; and while it works with g++, it isn't entirely clear to me that it is required to work in C++17. What should work, however, is that C++17 allows implicit conversion of a `noexcept(true)` function pointer to a `noexcept(false)` (i.e. default, noexcept-not-specified) function pointer. That was breaking in pybind11 because the cpp_function template used for lambdas provided a better match (i.e. without requiring an implicit conversion), but it then failed. This commit takes a different approach of using SFINAE on the lambda function to prevent it from matching a non-lambda object, which then gets implicit conversion from a `noexcept` function pointer to a `noexcept(false)` function pointer. This much nicer solution also gets rid of the C++17 NOEXCEPT macros, and works in both clang and g++.

…ons (#677) noexcept deduction, added in PR #555, doesn't work with clang's -std=c++1z; and while it works with g++, it isn't entirely clear to me that it is required to work in C++17. What should work, however, is that C++17 allows implicit conversion of a `noexcept(true)` function pointer to a `noexcept(false)` (i.e. default, noexcept-not-specified) function pointer. That was breaking in pybind11 because the cpp_function template used for lambdas provided a better match (i.e. without requiring an implicit conversion), but it then failed. This commit takes a different approach of using SFINAE on the lambda function to prevent it from matching a non-lambda object, which then gets implicit conversion from a `noexcept` function pointer to a `noexcept(false)` function pointer. This much nicer solution also gets rid of the C++17 NOEXCEPT macros, and works in both clang and g++.

jagerman mentioned this pull request Dec 14, 2016

Relax and optimize all_of_t/any_of_t #554

Merged

Silence warnings from eigen under g++ 7

02bb4d5

-Wint-in-bool-context triggers many warnings when compiling eigen code, so disable it locally in eigen.h.

jagerman force-pushed the cpp17-fixes branch from 96eaeee to bbcf8d0 Compare December 14, 2016 03:35

jagerman force-pushed the cpp17-fixes branch from bbcf8d0 to 08036b4 Compare December 14, 2016 04:23

jagerman added 6 commits December 14, 2016 01:39

Fix test compilation when both optional's exist

d79a18c

gcc 7 has both std::experimental::optional and std::optional, but this breaks the test compilation as we are trying to use the same `opt_int` type alias for both.

Add gcc-7 build from debian experimental

e81eb7a

Add a build using g++-7 snapshot from debian experimental. This build is set to allow failures without triggering an overall build failure (since this is an experimental compiler with experimental support for a future C++ standard).

Remove duplicate protected:/private:

4bac7b3

jagerman force-pushed the cpp17-fixes branch from 08036b4 to 4bac7b3 Compare December 14, 2016 06:47

wjakob merged commit b11b144 into pybind:master Dec 14, 2016

jagerman mentioned this pull request Feb 16, 2017

Revert noexcept deduction in favour of SFINAE lambda function matching #677

Merged

rwgk mentioned this pull request Feb 9, 2023

FWD pybind11 google/pybind11clif#555

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Various C++17 fixes #555

Various C++17 fixes #555

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

wjakob commented Dec 14, 2016 •

edited

Loading

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016 •

edited

Loading

Uh oh!

jagerman commented Dec 14, 2016 •

edited

Loading

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

Uh oh!

Various C++17 fixes #555

Various C++17 fixes #555

Uh oh!

Conversation

jagerman commented Dec 14, 2016

Uh oh!

wjakob commented Dec 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jagerman commented Dec 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

jagerman commented Dec 14, 2016

Uh oh!

wjakob commented Dec 14, 2016

Uh oh!

Uh oh!

wjakob commented Dec 14, 2016 •

edited

Loading

jagerman commented Dec 14, 2016 •

edited

Loading

jagerman commented Dec 14, 2016 •

edited

Loading