[C++ Frontend] Make call operator on module holder call forward #15831
Conversation
There was a problem hiding this comment.
@smessmer would you mind reviewing these templates?
There was a problem hiding this comment.
has_forward has a quite complex implementation that maybe can be simplified with void_t (it's a c++17 feature but we have c10::guts::void_t for C++11), but that's actually not in this diff but already landed. The templates added here look good.
goldsborough
force-pushed
the
cpp-forward-call-operator
branch
from
e9cc93b to
50bf528
Compare
There was a problem hiding this comment.
has_forward has a quite complex implementation that maybe can be simplified with void_t (it's a c++17 feature but we have c10::guts::void_t for C++11), but that's actually not in this diff but already landed. The templates added here look good.
There was a problem hiding this comment.
not sure if this handles functions with non-const lvalue references correctly since with declval, you always get rvalue references (you can convert rvalue references to const lvalue references, but not to non-const ones).
A better approach might be to use std::result_of, there I'd have a higher confidence that they implemented it correctly.
There was a problem hiding this comment.
Also, please add some static_assert test cases for this functionality. And make sure you test different kinds of references.
There was a problem hiding this comment.
std::result_of seems not recommended according to https://stackoverflow.com/questions/2689709/difference-between-stdresult-of-and-decltype
goldsborough
force-pushed
the
cpp-forward-call-operator
branch
from
50bf528 to
1cb8217
Compare
|
I've added tests. @ezyang would appreciate a stamp as it's blocking the C++ frontend tutorial |
There was a problem hiding this comment.
@goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
soumith
added
the
cherry-picked
Summary: In Python, you can use the call operator to invoke the `forward()` method of a module. In C++ this was currently not possible, because I couldn't figure out how to deduce the return type of a module's `forward()` method under the constraint that `forward()` may not exist at all (since the base module class in C++ does not mandate a `forward()` method). I now figured it out, so the call operator can be used. ezyang ebetica Pull Request resolved: #15831 Differential Revision: D13652676 Pulled By: goldsborough fbshipit-source-id: ccab45a15215dda56460e560f0038781b539135f
Summary: In Python, you can use the call operator to invoke the `forward()` method of a module. In C++ this was currently not possible, because I couldn't figure out how to deduce the return type of a module's `forward()` method under the constraint that `forward()` may not exist at all (since the base module class in C++ does not mandate a `forward()` method). I now figured it out, so the call operator can be used. ezyang ebetica Pull Request resolved: #15831 Differential Revision: D13652676 Pulled By: goldsborough fbshipit-source-id: ccab45a15215dda56460e560f0038781b539135f
5 participants
In Python, you can use the call operator to invoke the
forward()method of a module. In C++ this was currently not possible, because I couldn't figure out how to deduce the return type of a module'sforward()method under the constraint thatforward()may not exist at all (since the base module class in C++ does not mandate aforward()method). I now figured it out, so the call operator can be used.@ezyang @ebetica