Skip to content
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

[smart_holder] Make smart holder type caster of unique_ptr accept automatic_reference #4775

Merged

Conversation

wangxf123456
Copy link
Contributor

@wangxf123456 wangxf123456 commented Aug 3, 2023

Description

When calling a Python callback function that consumes std::unique_ptr in C++, we see RuntimeError: Invalid return_value_policy for unique_ptr. This is because when calling Python functions manually from C++ , we use return_value_policy::automatic_reference for function arguments, and the type caster of std::unique_ptr rejects this return value policy.

Pybind11 always override return_value_policy::automatic and return_value_policy::automatic_reference with return_value_policy::move for std::unique_ptr:

if (policy == return_value_policy::automatic) {
, so return_value_policy::automatic and return_value_policy::automatic_reference does not make any difference for std::unique_ptr. We should make the type caster accept both.

@wangxf123456 wangxf123456 changed the title [smart_holder] Make smart holder type caster of std::unique_ptr accept return_value_policy::automatic_reference [smart_holder] Make smart holder type caster of unique_ptr accept automatic_reference Aug 3, 2023
@rwgk
Copy link
Collaborator

rwgk commented Aug 4, 2023

Ignoring a test_iostream flake and Clang Dev failures fixed by PR #4767 (currently only on master).

@rwgk rwgk marked this pull request as ready for review August 4, 2023 01:33
@rwgk rwgk merged commit f1e2e55 into pybind:smart_holder Aug 4, 2023
147 of 150 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Aug 4, 2023
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Aug 4, 2023
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

Successfully merging this pull request may close these issues.

2 participants