[libcamera-devel] [PATCH v2 1/5] libcamera: flags: Add type-safe enum-based flags

[Laurent Pinchart]

Hi Jacopo,

On Mon, Jul 26, 2021 at 11:42:23AM +0200, Jacopo Mondi wrote:
> On Sun, Jul 25, 2021 at 08:18:23PM +0300, Laurent Pinchart wrote:
> > Add a Flags template class that provide type-safe bitwise operators on
> > enum values. This allows using enum types for bit fields, without giving
> > away type-safety as usually done when storing combined flags in integer
> > variables.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  include/libcamera/base/flags.h     | 195 +++++++++++++++++++++++++++++
> >  include/libcamera/base/meson.build |   1 +
> >  src/libcamera/base/flags.cpp       | 192 ++++++++++++++++++++++++++++
> >  src/libcamera/base/meson.build     |   1 +
> >  4 files changed, 389 insertions(+)
> >  create mode 100644 include/libcamera/base/flags.h
> >  create mode 100644 src/libcamera/base/flags.cpp
> >
> > diff --git a/include/libcamera/base/flags.h b/include/libcamera/base/flags.h
> > new file mode 100644
> > index 000000000000..adec549dccc9
> > --- /dev/null
> > +++ b/include/libcamera/base/flags.h
> > @@ -0,0 +1,195 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * flags.h - Type-safe enum-based bitfields
> > + */
> > +#ifndef __LIBCAMERA_BASE_FLAGS_H__
> > +#define __LIBCAMERA_BASE_FLAGS_H__
> > +
> > +#include <type_traits>
> > +
> > +namespace libcamera {
> > +
> > +template<typename E>
> > +class Flags
> > +{
> > +public:
> > +	static_assert(std::is_enum<E>::value,
> > +		      "Flags<> template parameter must be an enum");
> > +
> > +	using Type = std::underlying_type_t<E>;
> > +
> > +	constexpr Flags()
> > +		: value_(0)
> > +	{
> > +	}
> > +
> > +	constexpr Flags(E flag)
> > +		: value_(static_cast<Type>(flag))
> > +	{
> > +	}
> > +
> > +	constexpr Flags &operator&=(E flag)
> > +	{
> > +		value_ &= static_cast<Type>(flag);
> > +		return *this;
> > +	}
> > +
> > +	constexpr Flags &operator&=(Flags other)
> 
> Should other be const ?

It's a value, not a reference, so I don't see a need to make it const.

> > +	{
> > +		value_ &= other.value_;
> > +		return *this;
> > +	}
> > +
> > +	constexpr Flags &operator|=(E flag)
> > +	{
> > +		value_ |= static_cast<Type>(flag);
> > +		return *this;
> > +	}
> > +
> > +	constexpr Flags &operator|=(Flags other)
> > +	{
> > +		value_ |= other.value_;
> > +		return *this;
> > +	}
> > +
> > +	constexpr Flags &operator^=(E flag)
> > +	{
> > +		value_ ^= static_cast<Type>(flag);
> > +		return *this;
> > +	}
> > +
> > +	constexpr Flags &operator^=(Flags other)
> > +	{
> > +		value_ ^= other.value_;
> > +		return *this;
> > +	}
> > +
> > +	constexpr bool operator==(E flag)
> > +	{
> > +		return value_ == static_cast<Type>(flag);
> > +	}
> > +
> > +	constexpr bool operator==(Flags other)
> > +	{
> > +		return value_ == static_cast<Type>(other);
> > +	}
> > +
> > +	constexpr bool operator!=(E flag)
> > +	{
> > +		return value_ != static_cast<Type>(flag);
> > +	}
> > +
> > +	constexpr bool operator!=(Flags other)
> > +	{
> > +		return value_ != static_cast<Type>(other);
> > +	}
> > +
> > +	constexpr explicit operator Type() const
> > +	{
> > +		return value_;
> > +	}
> > +
> > +	constexpr explicit operator bool() const
> > +	{
> > +		return !!value_;
> > +	}
> > +
> > +	constexpr Flags operator&(E flag) const
> > +	{
> > +		return Flags(static_cast<E>(value_ & static_cast<Type>(flag)));
> > +	}
> > +
> > +	constexpr Flags operator&(Flags other) const
> > +	{
> > +		return Flags(static_cast<E>(value_ & other.value_));
> > +	}
> > +
> > +	constexpr Flags operator|(E flag) const
> > +	{
> > +		return Flags(static_cast<E>(value_ | static_cast<Type>(flag)));
> > +	}
> > +
> > +	constexpr Flags operator|(Flags other) const
> > +	{
> > +		return Flags(static_cast<E>(value_ | other.value_));
> > +	}
> > +
> > +	constexpr Flags operator^(E flag) const
> > +	{
> > +		return Flags(static_cast<E>(value_ ^ static_cast<Type>(flag)));
> > +	}
> > +
> > +	constexpr Flags operator^(Flags other) const
> > +	{
> > +		return Flags(static_cast<E>(value_ ^ other.value_));
> > +	}
> > +
> > +	constexpr Flags operator~() const
> > +	{
> > +		return Flags(static_cast<E>(~value_));
> > +	}
> > +
> > +	constexpr bool operator!() const
> > +	{
> > +		return !value_;
> > +	}
> > +
> > +private:
> > +	Type value_;
> > +};
> > +
> > +#ifndef __DOXYGEN__
> > +template<typename E>
> > +struct flags_enable_operators {
> > +	static const bool enable = false;
> > +};
> > +
> > +template<typename E>
> > +typename std::enable_if_t<flags_enable_operators<E>::enable, Flags<E>>
> > +operator|(E lhs, E rhs)
> > +{
> > +	using type = std::underlying_type_t<E>;
> > +	return Flags<E>(static_cast<E>(static_cast<type>(lhs) | static_cast<type>(rhs)));
> > +}
> > +
> > +template<typename E>
> > +typename std::enable_if_t<flags_enable_operators<E>::enable, Flags<E>>
> > +operator&(E lhs, E rhs)
> > +{
> > +	using type = std::underlying_type_t<E>;
> > +	return Flags<E>(static_cast<E>(static_cast<type>(lhs) & static_cast<type>(rhs)));
> > +}
> > +
> > +template<typename E>
> > +typename std::enable_if_t<flags_enable_operators<E>::enable, Flags<E>>
> > +operator^(E lhs, E rhs)
> > +{
> > +	using type = std::underlying_type_t<E>;
> > +	return Flags<E>(static_cast<E>(static_cast<type>(lhs) ^ static_cast<type>(rhs)));
> > +}
> > +
> > +template<typename E>
> > +typename std::enable_if_t<flags_enable_operators<E>::enable, Flags<E>>
> > +operator~(E rhs)
> > +{
> > +	using type = std::underlying_type_t<E>;
> > +	return Flags<E>(static_cast<E>(~static_cast<type>(rhs)));
> > +}
> > +
> > +#define LIBCAMERA_FLAGS_ENABLE_OPERATORS(_enum)				\
> > +template<>								\
> > +struct flags_enable_operators<_enum> {					\
> > +	static const bool enable = true;				\
> > +};
> > +
> > +#else /* __DOXYGEN__ */
> > +
> > +#define LIBCAMERA_FLAGS_ENABLE_OPERATORS(_enum)
> > +
> > +#endif /* __DOXYGEN__ */
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_BASE_FLAGS_H__ */
> > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
> > index 7c499b558fb0..9feb4b9346d5 100644
> > --- a/include/libcamera/base/meson.build
> > +++ b/include/libcamera/base/meson.build
> > @@ -9,6 +9,7 @@ libcamera_base_headers = files([
> >      'event_dispatcher_poll.h',
> >      'event_notifier.h',
> >      'file.h',
> > +    'flags.h',
> >      'log.h',
> >      'message.h',
> >      'object.h',
> > diff --git a/src/libcamera/base/flags.cpp b/src/libcamera/base/flags.cpp
> > new file mode 100644
> > index 000000000000..3e4320ace7c8
> > --- /dev/null
> > +++ b/src/libcamera/base/flags.cpp
> > @@ -0,0 +1,192 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * flags.cpp - Type-safe enum-based bitfields
> > + */
> > +
> > +#include <libcamera/base/flags.h>
> > +
> > +/**
> > + * \file base/flags.h
> > + * \brief Enum-based bit fields
> > + */
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \class Flags
> > + * \brief Type-safe container for enum-based bitfields
> > + *
> > + * The Flags template class provides type-safe bitwise operators on enum values.
> > + * It allows using enum types for bitfields, while preventing unsafe casts from
> > + * integer types and mixing of flags from different enum types.
> > + *
> > + * To use the Flags class, declare an enum containing the desired bit flags, and
> > + * use the Flags<enum> class to store bitfields based on the enum. If bitwise
> > + * operators on the underlying enum are also desired, they can be enabled with
> > + * the LIBCAMERA_FLAGS_ENABLE_OPERATORS(enum) macro.
> > + */
> > +
> > +/**
> > + * \typedef Flags::Type
> > + * \brief The underlying data type of the enum
> > + */
> > +
> > +/**
> > + * \fn Flags::Flags()
> > + * \brief Construct a Flags instance with a zero value
> > + */
> > +
> > +/**
> > + * \fn Flags::Flags(E flag)
> > + * \brief Construct a Flags instance storing the \a flag
> > + * \param[in] flag The initial value
> > + */
> > +
> > +/**
> > + * \fn Flags &Flags::operator&=(E flag)
> > + * \brief Store the bitwise AND of this Flags and the \a flag in this Flags
> > + * \param[in] flag The second operand
> > + * \return A reference to this Flags
> > + */
> > +
> > +/**
> > + * \fn Flags &Flags::operator&=(Flags other)
> > + * \brief Store the bitwise AND of this Flags and the \a other Flags in this Flags
> > + * \param[in] other The second operand
> > + * \return A reference to this Flags
> > + */
> > +
> > +/**
> > + * \fn Flags &Flags::operator|=(E flag)
> > + * \brief Store the bitwise OR of this Flags and the \a flag in this Flags
> > + * \param[in] flag The second operand
> > + * \return A reference to this Flags
> > + */
> > +
> > +/**
> > + * \fn Flags &Flags::operator|=(Flags other)
> > + * \brief Store the bitwise OR of this Flags and the \a other Flags in this Flags
> > + * \param[in] other The second operand
> > + * \return A reference to this Flags
> > + */
> > +
> > +/**
> > + * \fn Flags &Flags::operator^=(E flag)
> > + * \brief Store the bitwise XOR of this Flags and the \a flag in this Flags
> > + * \param[in] flag The second operand
> > + * \return A reference to this Flags
> > + */
> > +
> > +/**
> > + * \fn Flags &Flags::operator^=(Flags other)
> > + * \brief Store the bitwise XOR of this Flags and the \a other Flags in this Flags
> > + * \param[in] other The second operand
> > + * \return A reference to this Flags
> > + */
> > +
> > +/**
> > + * \fn bool Flags::operator==(E flag)
> > + * \brief Compare flags for equality
> > + * \param[in] flag The second operand
> > + * \return True if the Flags and \a flag are equal, false otherwise
> > + */
> > +
> > +/**
> > + * \fn bool Flags::operator==(Flags other)
> > + * \brief Compare flags for equality
> > + * \param[in] other The second operand
> > + * \return True if the Flags and \a other are equal, false otherwise
> > + */
> > +
> > +/**
> > + * \fn bool Flags::operator!=(E flag)
> > + * \brief Compare flags for non-equality
> > + * \param[in] flag The second operand
> > + * \return True if the Flags and \a flag are not equal, false otherwise
> > + */
> > +
> > +/**
> > + * \fn bool Flags::operator!=(Flags other)
> > + * \brief Compare flags for non-equality
> > + * \param[in] other The second operand
> > + * \return True if the Flags and \a other are not equal, false otherwise
> > + */
> > +
> > +/**
> > + * \fn Flags::operator Type() const
> > + * \brief Convert the Flags to the underlying integer type
> > + * \return The Flags value as an integer
> > + */
> > +
> > +/**
> > + * \fn Flags::operator bool() const
> > + * \brief Convert the Flags to a boolean
> > + * \return True if at least one flag is set, false otherwise
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator&(E flag) const
> > + * \brief Compute the bitwise AND of this Flags and the \a flag
> > + * \param[in] flag The second operand
> > + * \return A Flags containing the result of the AND operation
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator&(Flags other) const
> > + * \brief Compute the bitwise AND of this Flags and the \a other Flags
> > + * \param[in] other The second operand
> > + * \return A Flags containing the result of the AND operation
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator|(E flag) const
> > + * \brief Compute the bitwise OR of this Flags and the \a flag
> > + * \param[in] flag The second operand
> > + * \return A Flags containing the result of the OR operation
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator|(Flags other) const
> > + * \brief Compute the bitwise OR of this Flags and the \a other Flags
> > + * \param[in] other The second operand
> > + * \return A Flags containing the result of the OR operation
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator^(E flag) const
> > + * \brief Compute the bitwise XOR of this Flags and the \a flag
> > + * \param[in] flag The second operand
> > + * \return A Flags containing the result of the XOR operation
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator^(Flags other) const
> > + * \brief Compute the bitwise XOR of this Flags and the \a other Flags
> > + * \param[in] other The second operand
> > + * \return A Flags containing the result of the XOR operation
> > + */
> > +
> > +/**
> > + * \fn Flags Flags::operator~() const
> > + * \brief Compute the bitwise NOT of this Flags
> > + * \return A Flags containing the result of the NOT operation
> > + */
> > +
> > +/**
> > + * \fn bool Flags::operator!() const
> > + * \brief Check if flags are set
> > + * \return True if no flags is set, false otherwise
> > + */
> > +
> > +/**
> > + * \def LIBCAMERA_FLAGS_ENABLE_OPERATORS(enum)
> > + * \brief Enable bitwise operations on the \a enum enumeration
> > + *
> > + * This macro enables the bitwise AND, OR, XOR and NOT operators on the given
> > + * \a enum. This allows the enum values to be safely used in bitwise operations
> > + * with the Flags<> class.
> > + */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> > index 871721571e98..d799c66d8b3a 100644
> > --- a/src/libcamera/base/meson.build
> > +++ b/src/libcamera/base/meson.build
> > @@ -7,6 +7,7 @@ libcamera_base_sources = files([
> >      'event_dispatcher_poll.cpp',
> >      'event_notifier.cpp',
> >      'file.cpp',
> > +    'flags.cpp',
> >      'log.cpp',
> >      'message.cpp',
> >      'object.cpp',
> 
> Great!
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

-- 
Regards,

Laurent Pinchart