Allow python builtins to be used as callbacks (#1413)

* Allow python builtins to be used as callbacks

* Try to fix pypy segfault

* Add expected fail for PyPy

* Fix typo

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add more info to xfail

* Add env

* Try returning false

* Try removing the move for pypy

* Fix bugs

* Try removing move

* Just keep ignoring for PyPy

* Add back xfail

* Fix ctors

* Revert change of std::move

* Change to skip

* Fix bug and edit comments

* Remove clang-tidy bugprone fix skip bug

Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

include/pybind11/functional.h

diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h
index cc7099c..b9bf389 100644
--- a/include/pybind11/functional.h
+++ b/include/pybind11/functional.h
@@ -43,27 +43,33 @@
            captured variables), in which case the roundtrip can be avoided.
          */
         if (auto cfunc = func.cpp_function()) {
-            auto c = reinterpret_borrow<capsule>(PyCFunction_GET_SELF(cfunc.ptr()));
-            auto rec = (function_record *) c;
+            auto cfunc_self = PyCFunction_GET_SELF(cfunc.ptr());
+            if (isinstance<capsule>(cfunc_self)) {
+                auto c = reinterpret_borrow<capsule>(cfunc_self);
+                auto rec = (function_record *) c;
 
-            while (rec != nullptr) {
-                if (rec->is_stateless
-                    && same_type(typeid(function_type),
-                                 *reinterpret_cast<const std::type_info *>(rec->data[1]))) {
-                    struct capture {
-                        function_type f;
-                    };
-                    value = ((capture *) &rec->data)->f;
-                    return true;
+                while (rec != nullptr) {
+                    if (rec->is_stateless
+                        && same_type(typeid(function_type),
+                                     *reinterpret_cast<const std::type_info *>(rec->data[1]))) {
+                        struct capture {
+                            function_type f;
+                        };
+                        value = ((capture *) &rec->data)->f;
+                        return true;
+                    }
+                    rec = rec->next;
                 }
-                rec = rec->next;
             }
+            // PYPY segfaults here when passing builtin function like sum.
+            // Raising an fail exception here works to prevent the segfault, but only on gcc.
+            // See PR #1413 for full details
         }
 
         // ensure GIL is held during functor destruction
         struct func_handle {
             function f;
-            func_handle(function&& f_) : f(std::move(f_)) {}
+            func_handle(function &&f_) noexcept : f(std::move(f_)) {}
             func_handle(const func_handle& f_) {
                 gil_scoped_acquire acq;
                 f = f_.f;
@@ -77,7 +83,7 @@
         // to emulate 'move initialization capture' in C++11
         struct func_wrapper {
             func_handle hfunc;
-            func_wrapper(func_handle&& hf): hfunc(std::move(hf)) {}
+            func_wrapper(func_handle &&hf) noexcept : hfunc(std::move(hf)) {}
             Return operator()(Args... args) const {
                 gil_scoped_acquire acq;
                 object retval(hfunc.f(std::forward<Args>(args)...));

tests/test_callbacks.cpp

diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp
index a208b44..a507710 100644
--- a/tests/test_callbacks.cpp
+++ b/tests/test_callbacks.cpp
@@ -156,6 +156,12 @@
         .def(py::init<>())
         .def("triple", [](CppBoundMethodTest &, int val) { return 3 * val; });
 
+    // This checks that builtin functions can be passed as callbacks
+    // rather than throwing RuntimeError due to trying to extract as capsule
+    m.def("test_sum_builtin", [](const std::function<double(py::iterable)> &sum_builtin, const py::iterable &i) {
+      return sum_builtin(i);
+    });
+
     // test async Python callbacks
     using callback_f = std::function<void(int)>;
     m.def("test_async_callback", [](const callback_f &f, const py::list &work) {

tests/test_callbacks.py

diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py
index 397bf63..5bc4d17 100644
--- a/tests/test_callbacks.py
+++ b/tests/test_callbacks.py
@@ -3,6 +3,7 @@
 from pybind11_tests import callbacks as m
 from threading import Thread
 import time
+import env  # NOQA: F401
 
 
 def test_callbacks():
@@ -124,6 +125,16 @@
     assert m.callback_with_movable(lambda _: None) is True
 
 
+@pytest.mark.skipif(
+    "env.PYPY",
+    reason="PyPy segfaults on here. See discussion on #1413.",
+)
+def test_python_builtins():
+    """Test if python builtins like sum() can be used as callbacks"""
+    assert m.test_sum_builtin(sum, [1, 2, 3]) == 6
+    assert m.test_sum_builtin(sum, []) == 0
+
+
 def test_async_callbacks():
     # serves as state for async callback
     class Item: