-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix: add missing std::forward calls #3443
Conversation
Two of the four cpp_function overloads are missing std::forward calls, which seems like a simple oversight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Boris-Rasin, thanks for fixing the oversight, looks good to me. |
Hi @rwgk, ref qualified methods with rvalue reference parameters were broken: struct RValueParam {
void func1(std::string&&) {}
void func2(std::string&&) const {}
void func3(std::string&&) & {}
void func4(std::string&&) const & {}
};
void init(pybind11::module& m)
{
pybind11::class_<RValueParam>(m, "RValueParam")
.def("func1", &RValueParam::func1) //ok
.def("func2", &RValueParam::func2) //ok
.def("func3", &RValueParam::func3) //error
.def("func4", &RValueParam::func4) //error
;
} // error message: |
Thanks! It'd be great if you could add that to this PR, similar to what you see here (really straightforward): |
I triggered the CI, although I'm not sure if you're still working on tests/test_methods_and_attributes.py, to prove that the methods are actually runnable. Anything trivial is good enough for this PR, e.g (untested).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for adding the tests!
Holding off merging only to give @Skylion007 a chance to look at the tests, too.
Two of the four cpp_function overloads are missing std::forward calls, which seems like a simple oversight.
Description
Suggested changelog entry:
add missing std::forward calls to some ``cpp_function`` overloads