[PATCH v2] libcamera: base: Remove custom __nodiscard attribute
Mattijs Korpershoek
mkorpershoek at baylibre.com
Wed Jan 8 16:27:49 CET 2025
Hi Kieran,
On mer., janv. 08, 2025 at 15:24, Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
> Quoting Laurent Pinchart (2025-01-06 11:30:40)
>> Hi Mattijs,
>>
>> Thank you for the patch.
>>
>> On Mon, Jan 06, 2025 at 10:40:41AM +0100, Mattijs Korpershoek wrote:
>> > __nodiscard was introduced for compatibility with C++14.
>> > In C++17, there is an official attribute: [[nodiscard]].
>> > Moreover, some libc implementations (like bionic) already define the
>> > __nodiscard macro [1].
>> >
>> > Since:
>> > - libcamera builds with cpp_std=c++17
>> > - [[nodiscard]] is already used in the android HAL (exif)
>> >
>> > We should replace all usage __nodiscard of by [[nodiscard]] for
>> > consistency.
>> >
>> > Do the replacement and remove the no longer used compiler.h.
>> >
>> > [1] https://android-review.googlesource.com/c/platform/bionic/+/3254860
>> >
>> > Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> Seems fine to me.
>
> Three indentation issues that can be fixed while applying. I'll probably
> do that now.
Sorry about the indentation issues, I thought I ran utils/checkstyle.py.
I'm happy if you fixup while applying.
Thanks!
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>>
>> > ---
>> > Hi, it's been a while. I've found a (trivial) build issue when
>> > building against recent bionic versions.
>> >
>> > After discussion in v1, dropping compiler.h seemed the right choice.
>> > ---
>> > Changes in v2:
>> > - Drop support of __nodiscard instead of adding another #ifdef (Laurent)
>> > - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2025-January/047922.html
>> > ---
>> > include/libcamera/base/compiler.h | 14 --------------
>> > include/libcamera/base/meson.build | 1 -
>> > include/libcamera/base/unique_fd.h | 3 +--
>> > include/libcamera/geometry.h | 28 +++++++++++++---------------
>> > 4 files changed, 14 insertions(+), 32 deletions(-)
>> >
>> > diff --git a/include/libcamera/base/compiler.h b/include/libcamera/base/compiler.h
>> > deleted file mode 100644
>> > index fda8fdfdc543f86c5554e38ef790c00d72d60389..0000000000000000000000000000000000000000
>> > --- a/include/libcamera/base/compiler.h
>> > +++ /dev/null
>> > @@ -1,14 +0,0 @@
>> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> > -/*
>> > - * Copyright (C) 2021, Google Inc.
>> > - *
>> > - * Compiler support
>> > - */
>> > -
>> > -#pragma once
>> > -
>> > -#if __cplusplus >= 201703L
>> > -#define __nodiscard [[nodiscard]]
>> > -#else
>> > -#define __nodiscard
>> > -#endif
>> > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
>> > index 2a0cee317204b0d6b44276703fac229bfd7973b9..f28ae4d42a69c755710b51ffc92976bb6fb6a0d8 100644
>> > --- a/include/libcamera/base/meson.build
>> > +++ b/include/libcamera/base/meson.build
>> > @@ -5,7 +5,6 @@ libcamera_base_include_dir = libcamera_include_dir / 'base'
>> > libcamera_base_public_headers = files([
>> > 'bound_method.h',
>> > 'class.h',
>> > - 'compiler.h',
>> > 'flags.h',
>> > 'object.h',
>> > 'shared_fd.h',
>> > diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h
>> > index c9a3b5d0e8628533bd18649e156ba78da40ca200..3066bf28f745df1d26a74c20d21b607dba3d41f4 100644
>> > --- a/include/libcamera/base/unique_fd.h
>> > +++ b/include/libcamera/base/unique_fd.h
>> > @@ -10,7 +10,6 @@
>> > #include <utility>
>> >
>> > #include <libcamera/base/class.h>
>> > -#include <libcamera/base/compiler.h>
>> >
>> > namespace libcamera {
>> >
>> > @@ -43,7 +42,7 @@ public:
>> > return *this;
>> > }
>> >
>> > - __nodiscard int release()
>> > + [[nodiscard]] int release()
>> > {
>> > int fd = fd_;
>> > fd_ = -1;
>> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
>> > index e5f0a843d3144d2086c42c11ab40b0a98438a7e2..0a7133f24c78d4b70075fdd02eabfa52857937dc 100644
>> > --- a/include/libcamera/geometry.h
>> > +++ b/include/libcamera/geometry.h
>> > @@ -11,8 +11,6 @@
>> > #include <ostream>
>> > #include <string>
>> >
>> > -#include <libcamera/base/compiler.h>
>> > -
>> > namespace libcamera {
>> >
>> > class Rectangle;
>> > @@ -110,7 +108,7 @@ public:
>> > return *this;
>> > }
>> >
>> > - __nodiscard constexpr Size alignedDownTo(unsigned int hAlignment,
>> > + [[nodiscard]] constexpr Size alignedDownTo(unsigned int hAlignment,
>> > unsigned int vAlignment) const
>
> Nit for perhaps while applying, this second line is now 2 chars unaligned.
>
>> > {
>> > return {
>> > @@ -119,7 +117,7 @@ public:
>> > };
>> > }
>> >
>> > - __nodiscard constexpr Size alignedUpTo(unsigned int hAlignment,
>> > + [[nodiscard]] constexpr Size alignedUpTo(unsigned int hAlignment,
>> > unsigned int vAlignment) const
>
> Nit for perhaps while applying, this second line is now 2 chars unaligned.
>
>> > {
>> > return {
>> > @@ -128,7 +126,7 @@ public:
>> > };
>> > }
>> >
>> > - __nodiscard constexpr Size boundedTo(const Size &bound) const
>> > + [[nodiscard]] constexpr Size boundedTo(const Size &bound) const
>> > {
>> > return {
>> > std::min(width, bound.width),
>> > @@ -136,7 +134,7 @@ public:
>> > };
>> > }
>> >
>> > - __nodiscard constexpr Size expandedTo(const Size &expand) const
>> > + [[nodiscard]] constexpr Size expandedTo(const Size &expand) const
>> > {
>> > return {
>> > std::max(width, expand.width),
>> > @@ -144,7 +142,7 @@ public:
>> > };
>> > }
>> >
>> > - __nodiscard constexpr Size grownBy(const Size &margins) const
>> > + [[nodiscard]] constexpr Size grownBy(const Size &margins) const
>> > {
>> > return {
>> > width + margins.width,
>> > @@ -152,7 +150,7 @@ public:
>> > };
>> > }
>> >
>> > - __nodiscard constexpr Size shrunkBy(const Size &margins) const
>> > + [[nodiscard]] constexpr Size shrunkBy(const Size &margins) const
>> > {
>> > return {
>> > width > margins.width ? width - margins.width : 0,
>> > @@ -160,10 +158,10 @@ public:
>> > };
>> > }
>> >
>> > - __nodiscard Size boundedToAspectRatio(const Size &ratio) const;
>> > - __nodiscard Size expandedToAspectRatio(const Size &ratio) const;
>> > + [[nodiscard]] Size boundedToAspectRatio(const Size &ratio) const;
>> > + [[nodiscard]] Size expandedToAspectRatio(const Size &ratio) const;
>> >
>> > - __nodiscard Rectangle centeredTo(const Point ¢er) const;
>> > + [[nodiscard]] Rectangle centeredTo(const Point ¢er) const;
>> >
>> > Size operator*(float factor) const;
>> > Size operator/(float factor) const;
>> > @@ -294,11 +292,11 @@ public:
>> > Rectangle &scaleBy(const Size &numerator, const Size &denominator);
>> > Rectangle &translateBy(const Point &point);
>> >
>> > - __nodiscard Rectangle boundedTo(const Rectangle &bound) const;
>> > - __nodiscard Rectangle enclosedIn(const Rectangle &boundary) const;
>> > - __nodiscard Rectangle scaledBy(const Size &numerator,
>> > + [[nodiscard]] Rectangle boundedTo(const Rectangle &bound) const;
>> > + [[nodiscard]] Rectangle enclosedIn(const Rectangle &boundary) const;
>> > + [[nodiscard]] Rectangle scaledBy(const Size &numerator,
>> > const Size &denominator) const;
>
> Nit for perhaps while applying, this second line is now 2 chars unaligned.
>
>> > - __nodiscard Rectangle translatedBy(const Point &point) const;
>> > + [[nodiscard]] Rectangle translatedBy(const Point &point) const;
>> >
>> > Rectangle transformedBetween(const Rectangle &source,
>> > const Rectangle &target) const;
>> >
>> > ---
>> > base-commit: 35ed4b91291d9f3d08e4b51acfb51163e65df8f8
>> > change-id: 20250103-nodiscard-redef-9158e8fdc3f5
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
More information about the libcamera-devel
mailing list