[libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move and copy disablers

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Feb 12, 2021 at 01:30:51PM +0000, Kieran Bingham wrote:
> It can be difficult to correctly parse the syntax for copy/move and the
> associated assignment operators.
> 
> Provide helpers as syntactic sugar to facilitate disabling either the
> copy constructor, and copy assignment operator, and the move constructor
> and move assignment operator in a way which is explicit and clear.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/class.h | 18 +++++++++++++
>  src/libcamera/class.cpp   | 57 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
> index cb278e58204a..920624d8e726 100644
> --- a/include/libcamera/class.h
> +++ b/include/libcamera/class.h
> @@ -11,6 +11,24 @@
>  
>  namespace libcamera {
>  
> +#ifndef __DOXYGEN__
> +#define LIBCAMERA_DISABLE_COPY(klass)  \
> +	klass(const klass &) = delete; \

Perhaps the \ could be aligned with tabs, with the same indentation for
all macros ? We don't have a coding style rule for this, so you can pick
what you like best :-)

> +	klass &operator=(const klass &) = delete;
> +
> +#define LIBCAMERA_DISABLE_MOVE(klass) \
> +	klass(klass &&) = delete;     \
> +	klass &operator=(klass &&) = delete;
> +
> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass) \
> +	LIBCAMERA_DISABLE_COPY(klass)          \
> +	LIBCAMERA_DISABLE_MOVE(klass)
> +#else
> +#define LIBCAMERA_DISABLE_COPY(klass)
> +#define LIBCAMERA_DISABLE_MOVE(klass)
> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass)
> +#endif

You could merge this with the next block to avoid two #ifndef
__DOXYGEN__, up to you.

> +
>  #ifndef __DOXYGEN__
>  #define LIBCAMERA_DECLARE_PRIVATE(klass)				\
>  public:									\
> diff --git a/src/libcamera/class.cpp b/src/libcamera/class.cpp
> index ce230be91e61..c1db14197b4e 100644
> --- a/src/libcamera/class.cpp
> +++ b/src/libcamera/class.cpp
> @@ -17,6 +17,63 @@
>  
>  namespace libcamera {
>  
> +/**
> + * \def LIBCAMERA_DISABLE_COPY
> + * \brief Disable copy construction and assignment of the \a klass
> + * \param klass The name of the class
> + *
> + * Example usage:
> + * \code{.cpp}
> + * class NonCopyable
> + * {
> + * public:
> + * 	NonCopyable();
> + * 	...
> + *
> + * private:
> + * 	LIBCAMERA_DISABLE_COPY(NonCopyable)
> + * };
> + * \endcode
> + */
> +
> +/**
> + * \def LIBCAMERA_DISABLE_MOVE
> + * \brief Disable move construction and assignment of the \a klass
> + * \param klass The name of the class
> + *
> + * Example usage:
> + * \code{.cpp}
> + * class NonMoveable
> + * {
> + * public:
> + * 	NonMoveable();
> + * 	...
> + *
> + * private:
> + * 	LIBCAMERA_DISABLE_MOVE(NonMoveable)
> + * };
> + * \endcode
> + */
> +
> +/**
> + * \def LIBCAMERA_DISABLE_COPY_AND_MOVE
> + * \brief Disable copy and move construction and assignment of the \a klass
> +* \param klass The name of the class

Missing space at the beginning of the line.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + *
> + * Example usage:
> + * \code{.cpp}
> + * class NonCopyableNonMoveable
> + * {
> + * public:
> + * 	NonCopyableNonMoveable();
> + * 	...
> + *
> + * private:
> + * 	LIBCAMERA_DISABLE_COPY_AND_MOVE(NonCopyableNonMoveable)
> + * };
> + * \endcode
> + */
> +
>  /**
>   * \def LIBCAMERA_DECLARE_PRIVATE
>   * \brief Declare private data for a public class

-- 
Regards,

Laurent Pinchart