[libcamera-devel] [PATCH v5 1/9] libcamera: base: utils: Provide defopt to simplify std::optional::value_or() usage
Florian Sylvestre
fsylvestre at baylibre.com
Fri Jul 29 09:10:21 CEST 2022
Hi Laurent,
On Thu, 28 Jul 2022 at 12:30, Laurent Pinchart via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Hi Jacopo,
>
> On Thu, Jul 28, 2022 at 08:47:05AM +0200, Jacopo Mondi wrote:
> > Hi Laurent
> >
> > On Thu, Jul 28, 2022 at 01:21:41AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > The std::optional<T>::value_or(U &&default_value) function returns the
> > > contained value if available, or \a default_value if the std::optional
> >
> > Are the \a in the commit message intended ?
>
> Oops :-) I'll drop it.
>
> > > has no value. If the desired default value is a default-constructed T,
> > > the obvious option is to call std::optional<T>::value_or(T{}). This
> > > approach has two drawbacks:
> > >
> > > - The \a default_value T{} is constructed even if the std::optional
> > > instance has a value, which impacts efficiency.
> > > - The T{} default constructor needs to be spelled out explicitly in the
> > > value_or() call, leading to long lines if the type is complex.
> > >
> > > Introduce a defopt variable that solves these issues by providing a
> > > value that can be passed to std::optional<T>::value_or() and get
> > > implicitly converted to a default-constructed T.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > include/libcamera/base/utils.h | 14 +++++++++
> > > src/libcamera/base/utils.cpp | 21 +++++++++++++
> > > test/utils.cpp | 54 ++++++++++++++++++++++++++++++++++
> > > 3 files changed, 89 insertions(+)
> > >
> > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> > > index cfff05836de7..889bb4a2270e 100644
> > > --- a/include/libcamera/base/utils.h
> > > +++ b/include/libcamera/base/utils.h
> > > @@ -361,6 +361,20 @@ decltype(auto) abs_diff(const T &a, const T &b)
> > > return a - b;
> > > }
> > >
> > > +namespace details {
> > > +
> > > +struct defopt_t {
> > > + template<typename T>
> > > + operator T() const
> > > + {
> > > + return T{};
> > > + }
> > > +};
> > > +
> > > +} /* namespace details */
> > > +
> > > +constexpr details::defopt_t defopt;
> > > +
> >
> > Clever! this indeed explains how it works:
> >
> > template< class U >
> > constexpr T value_or( U&& default_value ) const&;
> >
> > Equivalent to bool(*this) ? **this : static_cast<T>(std::forward<U>(default_value))
> >
> >
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > > } /* namespace utils */
> > >
> > > #ifndef __DOXYGEN__
> > > diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> > > index 6a307940448e..9cd6cb197243 100644
> > > --- a/src/libcamera/base/utils.cpp
> > > +++ b/src/libcamera/base/utils.cpp
> > > @@ -463,6 +463,27 @@ std::string toAscii(const std::string &str)
> > > * \a b
> > > */
> > >
> > > +/**
> > > + * \var defopt
> > > + * \brief Constant used with std::optional::value_or() to create a
> > > + * default-constructed object
> > > + *
> > > + * The std::optional<T>::value_or(U &&default_value) function returns the
> > > + * contained value if available, or \a default_value if the std::optional has no
> > > + * value. If the desired default value is a default-constructed T, the obvious
> > > + * option is to call std::optional<T>::value_or(T{}). This approach has two
> > > + * drawbacks:
> > > + *
> > > + * * The \a default_value T{} is constructed even if the std::optional instance
> > > + * has a value, which impacts efficiency.
> > > + * * The T{} default constructor needs to be spelled out explicitly in the
> > > + * value_or() call, leading to long lines if the type is complex.
> > > + *
> > > + * The defopt variable solves these issues by providing a value that can be
> > > + * passed to std::optional<T>::value_or() and get implicitly converted to a
> > > + * default-constructed T.
> > > + */
> > > +
> > > } /* namespace utils */
> > >
> > > #ifndef __DOXYGEN__
> > > diff --git a/test/utils.cpp b/test/utils.cpp
> > > index d65467b5102c..129807a63ec6 100644
> > > --- a/test/utils.cpp
> > > +++ b/test/utils.cpp
> > > @@ -7,6 +7,7 @@
> > >
> > > #include <iostream>
> > > #include <map>
> > > +#include <optional>
> > > #include <sstream>
> > > #include <string>
> > > #include <vector>
> > > @@ -169,6 +170,55 @@ protected:
> > > return TestPass;
> > > }
> > >
> > > + int testDefopt()
> > > + {
> > > + static bool defaultConstructed = false;
> > > +
> > > + struct ValueType {
> > > + ValueType()
> > > + : value_(-1)
> > > + {
> > > + defaultConstructed = true;
> > > + }
> > > +
> > > + ValueType(int value)
> > > + : value_(value)
> > > + {
> > > + }
> > > +
> > > + int value_;
> > > + };
> > > +
> > > + /*
> > > + * Test that utils::defopt doesn't cause default-construction
> > > + * of a ValueType instance when value_or(utils::defopt) is
> > > + * called on a std::optional that has a value.
> > > + */
> > > + std::optional<ValueType> opt = ValueType(0);
> > > + ValueType value = opt.value_or(utils::defopt);
> > > +
> > > + if (defaultConstructed || value.value_ != 0) {
> > > + std::cerr << "utils::defopt didn't prevent default construction"
> > > + << std::endl;
> > > + return TestFail;
> > > + }
> > > +
> > > + /*
> > > + * Then test that the ValueType is correctly default-constructed
> > > + * when the std::optional has no value.
> > > + */
> > > + opt = std::nullopt;
> > > + value = opt.value_or(utils::defopt);
> > > +
> > > + if (!defaultConstructed || value.value_ != -1) {
> > > + std::cerr << "utils::defopt didn't cause default construction"
> > > + << std::endl;
> > > + return TestFail;
> > > + }
> > > +
> > > + return TestPass;
> > > + }
> > > +
> > > int run()
> > > {
> > > /* utils::hex() test. */
> > > @@ -281,6 +331,10 @@ protected:
> > > if (testDuration() != TestPass)
> > > return TestFail;
> > >
> > > + /* utils::defopt test. */
> > > + if (testDefopt() != TestPass)
> > > + return TestFail;
> > > +
> > > return TestPass;
> > > }
> > > };
>
> --
> Regards,
>
> Laurent Pinchart
Reviewed-by: Florian Sylvestre <fsylvestre at baylibre.com>
--
Florian Sylvestre
More information about the libcamera-devel
mailing list