| Message ID | 20250331160301.534243-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@ideasonboard.com> > } > > public:
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@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@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... Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > } > > > > public: > > -- > Regards, > > Laurent Pinchart
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@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@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@ideasonboard.com> > > > > > > } > > > > > > public: > > > > -- > > Regards, > > > > Laurent Pinchart
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@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@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@ideasonboard.com> >> >>> >>>> } >>>> >>>> public: >>> >>> -- >>> Regards, >>> >>> Laurent Pinchart
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...>) { 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_)...); } public:
Use `if constexpr` instead of SFINAE to handle return values of type `void`. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/base/bound_method.h | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)