[libcamera-devel,1/5] libcamera: Provide class.h

10199 diff mbox series

Provide a public header to define helpers for class definitions.
This initially implements macros to clearly define the deletion of
copy/move/assignment constructors/operators for classes to restrict
their capabilities explicitly.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/class.h     | 35 +++++++++++++++++++++++++++++++++++
 include/libcamera/meson.build |  1 +
 2 files changed, 36 insertions(+)
 create mode 100644 include/libcamera/class.h

Hi Kieran,

Thank you for the patch.

On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:
> Provide a public header to define helpers for class definitions.
> This initially implements macros to clearly define the deletion of
> copy/move/assignment constructors/operators for classes to restrict
> their capabilities explicitly.

Thanks for picking this up after my unsuccessful experimented with a
non-copyable base class. There's a few bikesheeding comments beow, but
idea looks good.

Have you considered including this in a more generic header, such as
utils.h (or global.h, or <bikeshedding>.h) ? I don't foresee lots of new
features to be added to class.h, while I think we'll have more
miscellaneous macros and functions moving forward. It would be nice to
avoid creating micro-headers.

> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  include/libcamera/class.h     | 35 +++++++++++++++++++++++++++++++++++
>  include/libcamera/meson.build |  1 +
>  2 files changed, 36 insertions(+)
>  create mode 100644 include/libcamera/class.h
> 
> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
> new file mode 100644
> index 000000000000..adcfa8860957
> --- /dev/null
> +++ b/include/libcamera/class.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * class.h - class helpers
> + */
> +#ifndef __LIBCAMERA_CLASS_H__
> +#define __LIBCAMERA_CLASS_H__
> +
> +/**
> + * \brief Delete the copy constructor and assignment operator.

Missing \param ?

> + */
> +#define DELETE_COPY_AND_ASSIGN(klass)   \

Macros are unfortunately not part of namespaces, so to avoid a risk of
collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting
too long, I think you can drop the _AND_ASSIGN suffix, as the macro is
disabling copying as a whole. Otherwise it should be
DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)

Another bikeshedding topic, I would have gone for DISABLE instead of
DELETE to emphasize the purpose of the macro instead of how it's
implemented (not that libcamera has to care about this, but in older C++
versions we would have needed to declare the functions as private as
there was no = delete).

> +	/* copy constructor. */         \

I think you can drop the comments, it's quite explicit already.

> +	klass(const klass &) = delete;  \
> +	/* copy assignment operator. */ \
> +	klass &operator=(const klass &) = delete
> +
> +/**
> + * \brief Delete the move construtor and assignment operator.
> + */
> +#define DELETE_MOVE_AND_ASSIGN(klass)   \
> +	/* move constructor. */         \
> +	klass(klass &&) = delete;       \
> +	/* move assignment operator. */ \
> +	klass &operator=(klass &&) = delete
> +
> +/**
> + * \brief Delete all copy and move constructors, and assignment operators.
> + */
> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \
> +	DELETE_COPY_AND_ASSIGN(klass);     \
> +	DELETE_MOVE_AND_ASSIGN(klass)
> +
> +#endif /* __LIBCAMERA_CLASS_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 3d5fc70134ad..b3363a6f735b 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_public_headers = files([
>      'buffer.h',
>      'camera.h',
>      'camera_manager.h',
> +    'class.h',
>      'controls.h',
>      'event_dispatcher.h',
>      'event_notifier.h',

Hi Laurent,

On 23/10/2020 05:23, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:
>> Provide a public header to define helpers for class definitions.
>> This initially implements macros to clearly define the deletion of
>> copy/move/assignment constructors/operators for classes to restrict
>> their capabilities explicitly.
> 
> Thanks for picking this up after my unsuccessful experimented with a
> non-copyable base class. There's a few bikesheeding comments beow, but
> idea looks good.
> 
> Have you considered including this in a more generic header, such as
> utils.h (or global.h, or <bikeshedding>.h) ? I don't foresee lots of new
> features to be added to class.h, while I think we'll have more
> miscellaneous macros and functions moving forward. It would be nice to
> avoid creating micro-headers.

That's why I chose class.h ...

I thought this would cover more than just these macros.

Throwing your argument back at you, You've recently posted a patch which
adds 'extensible.h' ... and both these, and your extensible concepts are
attributed to 'class' helpers, (yet these could not go in to
extensible.h) ...

So ... Would you prefer to put your extensible types in class.h, or
global.h?



We have utils.h of course, but that's in internal/ and this needs to be
in public header space.

Of course we could also have a public utils.h ... but I'm a bit opposed
to having lots of duplicated header names in both public and private
space, as it gets confusing as to which one is being used.


> 
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  include/libcamera/class.h     | 35 +++++++++++++++++++++++++++++++++++
>>  include/libcamera/meson.build |  1 +
>>  2 files changed, 36 insertions(+)
>>  create mode 100644 include/libcamera/class.h
>>
>> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
>> new file mode 100644
>> index 000000000000..adcfa8860957
>> --- /dev/null
>> +++ b/include/libcamera/class.h
>> @@ -0,0 +1,35 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2020, Google Inc.
>> + *
>> + * class.h - class helpers
>> + */
>> +#ifndef __LIBCAMERA_CLASS_H__
>> +#define __LIBCAMERA_CLASS_H__
>> +
>> +/**
>> + * \brief Delete the copy constructor and assignment operator.
> 
> Missing \param ?
> 
>> + */
>> +#define DELETE_COPY_AND_ASSIGN(klass)   \
> 
> Macros are unfortunately not part of namespaces, so to avoid a risk of
> collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting
> too long, I think you can drop the _AND_ASSIGN suffix, as the macro is
> disabling copying as a whole. Otherwise it should be
> DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)
> 
> Another bikeshedding topic, I would have gone for DISABLE instead of
> DELETE to emphasize the purpose of the macro instead of how it's
> implemented (not that libcamera has to care about this, but in older C++
> versions we would have needed to declare the functions as private as
> there was no = delete).

That's why I chose delete, to express explicitly that this is 'deleting'
the functions.

The similar 'google' macros are called 'DISALLOW_'...

In fact, somewhat frustratingly, the chromium style guides deprecated
the equivalent macros:

https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md

And instead prefer to code them directly. But I disagree, as we've seen
this can lead to errors, both in not deleting both the constructor and
assignment operator, or in getting the names wrong, and quite honestly I
find them really hard to interpret, just from looking at them. (More below).

> 
>> +	/* copy constructor. */         \
> 
> I think you can drop the comments, it's quite explicit already.

The reason I've put these here, is because I find it really hard to look
at these and say "Oh - that's the copy constructor", (as opposed to the
move constructor), and "That's the copy assignment operator", as opposed
to the move assignment operator.

Lets consider it like the C++ equivalent of English Homophones. Things
that look/read/sound similar but have a different meaning.

i.e. : If I hadn't looked at this in more than one day, I would have to
look up which of these does which :
	klass(klass &&) = delete;
	klass(const klass &) = delete;

and equally for the operators:
	klass &operator=(const klass &) = delete
	klass &operator=(klass &&) = delete

And in code - I don't want the meaning of the code to be easy to
mis-interpret.


> 
>> +	klass(const klass &) = delete;  \
>> +	/* copy assignment operator. */ \
>> +	klass &operator=(const klass &) = delete
>> +
>> +/**
>> + * \brief Delete the move construtor and assignment operator.
>> + */
>> +#define DELETE_MOVE_AND_ASSIGN(klass)   \
>> +	/* move constructor. */         \
>> +	klass(klass &&) = delete;       \
>> +	/* move assignment operator. */ \
>> +	klass &operator=(klass &&) = delete
>> +
>> +/**
>> + * \brief Delete all copy and move constructors, and assignment operators.
>> + */
>> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \
>> +	DELETE_COPY_AND_ASSIGN(klass);     \
>> +	DELETE_MOVE_AND_ASSIGN(klass)
>> +
>> +#endif /* __LIBCAMERA_CLASS_H__ */
>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>> index 3d5fc70134ad..b3363a6f735b 100644
>> --- a/include/libcamera/meson.build
>> +++ b/include/libcamera/meson.build
>> @@ -5,6 +5,7 @@ libcamera_public_headers = files([
>>      'buffer.h',
>>      'camera.h',
>>      'camera_manager.h',
>> +    'class.h',
>>      'controls.h',
>>      'event_dispatcher.h',
>>      'event_notifier.h',
>

Hi Kieran,

On Fri, Oct 23, 2020 at 09:16:55AM +0100, Kieran Bingham wrote:
> On 23/10/2020 05:23, Laurent Pinchart wrote:
> > On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:
> >> Provide a public header to define helpers for class definitions.
> >> This initially implements macros to clearly define the deletion of
> >> copy/move/assignment constructors/operators for classes to restrict
> >> their capabilities explicitly.
> > 
> > Thanks for picking this up after my unsuccessful experimented with a
> > non-copyable base class. There's a few bikesheeding comments beow, but
> > idea looks good.
> > 
> > Have you considered including this in a more generic header, such as
> > utils.h (or global.h, or <bikeshedding>.h) ? I don't foresee lots of new
> > features to be added to class.h, while I think we'll have more
> > miscellaneous macros and functions moving forward. It would be nice to
> > avoid creating micro-headers.
> 
> That's why I chose class.h ...
> 
> I thought this would cover more than just these macros.
> 
> Throwing your argument back at you, You've recently posted a patch which
> adds 'extensible.h' ... and both these, and your extensible concepts are
> attributed to 'class' helpers, (yet these could not go in to
> extensible.h) ...
> 
> So ... Would you prefer to put your extensible types in class.h, or
> global.h?
> 
> We have utils.h of course, but that's in internal/ and this needs to be
> in public header space.
>
> Of course we could also have a public utils.h ... but I'm a bit opposed
> to having lots of duplicated header names in both public and private
> space, as it gets confusing as to which one is being used.

The move of the private headers to an internal/ directory was meant to
allow public and private headers with the same name, but it of course
doesn't mean we have to (ab)use that feature.

Merging these macros and the Extensible class in a single header makes
sense to me. That broadens the scope of class.h a bit so we could go
with that. I still feel we'll have a need for miscellaneous public
"things" in the future which wouldn't be a good match for class.h, but
we could address that problem at that time.

> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> ---
> >>  include/libcamera/class.h     | 35 +++++++++++++++++++++++++++++++++++
> >>  include/libcamera/meson.build |  1 +
> >>  2 files changed, 36 insertions(+)
> >>  create mode 100644 include/libcamera/class.h
> >>
> >> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
> >> new file mode 100644
> >> index 000000000000..adcfa8860957
> >> --- /dev/null
> >> +++ b/include/libcamera/class.h
> >> @@ -0,0 +1,35 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2020, Google Inc.
> >> + *
> >> + * class.h - class helpers
> >> + */
> >> +#ifndef __LIBCAMERA_CLASS_H__
> >> +#define __LIBCAMERA_CLASS_H__
> >> +
> >> +/**
> >> + * \brief Delete the copy constructor and assignment operator.
> > 
> > Missing \param ?
> > 
> >> + */
> >> +#define DELETE_COPY_AND_ASSIGN(klass)   \
> > 
> > Macros are unfortunately not part of namespaces, so to avoid a risk of
> > collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting
> > too long, I think you can drop the _AND_ASSIGN suffix, as the macro is
> > disabling copying as a whole. Otherwise it should be
> > DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)
> > 
> > Another bikeshedding topic, I would have gone for DISABLE instead of
> > DELETE to emphasize the purpose of the macro instead of how it's
> > implemented (not that libcamera has to care about this, but in older C++
> > versions we would have needed to declare the functions as private as
> > there was no = delete).
> 
> That's why I chose delete, to express explicitly that this is 'deleting'
> the functions.
> 
> The similar 'google' macros are called 'DISALLOW_'...

And one more data point,
https://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY :-)

I prefer expressing the purpose in the name rather than the
implementation details. That arguments makes more sense for Qt that has
to support multiple C++ versions, including older versions where =
delete wasn't available.

If you prefer DELETE over DISABLE I won't fight hard for it (we know we
want to avoid DISALLOW as that's what Google deprecates, but DISABLE or
DELETE are not deprecated, right ? ;-)). I feel a bit stronger about
writing 'COPY' instead of 'COPY_AND_ASSIGN', as the copy concept is
really the combination of a constructor and an assignment operator (same
for move).

> In fact, somewhat frustratingly, the chromium style guides deprecated
> the equivalent macros:
> 
> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md
> 
> And instead prefer to code them directly. But I disagree, as we've seen
> this can lead to errors, both in not deleting both the constructor and
> assignment operator, or in getting the names wrong, and quite honestly I
> find them really hard to interpret, just from looking at them. (More below).

I agree with you, I think the macros make sense. We wouldn't be having
this conversation if just deleting the correct functions wasn't
error-prone.

> > 
> >> +	/* copy constructor. */         \
> > 
> > I think you can drop the comments, it's quite explicit already.
> 
> The reason I've put these here, is because I find it really hard to look
> at these and say "Oh - that's the copy constructor", (as opposed to the
> move constructor), and "That's the copy assignment operator", as opposed
> to the move assignment operator.
> 
> Lets consider it like the C++ equivalent of English Homophones. Things
> that look/read/sound similar but have a different meaning.
> 
> i.e. : If I hadn't looked at this in more than one day, I would have to
> look up which of these does which :
> 	klass(klass &&) = delete;
> 	klass(const klass &) = delete;
> 
> and equally for the operators:
> 	klass &operator=(const klass &) = delete
> 	klass &operator=(klass &&) = delete
> 
> And in code - I don't want the meaning of the code to be easy to
> mis-interpret.

Maybe it's a matter of getting used to it :-) My comment was more about
the fact that telling the constructor and assignment operator apart is
fairly straightforward (at least more than telling the copy and move
versions apart at a quick glance), and which pair you delete is part of
the macro name. That's why I didn't consider the comments to add much,
but if you think they help, I don't mind (we usually avoid comments in
headers though, but there are exceptions where it makes sense).

> >> +	klass(const klass &) = delete;  \
> >> +	/* copy assignment operator. */ \
> >> +	klass &operator=(const klass &) = delete
> >> +
> >> +/**
> >> + * \brief Delete the move construtor and assignment operator.
> >> + */
> >> +#define DELETE_MOVE_AND_ASSIGN(klass)   \
> >> +	/* move constructor. */         \
> >> +	klass(klass &&) = delete;       \
> >> +	/* move assignment operator. */ \
> >> +	klass &operator=(klass &&) = delete
> >> +
> >> +/**
> >> + * \brief Delete all copy and move constructors, and assignment operators.
> >> + */
> >> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \
> >> +	DELETE_COPY_AND_ASSIGN(klass);     \
> >> +	DELETE_MOVE_AND_ASSIGN(klass)
> >> +
> >> +#endif /* __LIBCAMERA_CLASS_H__ */
> >> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> >> index 3d5fc70134ad..b3363a6f735b 100644
> >> --- a/include/libcamera/meson.build
> >> +++ b/include/libcamera/meson.build
> >> @@ -5,6 +5,7 @@ libcamera_public_headers = files([
> >>      'buffer.h',
> >>      'camera.h',
> >>      'camera_manager.h',
> >> +    'class.h',
> >>      'controls.h',
> >>      'event_dispatcher.h',
> >>      'event_notifier.h',

Hi Laurent,

On 23/10/2020 21:11, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Fri, Oct 23, 2020 at 09:16:55AM +0100, Kieran Bingham wrote:
>> On 23/10/2020 05:23, Laurent Pinchart wrote:
>>> On Thu, Oct 22, 2020 at 02:56:01PM +0100, Kieran Bingham wrote:
>>>> Provide a public header to define helpers for class definitions.
>>>> This initially implements macros to clearly define the deletion of
>>>> copy/move/assignment constructors/operators for classes to restrict
>>>> their capabilities explicitly.
>>>
>>> Thanks for picking this up after my unsuccessful experimented with a
>>> non-copyable base class. There's a few bikesheeding comments beow, but
>>> idea looks good.
>>>
>>> Have you considered including this in a more generic header, such as
>>> utils.h (or global.h, or <bikeshedding>.h) ? I don't foresee lots of new
>>> features to be added to class.h, while I think we'll have more
>>> miscellaneous macros and functions moving forward. It would be nice to
>>> avoid creating micro-headers.
>>
>> That's why I chose class.h ...
>>
>> I thought this would cover more than just these macros.
>>
>> Throwing your argument back at you, You've recently posted a patch which
>> adds 'extensible.h' ... and both these, and your extensible concepts are
>> attributed to 'class' helpers, (yet these could not go in to
>> extensible.h) ...
>>
>> So ... Would you prefer to put your extensible types in class.h, or
>> global.h?
>>
>> We have utils.h of course, but that's in internal/ and this needs to be
>> in public header space.
>>
>> Of course we could also have a public utils.h ... but I'm a bit opposed
>> to having lots of duplicated header names in both public and private
>> space, as it gets confusing as to which one is being used.
> 
> The move of the private headers to an internal/ directory was meant to
> allow public and private headers with the same name, but it of course
> doesn't mean we have to (ab)use that feature.
> 
> Merging these macros and the Extensible class in a single header makes
> sense to me. That broadens the scope of class.h a bit so we could go
> with that. I still feel we'll have a need for miscellaneous public
> "things" in the future which wouldn't be a good match for class.h, but
> we could address that problem at that time.

Now that you have merged the Extensible.h header, do you have any
preference for where these helpers would live?


>>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>>> ---
>>>>  include/libcamera/class.h     | 35 +++++++++++++++++++++++++++++++++++
>>>>  include/libcamera/meson.build |  1 +
>>>>  2 files changed, 36 insertions(+)
>>>>  create mode 100644 include/libcamera/class.h
>>>>
>>>> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
>>>> new file mode 100644
>>>> index 000000000000..adcfa8860957
>>>> --- /dev/null
>>>> +++ b/include/libcamera/class.h
>>>> @@ -0,0 +1,35 @@
>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>> +/*
>>>> + * Copyright (C) 2020, Google Inc.
>>>> + *
>>>> + * class.h - class helpers
>>>> + */
>>>> +#ifndef __LIBCAMERA_CLASS_H__
>>>> +#define __LIBCAMERA_CLASS_H__
>>>> +
>>>> +/**
>>>> + * \brief Delete the copy constructor and assignment operator.
>>>
>>> Missing \param ?
>>>

How about:

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


>>>> + */
>>>> +#define DELETE_COPY_AND_ASSIGN(klass)   \
>>>
>>> Macros are unfortunately not part of namespaces, so to avoid a risk of
>>> collision, I'd use a LIBCAMERA_ prefix. And to avoid the name getting
>>> too long, I think you can drop the _AND_ASSIGN suffix, as the macro is
>>> disabling copying as a whole. Otherwise it should be
>>> DELETE_COPY_CONSTRUCT_AND_ASSIGN :-)

LIBCAMERA_ feels a bit extraneous to me ... but I can add it.
(We could also wrap it in a #ifndef DELETE_COPY_xxxxx)

DELETE_COPY_CONSTRUCT_AND_ASSIGN is really quite long...
and
LIBCAMERA_DELETE_COPY_CONSTRUCT_AND_ASSIGN
even more so.

>>>
>>> Another bikeshedding topic, I would have gone for DISABLE instead of
>>> DELETE to emphasize the purpose of the macro instead of how it's
>>> implemented (not that libcamera has to care about this, but in older C++
>>> versions we would have needed to declare the functions as private as
>>> there was no = delete).
>>
>> That's why I chose delete, to express explicitly that this is 'deleting'
>> the functions.
>>
>> The similar 'google' macros are called 'DISALLOW_'...
> 
> And one more data point,
> https://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY :-)
> 
> I prefer expressing the purpose in the name rather than the
> implementation details. That arguments makes more sense for Qt that has
> to support multiple C++ versions, including older versions where =
> delete wasn't available.

I still prefer delete ...

These macros are providing syntactic sugar to make sure 'what' is being
deleted is explicit (in human readable terms).

And I don't believe we'll be supporting a C++ version which can't handle
= delete ?


> If you prefer DELETE over DISABLE I won't fight hard for it (we know we
> want to avoid DISALLOW as that's what Google deprecates, but DISABLE or
> DELETE are not deprecated, right ? ;-)). I feel a bit stronger about
> writing 'COPY' instead of 'COPY_AND_ASSIGN', as the copy concept is
> really the combination of a constructor and an assignment operator (same
> for move).

Tell me what colour you'd like this and I'll buy the paint.

Would you like to just copy QT and go with

LIBCAMERA_DISABLE_COPY
LIBCAMERA_DISABLE_MOVE?


>> In fact, somewhat frustratingly, the chromium style guides deprecated
>> the equivalent macros:
>>
>> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md
>>
>> And instead prefer to code them directly. But I disagree, as we've seen
>> this can lead to errors, both in not deleting both the constructor and
>> assignment operator, or in getting the names wrong, and quite honestly I
>> find them really hard to interpret, just from looking at them. (More below).
> 
> I agree with you, I think the macros make sense. We wouldn't be having
> this conversation if just deleting the correct functions wasn't
> error-prone.
> 
>>>
>>>> +	/* copy constructor. */         \
>>>
>>> I think you can drop the comments, it's quite explicit already.
>>
>> The reason I've put these here, is because I find it really hard to look
>> at these and say "Oh - that's the copy constructor", (as opposed to the
>> move constructor), and "That's the copy assignment operator", as opposed
>> to the move assignment operator.
>>
>> Lets consider it like the C++ equivalent of English Homophones. Things
>> that look/read/sound similar but have a different meaning.
>>
>> i.e. : If I hadn't looked at this in more than one day, I would have to
>> look up which of these does which :
>> 	klass(klass &&) = delete;
>> 	klass(const klass &) = delete;


Wow - coming back at this, and I really have to look up which version of
each does what. Which only solidifies in my head that these would be
much better with names.


>>
>> and equally for the operators:
>> 	klass &operator=(const klass &) = delete
>> 	klass &operator=(klass &&) = delete
>>
>> And in code - I don't want the meaning of the code to be easy to
>> mis-interpret.
> 
> Maybe it's a matter of getting used to it :-) My comment was more about

Perhaps someone who writes move/copy constructors everyday would easily
remember the declarations and it would become as obvious as reading a
for-loop... but they're a low churn code type, and often copied as a
template anyway.


> the fact that telling the constructor and assignment operator apart is
> fairly straightforward (at least more than telling the copy and move
> versions apart at a quick glance), and which pair you delete is part of
> the macro name. That's why I didn't consider the comments to add much,
> but if you think they help, I don't mind (we usually avoid comments in
> headers though, but there are exceptions where it makes sense).
> 
>>>> +	klass(const klass &) = delete;  \
>>>> +	/* copy assignment operator. */ \
>>>> +	klass &operator=(const klass &) = delete
>>>> +
>>>> +/**
>>>> + * \brief Delete the move construtor and assignment operator.
>>>> + */
>>>> +#define DELETE_MOVE_AND_ASSIGN(klass)   \
>>>> +	/* move constructor. */         \
>>>> +	klass(klass &&) = delete;       \
>>>> +	/* move assignment operator. */ \
>>>> +	klass &operator=(klass &&) = delete
>>>> +
>>>> +/**
>>>> + * \brief Delete all copy and move constructors, and assignment operators.
>>>> + */
>>>> +#define DELETE_COPY_MOVE_AND_ASSIGN(klass) \
>>>> +	DELETE_COPY_AND_ASSIGN(klass);     \
>>>> +	DELETE_MOVE_AND_ASSIGN(klass)
>>>> +
>>>> +#endif /* __LIBCAMERA_CLASS_H__ */
>>>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>>>> index 3d5fc70134ad..b3363a6f735b 100644
>>>> --- a/include/libcamera/meson.build
>>>> +++ b/include/libcamera/meson.build
>>>> @@ -5,6 +5,7 @@ libcamera_public_headers = files([
>>>>      'buffer.h',
>>>>      'camera.h',
>>>>      'camera_manager.h',
>>>> +    'class.h',
>>>>      'controls.h',
>>>>      'event_dispatcher.h',
>>>>      'event_notifier.h',
>