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

[Barnabás Pőcze]

Hi

2025. 03. 31. 20:13 keltezéssel, Kieran Bingham írta:
> 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

Yes, I know; it's missing a `[[maybe_unused]]`, but I did not want to
resend it immediately, but it passes with that small change:
https://gitlab.freedesktop.org/pobrn/libcamera/-/pipelines/1393539


Regards,
Barnabás Pőcze


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