| Message ID | 20250619093218.1335931-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2025-06-19 10:32:18) > Instead of copying, just move the returned value when the call is made > through an argument pack. This enables, e.g. `Object::invokeMethod()` > to be usable with functions returning types, such as`std::unique_ptr`, > that have no copy ctor/assignment. Since there are no other users of > the argument pack object, this is safe to do. Reference return types > are not supported, so a simple `std::move()` is sufficient. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=273#c1 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Is there any ABI breakage on this that I need to consider? Otherwise Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > changes in v2: > * add `[[maybe_unused]]` for gcc 9 > * review comments > > v1: https://patchwork.libcamera.org/patch/23598/ > --- > include/libcamera/base/bound_method.h | 21 ++++++++------------- > test/object-invoke.cpp | 17 +++++++++++++++++ > 2 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h > index 507c320d3..e14614bc4 100644 > --- a/include/libcamera/base/bound_method.h > +++ b/include/libcamera/base/bound_method.h > @@ -38,11 +38,6 @@ public: > { > } > > - R returnValue() > - { > - return ret_; > - } > - > std::tuple<typename std::remove_reference_t<Args>...> args_; > R ret_; > }; > @@ -56,10 +51,6 @@ public: > { > } > > - void returnValue() > - { > - } > - > std::tuple<typename std::remove_reference_t<Args>...> args_; > }; > > @@ -140,8 +131,10 @@ public: > return func_(args...); > > auto pack = std::make_shared<PackType>(args...); > - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > - return sync ? pack->returnValue() : R(); > + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > + > + if constexpr (!std::is_void_v<R>) > + return sync ? std::move(pack->ret_) : R(); > } > > R invoke(Args... args) override > @@ -175,8 +168,10 @@ public: > } > > auto pack = std::make_shared<PackType>(args...); > - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > - return sync ? pack->returnValue() : R(); > + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > + > + if constexpr (!std::is_void_v<R>) > + return sync ? std::move(pack->ret_) : R(); > } > > R invoke(Args... args) override > diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp > index def1e61e4..ab4a15751 100644 > --- a/test/object-invoke.cpp > +++ b/test/object-invoke.cpp > @@ -58,6 +58,19 @@ public: > return 42; > } > > + struct MoveOnly { > + MoveOnly() = default; > + MoveOnly(const MoveOnly &) = delete; > + MoveOnly &operator=(const MoveOnly &) = delete; > + MoveOnly(MoveOnly &&) = default; > + MoveOnly &operator=(MoveOnly &&) = default; > + }; > + > + MoveOnly methodWithReturnMoveOnly() > + { > + return {}; > + } > + > private: > Status status_; > int value_; > @@ -186,6 +199,10 @@ protected: > return TestFail; > } > > + /* Test invoking a method that returns type with no copy ctor/assignment. */ > + object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly, > + ConnectionTypeBlocking); > + > return TestPass; > } > > -- > 2.50.0
Hi 2025. 07. 18. 15:54 keltezéssel, Kieran Bingham írta: > Quoting Barnabás Pőcze (2025-06-19 10:32:18) >> Instead of copying, just move the returned value when the call is made >> through an argument pack. This enables, e.g. `Object::invokeMethod()` >> to be usable with functions returning types, such as`std::unique_ptr`, >> that have no copy ctor/assignment. Since there are no other users of >> the argument pack object, this is safe to do. Reference return types >> are not supported, so a simple `std::move()` is sufficient. >> >> Bug: https://bugs.libcamera.org/show_bug.cgi?id=273#c1 >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Is there any ABI breakage on this that I need to consider? There is an API change. It removes `returnValue()`, but I would still like to merge it because no one should be using these types directly (and I would be surprised if anyone did). No effect on ABI. Thoughts? Regards, Barnabás Pőcze > > Otherwise > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> --- >> changes in v2: >> * add `[[maybe_unused]]` for gcc 9 >> * review comments >> >> v1: https://patchwork.libcamera.org/patch/23598/ >> --- >> include/libcamera/base/bound_method.h | 21 ++++++++------------- >> test/object-invoke.cpp | 17 +++++++++++++++++ >> 2 files changed, 25 insertions(+), 13 deletions(-) >> >> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h >> index 507c320d3..e14614bc4 100644 >> --- a/include/libcamera/base/bound_method.h >> +++ b/include/libcamera/base/bound_method.h >> @@ -38,11 +38,6 @@ public: >> { >> } >> >> - R returnValue() >> - { >> - return ret_; >> - } >> - >> std::tuple<typename std::remove_reference_t<Args>...> args_; >> R ret_; >> }; >> @@ -56,10 +51,6 @@ public: >> { >> } >> >> - void returnValue() >> - { >> - } >> - >> std::tuple<typename std::remove_reference_t<Args>...> args_; >> }; >> >> @@ -140,8 +131,10 @@ public: >> return func_(args...); >> >> auto pack = std::make_shared<PackType>(args...); >> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >> - return sync ? pack->returnValue() : R(); >> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >> + >> + if constexpr (!std::is_void_v<R>) >> + return sync ? std::move(pack->ret_) : R(); >> } >> >> R invoke(Args... args) override >> @@ -175,8 +168,10 @@ public: >> } >> >> auto pack = std::make_shared<PackType>(args...); >> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >> - return sync ? pack->returnValue() : R(); >> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >> + >> + if constexpr (!std::is_void_v<R>) >> + return sync ? std::move(pack->ret_) : R(); >> } >> >> R invoke(Args... args) override >> diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp >> index def1e61e4..ab4a15751 100644 >> --- a/test/object-invoke.cpp >> +++ b/test/object-invoke.cpp >> @@ -58,6 +58,19 @@ public: >> return 42; >> } >> >> + struct MoveOnly { >> + MoveOnly() = default; >> + MoveOnly(const MoveOnly &) = delete; >> + MoveOnly &operator=(const MoveOnly &) = delete; >> + MoveOnly(MoveOnly &&) = default; >> + MoveOnly &operator=(MoveOnly &&) = default; >> + }; >> + >> + MoveOnly methodWithReturnMoveOnly() >> + { >> + return {}; >> + } >> + >> private: >> Status status_; >> int value_; >> @@ -186,6 +199,10 @@ protected: >> return TestFail; >> } >> >> + /* Test invoking a method that returns type with no copy ctor/assignment. */ >> + object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly, >> + ConnectionTypeBlocking); >> + >> return TestPass; >> } >> >> -- >> 2.50.0
Quoting Barnabás Pőcze (2025-07-18 15:01:20) > Hi > > 2025. 07. 18. 15:54 keltezéssel, Kieran Bingham írta: > > Quoting Barnabás Pőcze (2025-06-19 10:32:18) > >> Instead of copying, just move the returned value when the call is made > >> through an argument pack. This enables, e.g. `Object::invokeMethod()` > >> to be usable with functions returning types, such as`std::unique_ptr`, > >> that have no copy ctor/assignment. Since there are no other users of > >> the argument pack object, this is safe to do. Reference return types > >> are not supported, so a simple `std::move()` is sufficient. > >> > >> Bug: https://bugs.libcamera.org/show_bug.cgi?id=273#c1 > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Is there any ABI breakage on this that I need to consider? > > There is an API change. It removes `returnValue()`, but I would still like > to merge it because no one should be using these types directly (and I would > be surprised if anyone did). No effect on ABI. Thoughts? > If it's helpful then I'd like it to be merged too. What's the result of running ./utils/abi-compat.sh ? -- Kieran > > Regards, > Barnabás Pőcze > > > > > Otherwise > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > >> --- > >> changes in v2: > >> * add `[[maybe_unused]]` for gcc 9 > >> * review comments > >> > >> v1: https://patchwork.libcamera.org/patch/23598/ > >> --- > >> include/libcamera/base/bound_method.h | 21 ++++++++------------- > >> test/object-invoke.cpp | 17 +++++++++++++++++ > >> 2 files changed, 25 insertions(+), 13 deletions(-) > >> > >> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h > >> index 507c320d3..e14614bc4 100644 > >> --- a/include/libcamera/base/bound_method.h > >> +++ b/include/libcamera/base/bound_method.h > >> @@ -38,11 +38,6 @@ public: > >> { > >> } > >> > >> - R returnValue() > >> - { > >> - return ret_; > >> - } > >> - > >> std::tuple<typename std::remove_reference_t<Args>...> args_; > >> R ret_; > >> }; > >> @@ -56,10 +51,6 @@ public: > >> { > >> } > >> > >> - void returnValue() > >> - { > >> - } > >> - > >> std::tuple<typename std::remove_reference_t<Args>...> args_; > >> }; > >> > >> @@ -140,8 +131,10 @@ public: > >> return func_(args...); > >> > >> auto pack = std::make_shared<PackType>(args...); > >> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >> - return sync ? pack->returnValue() : R(); > >> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >> + > >> + if constexpr (!std::is_void_v<R>) > >> + return sync ? std::move(pack->ret_) : R(); > >> } > >> > >> R invoke(Args... args) override > >> @@ -175,8 +168,10 @@ public: > >> } > >> > >> auto pack = std::make_shared<PackType>(args...); > >> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >> - return sync ? pack->returnValue() : R(); > >> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >> + > >> + if constexpr (!std::is_void_v<R>) > >> + return sync ? std::move(pack->ret_) : R(); > >> } > >> > >> R invoke(Args... args) override > >> diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp > >> index def1e61e4..ab4a15751 100644 > >> --- a/test/object-invoke.cpp > >> +++ b/test/object-invoke.cpp > >> @@ -58,6 +58,19 @@ public: > >> return 42; > >> } > >> > >> + struct MoveOnly { > >> + MoveOnly() = default; > >> + MoveOnly(const MoveOnly &) = delete; > >> + MoveOnly &operator=(const MoveOnly &) = delete; > >> + MoveOnly(MoveOnly &&) = default; > >> + MoveOnly &operator=(MoveOnly &&) = default; > >> + }; > >> + > >> + MoveOnly methodWithReturnMoveOnly() > >> + { > >> + return {}; > >> + } > >> + > >> private: > >> Status status_; > >> int value_; > >> @@ -186,6 +199,10 @@ protected: > >> return TestFail; > >> } > >> > >> + /* Test invoking a method that returns type with no copy ctor/assignment. */ > >> + object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly, > >> + ConnectionTypeBlocking); > >> + > >> return TestPass; > >> } > >> > >> -- > >> 2.50.0 >
Hi 2025. 07. 18. 16:07 keltezéssel, Kieran Bingham írta: > Quoting Barnabás Pőcze (2025-07-18 15:01:20) >> Hi >> >> 2025. 07. 18. 15:54 keltezéssel, Kieran Bingham írta: >>> Quoting Barnabás Pőcze (2025-06-19 10:32:18) >>>> Instead of copying, just move the returned value when the call is made >>>> through an argument pack. This enables, e.g. `Object::invokeMethod()` >>>> to be usable with functions returning types, such as`std::unique_ptr`, >>>> that have no copy ctor/assignment. Since there are no other users of >>>> the argument pack object, this is safe to do. Reference return types >>>> are not supported, so a simple `std::move()` is sufficient. >>>> >>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=273#c1 >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> >>> Is there any ABI breakage on this that I need to consider? >> >> There is an API change. It removes `returnValue()`, but I would still like >> to merge it because no one should be using these types directly (and I would >> be surprised if anyone did). No effect on ABI. Thoughts? >> > > If it's helpful then I'd like it to be merged too. > > What's the result of running ./utils/abi-compat.sh ? Apparently it doesn't detect the change? It says 100% source and binary compatibility, and it is not listed under "Removed Symbols". This was between v0.5.1 and a9c2dd05fa3783e2284c64c558122ceb632eab1b + this patch. > > -- > Kieran > > >> >> Regards, >> Barnabás Pőcze >> >>> >>> Otherwise >>> >>> >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >>> >>>> --- >>>> changes in v2: >>>> * add `[[maybe_unused]]` for gcc 9 >>>> * review comments >>>> >>>> v1: https://patchwork.libcamera.org/patch/23598/ >>>> --- >>>> include/libcamera/base/bound_method.h | 21 ++++++++------------- >>>> test/object-invoke.cpp | 17 +++++++++++++++++ >>>> 2 files changed, 25 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h >>>> index 507c320d3..e14614bc4 100644 >>>> --- a/include/libcamera/base/bound_method.h >>>> +++ b/include/libcamera/base/bound_method.h >>>> @@ -38,11 +38,6 @@ public: >>>> { >>>> } >>>> >>>> - R returnValue() >>>> - { >>>> - return ret_; >>>> - } >>>> - >>>> std::tuple<typename std::remove_reference_t<Args>...> args_; >>>> R ret_; >>>> }; >>>> @@ -56,10 +51,6 @@ public: >>>> { >>>> } >>>> >>>> - void returnValue() >>>> - { >>>> - } >>>> - >>>> std::tuple<typename std::remove_reference_t<Args>...> args_; >>>> }; >>>> >>>> @@ -140,8 +131,10 @@ public: >>>> return func_(args...); >>>> >>>> auto pack = std::make_shared<PackType>(args...); >>>> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >>>> - return sync ? pack->returnValue() : R(); >>>> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >>>> + >>>> + if constexpr (!std::is_void_v<R>) >>>> + return sync ? std::move(pack->ret_) : R(); >>>> } >>>> >>>> R invoke(Args... args) override >>>> @@ -175,8 +168,10 @@ public: >>>> } >>>> >>>> auto pack = std::make_shared<PackType>(args...); >>>> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >>>> - return sync ? pack->returnValue() : R(); >>>> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); >>>> + >>>> + if constexpr (!std::is_void_v<R>) >>>> + return sync ? std::move(pack->ret_) : R(); >>>> } >>>> >>>> R invoke(Args... args) override >>>> diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp >>>> index def1e61e4..ab4a15751 100644 >>>> --- a/test/object-invoke.cpp >>>> +++ b/test/object-invoke.cpp >>>> @@ -58,6 +58,19 @@ public: >>>> return 42; >>>> } >>>> >>>> + struct MoveOnly { >>>> + MoveOnly() = default; >>>> + MoveOnly(const MoveOnly &) = delete; >>>> + MoveOnly &operator=(const MoveOnly &) = delete; >>>> + MoveOnly(MoveOnly &&) = default; >>>> + MoveOnly &operator=(MoveOnly &&) = default; >>>> + }; >>>> + >>>> + MoveOnly methodWithReturnMoveOnly() >>>> + { >>>> + return {}; >>>> + } >>>> + >>>> private: >>>> Status status_; >>>> int value_; >>>> @@ -186,6 +199,10 @@ protected: >>>> return TestFail; >>>> } >>>> >>>> + /* Test invoking a method that returns type with no copy ctor/assignment. */ >>>> + object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly, >>>> + ConnectionTypeBlocking); >>>> + >>>> return TestPass; >>>> } >>>> >>>> -- >>>> 2.50.0 >>
Quoting Barnabás Pőcze (2025-07-18 15:25:58) > Hi > > 2025. 07. 18. 16:07 keltezéssel, Kieran Bingham írta: > > Quoting Barnabás Pőcze (2025-07-18 15:01:20) > >> Hi > >> > >> 2025. 07. 18. 15:54 keltezéssel, Kieran Bingham írta: > >>> Quoting Barnabás Pőcze (2025-06-19 10:32:18) > >>>> Instead of copying, just move the returned value when the call is made > >>>> through an argument pack. This enables, e.g. `Object::invokeMethod()` > >>>> to be usable with functions returning types, such as`std::unique_ptr`, > >>>> that have no copy ctor/assignment. Since there are no other users of > >>>> the argument pack object, this is safe to do. Reference return types > >>>> are not supported, so a simple `std::move()` is sufficient. > >>>> > >>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=273#c1 > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> > >>> Is there any ABI breakage on this that I need to consider? > >> > >> There is an API change. It removes `returnValue()`, but I would still like > >> to merge it because no one should be using these types directly (and I would > >> be surprised if anyone did). No effect on ABI. Thoughts? > >> > > > > If it's helpful then I'd like it to be merged too. > > > > What's the result of running ./utils/abi-compat.sh ? > > Apparently it doesn't detect the change? It says 100% source and binary > compatibility, and it is not listed under "Removed Symbols". This was > between v0.5.1 and a9c2dd05fa3783e2284c64c558122ceb632eab1b + this patch. abi-compat will only check the public symbols, so maybe this one wasn't public ? Anyway - perhaps that means it's easy to merge, as both the tooling is fine - and you believe it shouldn't be used anyway. Feel free to push. -- Regards Kieran > > > > > > -- > > Kieran > > > > > >> > >> Regards, > >> Barnabás Pőcze > >> > >>> > >>> Otherwise > >>> > >>> > >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >>> > >>>> --- > >>>> changes in v2: > >>>> * add `[[maybe_unused]]` for gcc 9 > >>>> * review comments > >>>> > >>>> v1: https://patchwork.libcamera.org/patch/23598/ > >>>> --- > >>>> include/libcamera/base/bound_method.h | 21 ++++++++------------- > >>>> test/object-invoke.cpp | 17 +++++++++++++++++ > >>>> 2 files changed, 25 insertions(+), 13 deletions(-) > >>>> > >>>> diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h > >>>> index 507c320d3..e14614bc4 100644 > >>>> --- a/include/libcamera/base/bound_method.h > >>>> +++ b/include/libcamera/base/bound_method.h > >>>> @@ -38,11 +38,6 @@ public: > >>>> { > >>>> } > >>>> > >>>> - R returnValue() > >>>> - { > >>>> - return ret_; > >>>> - } > >>>> - > >>>> std::tuple<typename std::remove_reference_t<Args>...> args_; > >>>> R ret_; > >>>> }; > >>>> @@ -56,10 +51,6 @@ public: > >>>> { > >>>> } > >>>> > >>>> - void returnValue() > >>>> - { > >>>> - } > >>>> - > >>>> std::tuple<typename std::remove_reference_t<Args>...> args_; > >>>> }; > >>>> > >>>> @@ -140,8 +131,10 @@ public: > >>>> return func_(args...); > >>>> > >>>> auto pack = std::make_shared<PackType>(args...); > >>>> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >>>> - return sync ? pack->returnValue() : R(); > >>>> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >>>> + > >>>> + if constexpr (!std::is_void_v<R>) > >>>> + return sync ? std::move(pack->ret_) : R(); > >>>> } > >>>> > >>>> R invoke(Args... args) override > >>>> @@ -175,8 +168,10 @@ public: > >>>> } > >>>> > >>>> auto pack = std::make_shared<PackType>(args...); > >>>> - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >>>> - return sync ? pack->returnValue() : R(); > >>>> + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > >>>> + > >>>> + if constexpr (!std::is_void_v<R>) > >>>> + return sync ? std::move(pack->ret_) : R(); > >>>> } > >>>> > >>>> R invoke(Args... args) override > >>>> diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp > >>>> index def1e61e4..ab4a15751 100644 > >>>> --- a/test/object-invoke.cpp > >>>> +++ b/test/object-invoke.cpp > >>>> @@ -58,6 +58,19 @@ public: > >>>> return 42; > >>>> } > >>>> > >>>> + struct MoveOnly { > >>>> + MoveOnly() = default; > >>>> + MoveOnly(const MoveOnly &) = delete; > >>>> + MoveOnly &operator=(const MoveOnly &) = delete; > >>>> + MoveOnly(MoveOnly &&) = default; > >>>> + MoveOnly &operator=(MoveOnly &&) = default; > >>>> + }; > >>>> + > >>>> + MoveOnly methodWithReturnMoveOnly() > >>>> + { > >>>> + return {}; > >>>> + } > >>>> + > >>>> private: > >>>> Status status_; > >>>> int value_; > >>>> @@ -186,6 +199,10 @@ protected: > >>>> return TestFail; > >>>> } > >>>> > >>>> + /* Test invoking a method that returns type with no copy ctor/assignment. */ > >>>> + object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly, > >>>> + ConnectionTypeBlocking); > >>>> + > >>>> return TestPass; > >>>> } > >>>> > >>>> -- > >>>> 2.50.0 > >> >
diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h index 507c320d3..e14614bc4 100644 --- a/include/libcamera/base/bound_method.h +++ b/include/libcamera/base/bound_method.h @@ -38,11 +38,6 @@ public: { } - R returnValue() - { - return ret_; - } - std::tuple<typename std::remove_reference_t<Args>...> args_; R ret_; }; @@ -56,10 +51,6 @@ public: { } - void returnValue() - { - } - std::tuple<typename std::remove_reference_t<Args>...> args_; }; @@ -140,8 +131,10 @@ public: return func_(args...); auto pack = std::make_shared<PackType>(args...); - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); - return sync ? pack->returnValue() : R(); + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); + + if constexpr (!std::is_void_v<R>) + return sync ? std::move(pack->ret_) : R(); } R invoke(Args... args) override @@ -175,8 +168,10 @@ public: } auto pack = std::make_shared<PackType>(args...); - bool sync = BoundMethodBase::activatePack(pack, deleteMethod); - return sync ? pack->returnValue() : R(); + [[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod); + + if constexpr (!std::is_void_v<R>) + return sync ? std::move(pack->ret_) : R(); } R invoke(Args... args) override diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp index def1e61e4..ab4a15751 100644 --- a/test/object-invoke.cpp +++ b/test/object-invoke.cpp @@ -58,6 +58,19 @@ public: return 42; } + struct MoveOnly { + MoveOnly() = default; + MoveOnly(const MoveOnly &) = delete; + MoveOnly &operator=(const MoveOnly &) = delete; + MoveOnly(MoveOnly &&) = default; + MoveOnly &operator=(MoveOnly &&) = default; + }; + + MoveOnly methodWithReturnMoveOnly() + { + return {}; + } + private: Status status_; int value_; @@ -186,6 +199,10 @@ protected: return TestFail; } + /* Test invoking a method that returns type with no copy ctor/assignment. */ + object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly, + ConnectionTypeBlocking); + return TestPass; }