Skip to content

Commit

Permalink
Clarify rvalue reference to unique_ptr parameter in initialize_generic
Browse files Browse the repository at this point in the history
  • Loading branch information
YannickJadoul committed Jan 13, 2021
1 parent a5729a3 commit 9942614
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ class cpp_function : public function {
/// Register a function call with Python (generic non-templated code goes here)
void initialize_generic(unique_function_record &&unique_rec, const char *text,
const std::type_info *const *types, size_t args) {
// Do NOT receive `unique_rec` by value. If this function fails to steal the unique_ptr then

This comment has been minimized.

Copy link
@rwgk

rwgk Jan 13, 2021

Collaborator

Nit: maybe use "move" here instead of "steal"? To use proper std terms.

This comment has been minimized.

Copy link
@YannickJadoul

YannickJadoul Jan 13, 2021

Author Collaborator

Can do. I used "steal" since the unique_rec was already std::moved (even though that doesn't move), and steal seems more active. But sure.

This comment has been minimized.

Copy link
@bstaletic

bstaletic Jan 13, 2021

Collaborator

Bikeshedding perhaps, but "move" usually means from T to another T. This is talking about transfering ownership from T to U.

This comment has been minimized.

Copy link
@YannickJadoul

YannickJadoul Jan 13, 2021

Author Collaborator

I'll use "move out", then? That demonstrates the "stealing and leaving the original empty"?

This comment has been minimized.

Copy link
@bstaletic

bstaletic Jan 13, 2021

Collaborator

Sure, why not.

// we do not want this to destuct the pointer. `initialize` (the caller) still relies on the
// pointee being alive after this call. Only steal if a `capsule` is going to keep it alive.
auto rec = unique_rec.get();

// Keep track of strdup'ed strings, and clean them up as long as the function's capsule
Expand Down

0 comments on commit 9942614

Please sign in to comment.