[smart_holder] Auto select return value policy for clif_automatic #4381
Conversation
|
|
||
| struct nocopy { | ||
| std::string mtxt; | ||
| nocopy(const std::string &mtxt_) : mtxt(mtxt_) {} |
There was a problem hiding this comment.
Don't you still need to delete the copy ctor?
There was a problem hiding this comment.
Thanks for pointing this out. I forgot this. I deleted the copy ctor.
| @@ -51,9 +53,13 @@ const obj &return_const_reference() { | |||
| return value; | |||
| } | |||
|
|
|||
| std::shared_ptr<obj> return_shared_pointer() { return std::make_shared<obj>("shared_pointer"); } | |||
| std::shared_ptr<obj> return_shared_pointer() { | |||
| return std::shared_ptr<obj>(new obj("shared_pointer")); | |||
There was a problem hiding this comment.
Should be std::make_shared, right?
There was a problem hiding this comment.
Initially I used std::make_shared and std::make_unqiue, but then I see errors note: ‘std::make_unique’ is only available from C++14 onwards, so I modified both of them. I should be able to use std::make_shared with C++11. I will update the PR.
| #include "pybind11_tests.h" | ||
|
|
||
| namespace test_return_value_policy_override { | ||
|
|
||
| struct some_type {}; | ||
|
|
||
| struct obj { |
There was a problem hiding this comment.
obj is usually used for instances, it's surprising as a type name.
The combinations of copyable/movable keep coming up, it would be good have some systematic naming that we could use throughout. What do you think about
struct type_cp1_mv1
struct type_cp1_mv0
struct type_cp0_mv1
struct type_cp0_mv0
?
With a comment like:
// cp = copyable, mv = movable, 1 = yes, 0 = no
Question that's maybe beyond this PR: do we have (or would it be safest) to add tests for the other two combinations?
IIUC here you have only type_cp1_mv1 and type_cp0_mv1, is that correct?
There was a problem hiding this comment.
Thanks for your suggestions. I updated the PR with more tests.
wangxf123456
changed the title
…atic (pybind#4381)" This reverts commit c1f14f0. Conflicts resolved in: include/pybind11/detail/smart_holder_type_casters.h tests/test_return_value_policy_override.py
Description
PyCLIF automatically chooses lifetime management of return values based on the properties of them: https://github.com/google/clif/tree/main/clif/python#pointers-references-and-object-ownership. This PR should make the return value policy selection consistent with PyCLIF.
Note that for const raw pointers, PyCLIF is also making a copy: https://github.com/google/clif/blob/main/clif/python/clif_types.py#L124. This is not mentioned in the link above.