[PATCH v1] libcamera: base: bound_method: Simplify `invokePack()`

[Kieran Bingham]

Quoting Kieran Bingham (2025-03-31 17:58:31)
> Quoting Laurent Pinchart (2025-03-31 17:21:58)
> > Hi Barnabás,
> > 
> > Thank you for the patch.
> > 
> > On Mon, Mar 31, 2025 at 06:03:01PM +0200, Barnabás Pőcze wrote:
> > > Use `if constexpr` instead of SFINAE to handle return values of type `void`.
> > > 
> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> > > ---
> > >  include/libcamera/base/bound_method.h | 17 ++++++-----------
> > >  1 file changed, 6 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h
> > > index dd3488eeb..aad75c02a 100644
> > > --- a/include/libcamera/base/bound_method.h
> > > +++ b/include/libcamera/base/bound_method.h
> > > @@ -98,21 +98,16 @@ public:
> > >       using PackType = BoundMethodPack<R, Args...>;
> > >  
> > >  private:
> > > -     template<std::size_t... I, typename T = R>
> > > -     std::enable_if_t<!std::is_void<T>::value, void>
> > > +     template<std::size_t... I>
> > > +     void
> > >       invokePack(BoundMethodPackBase *pack, std::index_sequence<I...>)
> > 
> > You can write
> > 
> >         template<std::size_t... I>
> >         void invokePack(BoundMethodPackBase *pack, std::index_sequence<I...>)
> > 
> > >       {
> > >               PackType *args = static_cast<PackType *>(pack);
> > > -             args->ret_ = invoke(std::get<I>(args->args_)...);
> > > -     }
> > >  
> > > -     template<std::size_t... I, typename T = R>
> > > -     std::enable_if_t<std::is_void<T>::value, void>
> > > -     invokePack(BoundMethodPackBase *pack, std::index_sequence<I...>)
> > > -     {
> > > -             /* args is effectively unused when the sequence I is empty. */
> > > -             PackType *args [[gnu::unused]] = static_cast<PackType *>(pack);
> > > -             invoke(std::get<I>(args->args_)...);
> > > +             if constexpr (!std::is_void_v<R>)
> > > +                     args->ret_ = invoke(std::get<I>(args->args_)...);
> > > +             else
> > > +                     invoke(std::get<I>(args->args_)...);
> > 
> > I'm surprised that this works. I didn't know that within a templated
> > entity the discarded statement is not instantiated. It's interesting.
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> No chance of me putting a reviewed by tag on template magic - it would
> be a lie ;-)
> 
> But if Laurent's happy with this - and the CI passes, I'm fine to merge
> it...

Unfortunately, it didn't pass :

https://gitlab.freedesktop.org/camera/libcamera/-/jobs/73682114

> 
> Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > 
> > >       }
> > >  
> > >  public:
> > 
> > -- 
> > Regards,
> > 
> > Laurent Pinchart