pybind / pybind/pybind11

[BUG] `PYBIND11_OVERRIDE` macros do not allow changing the default `return_value_property`

Open
#2,516 2 comments 0 reactions 1 assignee View on GitHub

@YannickJadoul is already working on this.

Since Sep 20, 2020.

Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

Issue description

The PYBIND11_OVERRIDE macros don't allow for the return_value_policy to be specified. It is always object_api<T>::operator ()'s default, automatic_reference.
This results in problems when trying to nót copy a C++ object that doesn't have a Python object associated yet.

@davidelbaze bumped into this issue on Gitter, and I debugged and tracked the solution down to this issue.

Reproducible example code

@davidelbaze's original code:

#include <pybind11/functional.h>
#include <pybind11/iostream.h>
#include <pybind11/pybind11.h>
#include <pybind11/stl_bind.h>

namespace py = pybind11;

PYBIND11_MAKE_OPAQUE(std::vector<uint8_t>);

PYBIND11_MODULE(example, m) {
  py::bind_vector<std::vector<uint8_t>>(m, "Buffer", py::buffer_protocol()).def("ptr", [](std::vector<uint8_t>&v) { return (size_t) &v; });

using T = std::vector<uint8_t> &;
    class Muter {
     public:
      virtual void Mutate(T s) {
        s[0] = s[0] * 2;
      }
      size_t ptr() { return (size_t) this; }
      virtual ~Muter() = default;
    };

    class PyMuter : public Muter {
     public:
      using Muter::Muter;
      void Mutate(T s) override {
        // auto tmp = py::cast(s, py::return_value_policy::reference);  // This line/hack makes things work
         PYBIND11_OVERLOAD(void, /* Return type */
                          Muter,   /* Parent class */
                          Mutate,  /* Name of function in C++ (must match Python name) */
                          s        /* Argument(s) */
        );
      }
    };

    py::class_<Muter, PyMuter>(m, "Muter").def(py::init<>()).def("Mutate", &Muter::Mutate).def("ptr", &Muter::ptr);
    m.def(
        "test",
        [](Muter &ref) {
          std::vector<uint8_t> v{1,2,3,4,5};
          ref.Mutate(v);
          return v;
        });
}
import example

class CustomMuter(example.Muter):
    def Mutate(self, v):
        print(type(v))
        v[0]*=20

b=CustomMuter()
example.test(b)

So, it seems like we 1. need to add a warning to the docs about the default return_value_policy, but it would also be nice to avoid a way to specify the return_value_policy. Better ideas than adding 4 more PYBIND11_OVERRIDE macro variants are welcome. I can create the PR, obviously, but I'd first like to discuss how to best solve this.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.