| Message ID | 20210827023829.5871-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | d1315600b8d406291215efc2be722dd93a38e481 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 8/27/21 8:08 AM, Laurent Pinchart wrote: > The BoundMethodMember specialization for the void return type is only > needed to avoid accessing the ret_ member variable that is lacking from > the corresponding BoundMethodPack specialization. By adding a > BoundMethodPack::returnValue() function to read the member variable, we > can remove the complete BoundMethodMember specialization. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/base/bound_method.h | 46 ++++++--------------------- > 1 file changed, 10 insertions(+), 36 deletions(-) Ok Let's see, my review attempt #3 to understand these patches. There is quite a level of inheritance of classes going on here.. so I hope I don't get lost. > > diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h > index 282f9b58ab60..9c212f1ecc3a 100644 > --- a/include/libcamera/base/bound_method.h > +++ b/include/libcamera/base/bound_method.h > @@ -38,6 +38,11 @@ public: > { > } > > + R returnValue() > + { > + return ret_; > + } > + Make sense. > std::tuple<typename std::remove_reference_t<Args>...> args_; > R ret_; > }; > @@ -51,6 +56,10 @@ public: > { > } > > + void returnValue() > + { > + } > + Makes sense > std::tuple<typename std::remove_reference_t<Args>...> args_; > }; > > @@ -160,7 +169,7 @@ public: > > auto pack = std::make_shared<PackType>(args...); > bool sync = BoundMethodBase::activatePack(pack, deleteMethod); > - return sync ? pack->ret_ : R(); > + return sync ? pack->returnValue() : R(); Yep > } > > R invoke(Args... args) override > @@ -173,41 +182,6 @@ private: > R (T::*func_)(Args...); > }; > > -template<typename T, typename... Args> > -class BoundMethodMember<T, void, Args...> : public BoundMethodArgs<void, Args...> Ah nice. This is nuked. I was going to write that we probably need to nuke BoundMethodArgs<void, Args...> as no class inherits that now. ...But I already see that happening in 3/6, so slowly I am getting the essence of it yay! Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > -{ > -public: > - using PackType = typename BoundMethodArgs<void, Args...>::PackType; > - > - BoundMethodMember(T *obj, Object *object, void (T::*func)(Args...), > - ConnectionType type = ConnectionTypeAuto) > - : BoundMethodArgs<void, Args...>(obj, object, type), func_(func) > - { > - } > - > - bool match(void (T::*func)(Args...)) const { return func == func_; } > - > - void activate(Args... args, bool deleteMethod = false) override > - { > - if (!this->object_) { > - T *obj = static_cast<T *>(this->obj_); > - return (obj->*func_)(args...); > - } > - > - auto pack = std::make_shared<PackType>(args...); > - BoundMethodBase::activatePack(pack, deleteMethod); > - } > - > - void invoke(Args... args) override > - { > - T *obj = static_cast<T *>(this->obj_); > - return (obj->*func_)(args...); > - } > - > -private: > - void (T::*func_)(Args...); > -}; > - > template<typename R, typename... Args> > class BoundMethodStatic : public BoundMethodArgs<R, Args...> > {
diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h index 282f9b58ab60..9c212f1ecc3a 100644 --- a/include/libcamera/base/bound_method.h +++ b/include/libcamera/base/bound_method.h @@ -38,6 +38,11 @@ public: { } + R returnValue() + { + return ret_; + } + std::tuple<typename std::remove_reference_t<Args>...> args_; R ret_; }; @@ -51,6 +56,10 @@ public: { } + void returnValue() + { + } + std::tuple<typename std::remove_reference_t<Args>...> args_; }; @@ -160,7 +169,7 @@ public: auto pack = std::make_shared<PackType>(args...); bool sync = BoundMethodBase::activatePack(pack, deleteMethod); - return sync ? pack->ret_ : R(); + return sync ? pack->returnValue() : R(); } R invoke(Args... args) override @@ -173,41 +182,6 @@ private: R (T::*func_)(Args...); }; -template<typename T, typename... Args> -class BoundMethodMember<T, void, Args...> : public BoundMethodArgs<void, Args...> -{ -public: - using PackType = typename BoundMethodArgs<void, Args...>::PackType; - - BoundMethodMember(T *obj, Object *object, void (T::*func)(Args...), - ConnectionType type = ConnectionTypeAuto) - : BoundMethodArgs<void, Args...>(obj, object, type), func_(func) - { - } - - bool match(void (T::*func)(Args...)) const { return func == func_; } - - void activate(Args... args, bool deleteMethod = false) override - { - if (!this->object_) { - T *obj = static_cast<T *>(this->obj_); - return (obj->*func_)(args...); - } - - auto pack = std::make_shared<PackType>(args...); - BoundMethodBase::activatePack(pack, deleteMethod); - } - - void invoke(Args... args) override - { - T *obj = static_cast<T *>(this->obj_); - return (obj->*func_)(args...); - } - -private: - void (T::*func_)(Args...); -}; - template<typename R, typename... Args> class BoundMethodStatic : public BoundMethodArgs<R, Args...> {
The BoundMethodMember specialization for the void return type is only needed to avoid accessing the ret_ member variable that is lacking from the corresponding BoundMethodPack specialization. By adding a BoundMethodPack::returnValue() function to read the member variable, we can remove the complete BoundMethodMember specialization. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/base/bound_method.h | 46 ++++++--------------------- 1 file changed, 10 insertions(+), 36 deletions(-)