Skip to content

boost::optional example doesn't work on older Boost #847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bmerry opened this issue May 12, 2017 · 4 comments
Closed

boost::optional example doesn't work on older Boost #847

bmerry opened this issue May 12, 2017 · 4 comments
Assignees

Comments

@bmerry
Copy link
Contributor

bmerry commented May 12, 2017

Issue description

The documentation describes how to wrap boost::python. It fails to compile on Boost 1.58 due to the line value = {}; which was only fixed to work in Boost 1.63.

Reproducible example code

C++:

#include <iostream>
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/numpy.h>

namespace py = pybind11;

#define USE_BOOST
#ifdef USE_BOOST
#include <boost/optional.hpp>
using boost::optional;

namespace pybind11 { namespace detail {
    template <typename T>
    struct type_caster<boost::optional<T>> : optional_caster<boost::optional<T>> {};
}}
#else
#include <experimental/optional>
using std::experimental::optional;
#endif

static void func(optional<py::array_t<double>> x)
{
    if (x)
        std::cout << "Called with array of size " << x->size() << '\n';
    else
        std::cout << "Called with None\n";
}

PYBIND11_PLUGIN(use_opt)
{
    py::module m("use_opt", "Test wrapping of optional");
    m.def("func", &func);
    return m.ptr();
}

Python:

#!/usr/bin/env python
import numpy as np
import use_opt

use_opt.func(np.array([1.0, 2.0]))
use_opt.func(None)

There has been some discussion in gitter about approaches to fix this. I'm going to assign to myself to create a PR.

@bmerry
Copy link
Contributor Author

bmerry commented May 12, 2017

Looks like I can't actually assign to myself, presumably because I'm not a committer. I'll work on a PR next week though.

bmerry added a commit to ska-sa/katsdppipelines that referenced this issue May 12, 2017
Replaced with std::optional, std::experimental::option or
boost::optional, depending on compiler support.

There is still more boilerplate than necessary due to
pybind/pybind11#847, which can hopefully go away once that gets fixed.
@jagerman
Copy link
Member

Some potentials from the gitter discussion:

  • a type_traits-type class to yield the std::nullopt-equivalent class
  • a SFINAE reset() method with = {}; fallback (or perhaps vice versa)
  • Adding a typename NullT template parameter (e.g. set to = std::nullopt_t for std::optional). This would break backwards compatibility, however, if someone out there has already used optional_caster with Boost 1.63+.

@bmerry
Copy link
Contributor Author

bmerry commented May 14, 2017

I've added example code to reproduce the issue. It seems that Boost 1.58 sometimes works with value = {}, depending on the wrapped class.

bmerry added a commit to bmerry/pybind11 that referenced this issue May 14, 2017
With older versions of Boost, `value = {}` can fail to compile because
the interpretation of `{}` is ambiguous (could construct `boost::none`
or the value type). It can also fail with `std::experimental::optional`,
at least on GCC 5.4, because it would construct an instance of the
optional type and then move-assign it, and if the value type isn't move
assignable this would fail.

This is replaced by preferring `.reset` if the type supports it, and
also providing a traits class that can be extended to override the
behaviour where necessary (which is done for
std::experimental::optional).

Additionally, the assignment of the value from the inner caster was by
copy rather than move, which prevented use with uncopyable types.

Closes pybind#847.
@dean0x7d
Copy link
Member

Fixed via #874.

bmerry added a commit to ska-sa/katsdpimager that referenced this issue Sep 12, 2019
Replaced with std::optional, std::experimental::option or
boost::optional, depending on compiler support.

There is still more boilerplate than necessary due to
pybind/pybind11#847, which can hopefully go away once that gets fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants