Allow python builtins to be used as callbacks #1413

davidhewitt · 2018-05-26T11:25:58Z

At the moment when passing python builtins like sum() as callback functions to c++, the std::function caster will fail with RuntimeError: Unable to extract capsule contents. This is because of the machinery to extract stateless c++ function pointers to avoid the c++ -> python -> c++ overhead.

I've added a simple check that the definition being extracted is indeed a capsule and an accompanying test that I can now use sum() as a callback. Hope you find this useful.

Suggested changelog entry:

Allow Python builtins to be used as callbacks in CPython

jagerman · 2018-06-21T14:36:06Z

The change looks good to me. There's one minor change needed in the added python test to fix the flake warning (two blank lines needed before the added test).

davidhewitt · 2018-06-21T21:22:06Z

Great, I'll find a moment to tidy that up at the weekend!

davidhewitt · 2018-06-24T06:49:58Z

@jagerman should be all good now. Thanks

davidhewitt · 2018-08-03T00:48:33Z

@jagerman just wanted to make sure this wasn't forgotten - I still run into this every so often.

TobiasBruell · 2018-08-13T13:01:40Z

I just ran into the same problem. Many thanks!

lycantropos · 2020-07-09T16:46:37Z

what about progress on this one?

YannickJadoul · 2020-07-09T20:00:18Z

what about progress on this one?

We're in the middle of cleaning up issues and PRs. Give us a little bit of time, please.

If you want to help this PR progress, fixing the merge conflicts would help us greatly! :-)

rwgk

Looks great! Is the original author still around? If someone rebases on current master and the CI comes back green I think we should merge this straightaway.

…tin_callbacks

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

Skylion007 · 2021-07-21T18:17:30Z

@rwgk @henryiii I added an xfail for PyPy, but I would appreciate if you either of you could see if there is an easy fix / workaround we are missing. Like maybe wrapping it in a lambda or something?

Skylion007 · 2021-07-22T16:18:59Z

include/pybind11/functional.h

+#if            defined(PYPY_VERSION)
+               // PyPy will segfault otherwise when passing in raw builtin functions.
+               return false;
+               //pybind11_fail("Passing raw builtin functions not supported with PyPy. Wrap in function.");


Do a fail her also caused a segfault. Weird.

Skylion007 · 2021-07-22T17:43:19Z

Now it segfaults on every PyPy EXCEPT Ubuntu. So weird...

Skylion007 · 2021-07-22T18:07:46Z

include/pybind11/functional.h

+#if            defined(PYPY_VERSION)
+               // PyPy will segfault otherwise when passing in raw builtin functions.
+               // This will lead to a TypeError instead.
+               return false;


So it looks like this test case ALWAYS segfaulted on non-ubuntu PyPy then which means that that the code is still better than before. I am really stumped on how to avoid this segfault though.

I just counted, we have 19 xfail for PyPy already.

PyPy has bugs. If we aren't adding a new one or making it worse (which it appears we are not), then I'm happy with it if it passes Google's testing.

include/pybind11/functional.h

davidhewitt · 2021-07-22T20:26:12Z

👋 original author still here! Sorry it's taken me a couple of days to reply. It looks like you've got this one under control without me, so I won't push anything unless you ask me to.

These days I don't use C++ much any more, I tend to use Rust instead. I'm still very much working in the Python + native domain though, I am a maintainer of PyO3.

I still watch progress here closely; I like to think that both projects can benefit from sharing ideas and pushing the ecosystem forward. Sadly we're still some way behind in PyO3 so I don't think there's much learning from PyO3 that would interest you yet.

Do feel free to ping me any time (on this issue or any other) if you want my input as a "neighbour" 😄

rwgk · 2021-07-23T04:32:37Z

I imported this PR into the Google environment and ran ASAN & MSAN. Both are happy. I'm now submitting this PR to our global testing system. I'll report on the results tomorrow.

rwgk · 2021-07-23T14:36:31Z

EDIT: SEE NEXT COMMENT

Unfortunately there is a problem, triggered by code that's not open source, so I cannot easily point or share. It's a compilation failure (using some very modern clang version). I'll share the full traceback non-publicly. Here is the result of grep functional in the traceback:

pybind11/functional.h(90,74): error: use of undeclared identifier 'func_handle'
pybind11/functional.h(99,17): error: no matching conversion for functional-style cast from 'func_handle' to 'func_wrapper'
pybind11/functional.h(88,16): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'func_handle' to 'const func_wrapper' for 1st argument
pybind11/functional.h(88,16): note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'func_handle' to 'func_wrapper' for 1st argument
pybind11/functional.h(88,16): note: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 1 was provided
pybind11/functional.h(99,17): error: no matching conversion for functional-style cast from 'func_handle' to 'func_wrapper'
pybind11/functional.h(88,16): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'func_handle' to 'const func_wrapper' for 1st argument
pybind11/functional.h(88,16): note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'func_handle' to 'func_wrapper' for 1st argument
pybind11/functional.h(88,16): note: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 1 was provided

rwgk · 2021-07-23T15:04:39Z

WAIT: after reporting the above I tried to reproduce the errors interactively, but it works!
I'm confused...

rwgk · 2021-07-23T15:12:12Z

It turns out to be some kind of Windows build.
I never had such failures before and don't know how to reproduce interactively.

rwgk · 2021-07-23T16:10:45Z

In the meantime the author of #1883 gave me some pointers, including how to reproduce the error interactively. Theory: we could be up against a bug in the Windows clang compiler. I'll try to get the attention of the team maintaining that compiler.

…tin_callbacks

Skylion007 · 2021-07-27T17:48:10Z

I'll split off the bugprone checks into their own PR.

* 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>

davidhewitt force-pushed the builtin_callbacks branch from 5ef30fd to 4197abb Compare May 26, 2018 11:38

davidhewitt force-pushed the builtin_callbacks branch from 4197abb to 1c4261b Compare June 24, 2018 06:47

Allow python builtins to be used as callbacks

a9ccb02

davidhewitt force-pushed the builtin_callbacks branch from 1c4261b to a9ccb02 Compare June 24, 2018 07:17

wjakob force-pushed the master branch from c4321d3 to cf36e3d Compare June 11, 2019 21:29

Skylion007 requested a review from rwgk July 17, 2021 14:43

rwgk approved these changes Jul 20, 2021

View reviewed changes

Skylion007 and others added 6 commits July 21, 2021 12:29

Merge branch 'master' of https://github.com/pybind/pybind11 into buil…

22f1b88

…tin_callbacks

Try to fix pypy segfault

764c88e

Add expected fail for PyPy

4dc2c35

Fix typo

1392c05

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

0435d5c

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

Add more info to xfail

2a8e139

Skylion007 added 2 commits July 21, 2021 14:20

Add env

c712bea

Try returning false

f863c57

Skylion007 reviewed Jul 22, 2021

View reviewed changes

Try removing the move for pypy

5d9f53d

davidhewitt requested a review from henryiii as a code owner July 22, 2021 16:48

Skylion007 added 4 commits July 22, 2021 12:58

Fix bugs

996d214

Try removing move

7ce0e28

Just keep ignoring for PyPy

0bd0854

Add back xfail

f3d9dbf

Fix ctors

fcd37eb

Skylion007 reviewed Jul 22, 2021

View reviewed changes

include/pybind11/functional.h Outdated Show resolved Hide resolved

Skylion007 added 4 commits July 27, 2021 13:09

Revert change of std::move

62cbb30

Merge branch 'master' of https://github.com/pybind/pybind11 into buil…

11f14f7

…tin_callbacks

Change to skip

fb6b064

Fix bug and edit comments

fa42611

Remove clang-tidy bugprone fix skip bug

efe32cf

Skylion007 merged commit a0b9759 into pybind:master Jul 27, 2021

github-actions bot added the needs changelog Possibly needs a changelog entry label Jul 27, 2021

Skylion007 added this to the v2.8 milestone Jul 27, 2021

Skylion007 mentioned this pull request Sep 10, 2021

[BUG] cannot pass random.random() as std::function<double()> argument #2853

Open

henryiii removed the needs changelog Possibly needs a changelog entry label Sep 16, 2021

zwimer mentioned this pull request Feb 25, 2022

Update to use pybind11 2.9.1 (Feb 2, 2022) RosettaCommons/binder#173

Open

rwgk mentioned this pull request Feb 9, 2023

FWD pybind11 google/pybind11clif#1413

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow python builtins to be used as callbacks #1413

Allow python builtins to be used as callbacks #1413

davidhewitt commented May 26, 2018 •

edited by henryiii

Loading

jagerman commented Jun 21, 2018

davidhewitt commented Jun 21, 2018

davidhewitt commented Jun 24, 2018

davidhewitt commented Aug 3, 2018

TobiasBruell commented Aug 13, 2018

lycantropos commented Jul 9, 2020

YannickJadoul commented Jul 9, 2020

rwgk left a comment

Skylion007 commented Jul 21, 2021

Skylion007 Jul 22, 2021

Skylion007 commented Jul 22, 2021

Skylion007 Jul 22, 2021

rwgk Jul 23, 2021

henryiii Jul 23, 2021

davidhewitt commented Jul 22, 2021

rwgk commented Jul 23, 2021

rwgk commented Jul 23, 2021 •

edited

Loading

rwgk commented Jul 23, 2021

rwgk commented Jul 23, 2021

rwgk commented Jul 23, 2021

Skylion007 commented Jul 27, 2021

Allow python builtins to be used as callbacks #1413

Allow python builtins to be used as callbacks #1413

Conversation

davidhewitt commented May 26, 2018 • edited by henryiii Loading

Suggested changelog entry:

jagerman commented Jun 21, 2018

davidhewitt commented Jun 21, 2018

davidhewitt commented Jun 24, 2018

davidhewitt commented Aug 3, 2018

TobiasBruell commented Aug 13, 2018

lycantropos commented Jul 9, 2020

YannickJadoul commented Jul 9, 2020

rwgk left a comment

Choose a reason for hiding this comment

Skylion007 commented Jul 21, 2021

Skylion007 Jul 22, 2021

Choose a reason for hiding this comment

Skylion007 commented Jul 22, 2021

Skylion007 Jul 22, 2021

Choose a reason for hiding this comment

rwgk Jul 23, 2021

Choose a reason for hiding this comment

henryiii Jul 23, 2021

Choose a reason for hiding this comment

davidhewitt commented Jul 22, 2021

rwgk commented Jul 23, 2021

rwgk commented Jul 23, 2021 • edited Loading

rwgk commented Jul 23, 2021

rwgk commented Jul 23, 2021

rwgk commented Jul 23, 2021

Skylion007 commented Jul 27, 2021

davidhewitt commented May 26, 2018 •

edited by henryiii

Loading

rwgk commented Jul 23, 2021 •

edited

Loading