Fixed compilation error when binding function accepting some forms of std::function (#689)

* Fixed compilation error when defining function accepting some forms of std::function.

The compilation error happens only when the functional.h header is
present, and the build is done in debug mode, with NDEBUG being
undefined.  In addition, the std::function must accept an abstract
base class by reference.

The compilation error occurred in cast.h, when trying to construct a
std::tuple<AbstractBase>, rather than a std::tuple<AbstractBase&>.
This was caused by functional.h using std::move rather than
std::forward, changing the signature of the function being used.

This commit contains the fix, along with a test that exhibits the
issue when compiled in debug mode without the fix applied.

* Moved new std::function tests into test_callbacks, added callback_with_movable test.

include/pybind11/functional.h

@@ -52,7 +52,7 @@ public:
         auto src = reinterpret_borrow<object>(src_);
         value = [src](Args... args) -> Return {
             gil_scoped_acquire acq;
-            object retval(src(std::move(args)...));
+            object retval(src(std::forward<Args>(args)...));
             /* Visual studio 2015 parser issue: need parentheses around this expression */
             return (retval.template cast<Return>());
         };

tests/test_callbacks.cpp

@@ -74,6 +74,27 @@ struct Payload {
 /// Something to trigger a conversion error
 struct Unregistered {};
 
+class AbstractBase {
+public:
+  virtual unsigned int func() = 0;
+};
+
+void func_accepting_func_accepting_base(std::function<double(AbstractBase&)>) { }
+
+struct MovableObject {
+  bool valid = true;
+
+  MovableObject() = default;
+  MovableObject(const MovableObject &) = default;
+  MovableObject &operator=(const MovableObject &) = default;
+  MovableObject(MovableObject &&o) : valid(o.valid) { o.valid = false; }
+  MovableObject &operator=(MovableObject &&o) {
+    valid = o.valid;
+    o.valid = false;
+    return *this;
+  }
+};
+
 test_initializer callbacks([](py::module &m) {
     m.def("test_callback1", &test_callback1);
     m.def("test_callback2", &test_callback2);
@@ -146,4 +167,15 @@ test_initializer callbacks([](py::module &m) {
     m.def("test_dummy_function", &test_dummy_function);
     // Export the payload constructor statistics for testing purposes:
     m.def("payload_cstats", &ConstructorStats::get<Payload>);
+
+    m.def("func_accepting_func_accepting_base",
+          func_accepting_func_accepting_base);
+
+    py::class_<MovableObject>(m, "MovableObject");
+
+    m.def("callback_with_movable", [](std::function<void(MovableObject &)> f) {
+        auto x = MovableObject();
+        f(x); // lvalue reference shouldn't move out object
+        return x.valid; // must still return `true`
+      });
 });

tests/test_callbacks.py

@@ -96,3 +96,9 @@ def test_function_signatures(doc):
 
     assert doc(test_callback3) == "test_callback3(arg0: Callable[[int], int]) -> str"
     assert doc(test_callback4) == "test_callback4() -> Callable[[int], int]"
+
+
+def test_movable_object():
+    from pybind11_tests import callback_with_movable
+
+    assert callback_with_movable(lambda _: None) is True