[libcamera-devel] [PATCH v2 2/6] libcamera: class: Provide move and copy disablers

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Feb 11, 2021 at 01:34:40PM +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 | 12 ++++++++++++
>  src/libcamera/class.cpp   | 18 ++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
> index 2d9b7ebfdb08..5cd31d1b9f37 100644
> --- a/include/libcamera/class.h
> +++ b/include/libcamera/class.h
> @@ -11,6 +11,18 @@
>  
>  namespace libcamera {
>  
> +#define LIBCAMERA_DISABLE_COPY(klass)  \
> +	klass(const klass &) = delete; \
> +	klass &operator=(const klass &) = delete

I'd add a ; here as other similar macros include the last ;.

> +
> +#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)
> +
>  #ifndef __DOXYGEN__
>  #define LIBCAMERA_DECLARE_PRIVATE(klass)				\
>  public:									\
> diff --git a/src/libcamera/class.cpp b/src/libcamera/class.cpp
> index 8a608edb369b..b081d05ec3ac 100644
> --- a/src/libcamera/class.cpp
> +++ b/src/libcamera/class.cpp
> @@ -17,6 +17,24 @@
>  
>  namespace libcamera {
>  
> +/**
> + * \def LIBCAMERA_DISABLE_COPY
> + * \brief Delete the copy constructor and assignment operator.

s/.$// and same below. I'd write "Disable copy construction and
assignment of the \a klass" to match the macro name.

> + * \param klass The identifier of the class to modify

s/identifier/name/

and s/to modify/for which to disable copy/ ?

I'd also add example usage :

 * To disable copy of a class, use the LIBCAMERA_DISABLE_COPY macro in
 * the private section of the of the class declaration:
 *
 * \code{.cpp}
 * class NonCopyable
 * {
 * public:
 * 	NonCopyable();
 * 	...
 *
 * private:
 * 	LIBCAMERA_DISABLE_COPY(NonCopyable)
 * };
 * \encode

Same below.

> + */
> +
> +/**
> + * \def LIBCAMERA_DISABLE_MOVE
> + * \brief Delete the move construtor and assignment operator.
> + * \param klass The identifier of the class to modify
> + */
> +
> +/**
> + * \def LIBCAMERA_DISABLE_COPY_AND_MOVE
> + * \brief Delete all copy and move constructors, and assignment operators.
> + * \param klass The identifier of the class to modify
> + */
> +
>  /**
>   * \def LIBCAMERA_DECLARE_PRIVATE
>   * \brief Declare private data for a public class

-- 
Regards,

Laurent Pinchart