Add lvalue ref-qualified cpp_function constructors #2213
Conversation
Before, the code would have failed to build.
There was a problem hiding this comment.
This looks good to me, but two thoughts:
-
It's a bit unfortunate that highly complex code is copy-pasted, just to insert another
&. But I'm guessing the only way to avoid that is to use a macro, which isn't great either. @wjakob what's your take on this? — If we keep the copy-pasted code, a tiny comment like// Exact copy of above, just with & inserted.may be nice. (It takes a lot of human parsing to figure that out without a comment.) -
What would it take to also exercise the (const, lvalue ref-qualifier) case? — First idea that comes to mind: introduce a tiny dedicated test class (don't add anything to the existing test class), with
mutable int value = 0; // mutable to facilitate testing (const, lvalue ref-qualifier).
|
Hi Ralf, thank you for your reply! I have added the suggested comment and moved my test additions out into their own test class and -case. Agreed that the code duplication is unfortunate, but I do not know of a better way that is shorter and/or more readable. I am open to suggestions though. |
include/pybind11/pybind11.h
Outdated
| @@ -80,6 +80,8 @@ class cpp_function : public function { | |||
| } | |||
|
|
|||
| /// Construct a cpp_function from a class method (non-const, lvalue ref-qualifier) | |||
| /// Is a copy of the overload for non-const functions without explicit ref-qualifier | |||
There was a problem hiding this comment.
Maybe delete "Is" here and below?
// A copy of ...
There was a problem hiding this comment.
... and done 👍
|
I wish there was a way to merge all of these constructors using templating but, alas, there is not AFAIK. |
Hi there,
I tried and failed to bind a method with an explicit lvalue ref-qualifier (see c++ reference). I understand that we would not want to bind rvalue ref-qualified methods, but the lvalue counterparts should be safe..?
This PR adds two
cpp_functionconstructor overloads for lvalue ref-qualified methods: one for const, the other for not.I have also added a simple test method with an explicit lvalue ref-qualifier. The code would fail to compile without the additional constructors, now it compiles and works like it should (like a regular, non ref-qualified method).
Let me know what you think and thank you for all your hard work! pybind11 has been a joy to program against and it is used pretty much everywhere I worked.