[libcamera-devel,1/2] libcamera: Add macro to conditionally use [[nodiscard]]

11128 diff mbox series

The [[nodiscard]] attribute has been added to C++17. It can thus be used
inside libcamera, but would prevent applications compiled for C++14 to
use libcamera if the attribute was used in public headers.

To offer this feature when the application is compiled with a
recent-enough C++ version, as well as for compiling libcamera itself,
add a __nodiscard macro that expands as [[nodiscard]] when using C++17
or newer.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/compiler.h  | 16 ++++++++++++++++
 include/libcamera/meson.build |  1 +
 2 files changed, 17 insertions(+)
 create mode 100644 include/libcamera/compiler.h

Hi Laurent,

Thanks for your work.

On 2021-02-04 05:59:50 +0200, Laurent Pinchart wrote:
> The [[nodiscard]] attribute has been added to C++17. It can thus be used
> inside libcamera, but would prevent applications compiled for C++14 to
> use libcamera if the attribute was used in public headers.
> 
> To offer this feature when the application is compiled with a
> recent-enough C++ version, as well as for compiling libcamera itself,
> add a __nodiscard macro that expands as [[nodiscard]] when using C++17
> or newer.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/compiler.h  | 16 ++++++++++++++++
>  include/libcamera/meson.build |  1 +
>  2 files changed, 17 insertions(+)
>  create mode 100644 include/libcamera/compiler.h
> 
> diff --git a/include/libcamera/compiler.h b/include/libcamera/compiler.h
> new file mode 100644
> index 000000000000..dc56dbb8b792
> --- /dev/null
> +++ b/include/libcamera/compiler.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * compiler.h - Compiler support
> + */
> +#ifndef __LIBCAMERA_COMPILER_H__
> +#define __LIBCAMERA_COMPILER_H__
> +
> +#if __cplusplus >= 201703L
> +#define __nodiscard		[[nodiscard]]
> +#else
> +#define __nodiscard
> +#endif
> +
> +#endif /* __LIBCAMERA_COMPILER_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index cf2935f1ee95..13e9eeb6d6ad 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',
> +    'compiler.h',
>      'controls.h',
>      'extensible.h',
>      'file_descriptor.h',
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Laurent,

On 04/02/2021 03:59, Laurent Pinchart wrote:
> The [[nodiscard]] attribute has been added to C++17. It can thus be used
> inside libcamera, but would prevent applications compiled for C++14 to
> use libcamera if the attribute was used in public headers.
> 
> To offer this feature when the application is compiled with a
> recent-enough C++ version, as well as for compiling libcamera itself,
> add a __nodiscard macro that expands as [[nodiscard]] when using C++17
> or newer.
> 

Now that we have this, I'd be tempted to also use this style for the
[[maybe_unused]], and perhaps the [[fallthrough]].

I bet that's another bikeshedding rabbit-hole though.

But for this

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/compiler.h  | 16 ++++++++++++++++
>  include/libcamera/meson.build |  1 +
>  2 files changed, 17 insertions(+)
>  create mode 100644 include/libcamera/compiler.h
> 
> diff --git a/include/libcamera/compiler.h b/include/libcamera/compiler.h
> new file mode 100644
> index 000000000000..dc56dbb8b792
> --- /dev/null
> +++ b/include/libcamera/compiler.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * compiler.h - Compiler support
> + */
> +#ifndef __LIBCAMERA_COMPILER_H__
> +#define __LIBCAMERA_COMPILER_H__
> +
> +#if __cplusplus >= 201703L
> +#define __nodiscard		[[nodiscard]]
> +#else
> +#define __nodiscard
> +#endif
> +
> +#endif /* __LIBCAMERA_COMPILER_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index cf2935f1ee95..13e9eeb6d6ad 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',
> +    'compiler.h',
>      'controls.h',
>      'extensible.h',
>      'file_descriptor.h',
>

Hi Kieran,

On Thu, Feb 04, 2021 at 02:28:50PM +0000, Kieran Bingham wrote:
> On 04/02/2021 03:59, Laurent Pinchart wrote:
> > The [[nodiscard]] attribute has been added to C++17. It can thus be used
> > inside libcamera, but would prevent applications compiled for C++14 to
> > use libcamera if the attribute was used in public headers.
> > 
> > To offer this feature when the application is compiled with a
> > recent-enough C++ version, as well as for compiling libcamera itself,
> > add a __nodiscard macro that expands as [[nodiscard]] when using C++17
> > or newer.
> 
> Now that we have this, I'd be tempted to also use this style for the
> [[maybe_unused]], and perhaps the [[fallthrough]].
> 
> I bet that's another bikeshedding rabbit-hole though.

I was envisioning usage of this macro for the public headers only. We
can discuss using it, and adding new macros, for internal code as well,
but we don't have the same issue there of having to build with C++14.
The C++ [[...]] attribute syntax takes a bit of time to get used to, but
is it that bad after the initial phase ? :-)

> But for this
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/compiler.h  | 16 ++++++++++++++++
> >  include/libcamera/meson.build |  1 +
> >  2 files changed, 17 insertions(+)
> >  create mode 100644 include/libcamera/compiler.h
> > 
> > diff --git a/include/libcamera/compiler.h b/include/libcamera/compiler.h
> > new file mode 100644
> > index 000000000000..dc56dbb8b792
> > --- /dev/null
> > +++ b/include/libcamera/compiler.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Google Inc.
> > + *
> > + * compiler.h - Compiler support
> > + */
> > +#ifndef __LIBCAMERA_COMPILER_H__
> > +#define __LIBCAMERA_COMPILER_H__
> > +
> > +#if __cplusplus >= 201703L
> > +#define __nodiscard		[[nodiscard]]
> > +#else
> > +#define __nodiscard
> > +#endif
> > +
> > +#endif /* __LIBCAMERA_COMPILER_H__ */
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index cf2935f1ee95..13e9eeb6d6ad 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',
> > +    'compiler.h',
> >      'controls.h',
> >      'extensible.h',
> >      'file_descriptor.h',