Add more return value policy support

[lczech]

This PR adds more support for return value policies (RVP):

• Add RVP defaults for free functions. They were mixed in with static member functions before.
• Add RVP default for assignment operators. See below for details.
• Add RVP customization per function. That's the main addition I was after, see Custom return value policy per function #319
• Add test case and documentation for all of the above.

The RVPs for assignment operators (operator=, but also all the operator+= etc, see here) are an interesting case: When they are defaulted (implemented as = default;), they implicitly end with return *this;, returning the called instance itself. Thus, the pybind RVP automatic is not the best choice for them, as it uses copy for lvalue references. Instead, reference_internal is a better fit. Hence this addition, to separate those special functions from others.

However, to not break backwards compatibility with previous Binder versions, I have implemented this such that default_member_assignment_operator_return_value_policy is empty by default and not used unless explicitly set in the Binder config by the user. This is the least disruptive for existing bindings - although I'd argue that introducing a breaking change here and defaulting this to reference_internal might be a good choice in terms of "least surprise" for users. It would probably not break any bindings code to go from copy to reference_internal for assignment operators, as likely no one in Python will use the return value of an assignment anyway... But still it felt like the right thing to do. @lyskov, let me know what you think.

While at it, I also refined some other parts:

• Add some new headings and refinements to config doc, just to clear it up a bit.
• Add checks for correct +/- in config token parsing - those were missing for many tokens.

I hope that was okay to mix this in here - @lyskov, let me know if you prefer that in a different PR instead.

Lastly, while playing around with this, I noticed that there are defaults for the return value policies of rvalue references, but Binder never actually emits bindings for such functions... I kept everything as it was (and even added rvalue references for free functions for completeness), but I was wondering if it's worth having those around. Or am I missing something and functions returning rvalue references can indeed be bound by Binder? If so, how?

Cheers and looking forward to hearing your thoughts
Lucas

[lyskov]

Thank you for PR @lczech ! Would it be possible to submit 1b21bd3 as separate PR so it is easy to review and reason about the changes? And if so, would you mind doin this?
Thanks,

[lczech]

Sure, done in #330. I cannot take this commit out of here though. If you want that, I'd have to start a new PR altogether. Do you want me to do that? In that case I think the new PR will have to be merged first though, as I think otherwise it might result in merge conflicts if I tried to apply the changes here to the main branch without the formatting first.

[lczech]

Hey Sergey,

just added another bit of quality-of-life here: RV policy specification for a whole class, as a convenient middle ground between whole-project defaults and having to specify policies for each function. Hope you like it :-)

Cheers
Lucas

[lyskov]

I have given this a bit more thought and here is a few idea/thoughts:

• I agree that adding a way to further customize return policy's will be useful feature to add.
• My priority on adding this will be to make sure [a] backward compatibility and [b] that defaults should be safe to use even if it cost some performance.

Add RVP defaults for free functions. They were mixed in with static member functions before.

-- could you please elaborate on this? Ie in my view static function is effectively a free function that live in different namespace. Right now my vote will be to not add this one.

-- re choosing reference_internal default_member_assignment_operator_return_value_policy: I am not sure about this, automatic seems to be a safe hence will be what I want to choose.

[lyskov]

re default_member_assignment_operator_return_value_policy: the more I think about it then more it looks like this is highly depended on case-by-case basis. Could you please provide a justification why this category should have a separate config option?

EDIT: and why just heaving " RVP customization per function" is not enough?

[lczech]

My priority on adding this will be to make sure [a] backward compatibility and [b] that defaults should be safe to use even if it cost some performance.

Almost. Except for the static member functions, none of this should break existing Binder configs. Only if any of the new tokens are specified will they be used.

could you please elaborate on this? Ie in my view static function is effectively a free function that live in different namespace. Right now my vote will be to not add this one.

Well, they are not the same as far as C++ is concerned. Separating them gives a bit more flexibility, and is clearer. It was rather confusing to me to figure out that the RV policy setting for free functions is the "static" one... However, as mentioned, if I recall correctly, it might however not be backwards compatible, so that might need some tweaking if you want to fully maintain backwards compatible...

re choosing reference_internal default_member_assignment_operator_return_value_policy: I am not sure about this, automatic seems to be a safe hence will be what I want to choose.

re default_member_assignment_operator_return_value_policy: the more I think about it then more it looks like this is highly depended on case-by-case basis. Could you please provide a justification why this category should have a separate config option?

Assignment operators return an lvalue reference to *this, which the automatic policy will turn into a copy, which is wrong from a semantic point of view. I am not sure how needed this is, as likely no one will use it that way in Python (e.g., (a += 1) += 2 or the like), but it seems like the right thing to do in terms of code semantics. I can see though that this might be confusing.

EDIT: and why just heaving " RVP customization per function" is not enough?

That feature allows to do this as well of course. But assignment operators are so common that I thought it might be worth defaulting all of them at once to a reasonable RV policy.

[lyskov]

Want to revisit this, re:

Assignment operators return an lvalue reference to *this, which the automatic policy will turn into a copy, which is wrong from a semantic point of view.

-- well, what else can Python do besides copy at this point? I mean this seems to be the only safe choice for default, would you agree?