[{"id":32935,"web_url":"https://patchwork.libcamera.org/comment/32935/","msgid":"<20250106113040.GD10308@pendragon.ideasonboard.com>","date":"2025-01-06T11:30:40","subject":"Re: [PATCH v2] libcamera: base: Remove custom __nodiscard attribute","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Mattijs,\n\nThank you for the patch.\n\nOn Mon, Jan 06, 2025 at 10:40:41AM +0100, Mattijs Korpershoek wrote:\n> __nodiscard was introduced for compatibility with C++14.\n> In C++17, there is an official attribute: [[nodiscard]].\n> Moreover, some libc implementations (like bionic) already define the\n> __nodiscard macro [1].\n> \n> Since:\n> - libcamera builds with cpp_std=c++17\n> - [[nodiscard]] is already used in the android HAL (exif)\n> \n> We should replace all usage __nodiscard of by [[nodiscard]] for\n> consistency.\n> \n> Do the replacement and remove the no longer used compiler.h.\n> \n> [1] https://android-review.googlesource.com/c/platform/bionic/+/3254860\n> \n> Signed-off-by: Mattijs Korpershoek \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> Hi, it's been a while. I've found a (trivial) build issue when\n> building against recent bionic versions.\n> \n> After discussion in v1, dropping compiler.h seemed the right choice.\n> ---\n> Changes in v2:\n> - Drop support of __nodiscard instead of adding another #ifdef (Laurent)\n> - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2025-January/047922.html\n> ---\n> include/libcamera/base/compiler.h | 14 --------------\n> include/libcamera/base/meson.build | 1 -\n> include/libcamera/base/unique_fd.h | 3 +--\n> include/libcamera/geometry.h | 28 +++++++++++++---------------\n> 4 files changed, 14 insertions(+), 32 deletions(-)\n> \n> diff --git a/include/libcamera/base/compiler.h b/include/libcamera/base/compiler.h\n> deleted file mode 100644\n> index fda8fdfdc543f86c5554e38ef790c00d72d60389..0000000000000000000000000000000000000000\n> --- a/include/libcamera/base/compiler.h\n> +++ /dev/null\n> @@ -1,14 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2021, Google Inc.\n> - *\n> - * Compiler support\n> - */\n> -\n> -#pragma once\n> -\n> -#if __cplusplus >= 201703L\n> -#define __nodiscard\t\t[[nodiscard]]\n> -#else\n> -#define __nodiscard\n> -#endif\n> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> index 2a0cee317204b0d6b44276703fac229bfd7973b9..f28ae4d42a69c755710b51ffc92976bb6fb6a0d8 100644\n> --- a/include/libcamera/base/meson.build\n> +++ b/include/libcamera/base/meson.build\n> @@ -5,7 +5,6 @@ libcamera_base_include_dir = libcamera_include_dir / 'base'\n> libcamera_base_public_headers = files([\n> 'bound_method.h',\n> 'class.h',\n> - 'compiler.h',\n> 'flags.h',\n> 'object.h',\n> 'shared_fd.h',\n> diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h\n> index c9a3b5d0e8628533bd18649e156ba78da40ca200..3066bf28f745df1d26a74c20d21b607dba3d41f4 100644\n> --- a/include/libcamera/base/unique_fd.h\n> +++ b/include/libcamera/base/unique_fd.h\n> @@ -10,7 +10,6 @@\n> #include \n> \n> #include \n> -#include \n> \n> namespace libcamera {\n> \n> @@ -43,7 +42,7 @@ public:\n> \t\treturn *this;\n> \t}\n> \n> -\t__nodiscard int release()\n> +\t[[nodiscard]] int release()\n> \t{\n> \t\tint fd = fd_;\n> \t\tfd_ = -1;\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index e5f0a843d3144d2086c42c11ab40b0a98438a7e2..0a7133f24c78d4b70075fdd02eabfa52857937dc 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -11,8 +11,6 @@\n> #include \n> #include \n> \n> -#include \n> -\n> namespace libcamera {\n> \n> class Rectangle;\n> @@ -110,7 +108,7 @@ public:\n> \t\treturn *this;\n> \t}\n> \n> -\t__nodiscard constexpr Size alignedDownTo(unsigned int hAlignment,\n> +\t[[nodiscard]] constexpr Size alignedDownTo(unsigned int hAlignment,\n> \t\t\t\t\t\t unsigned int vAlignment) const\n> \t{\n> \t\treturn {\n> @@ -119,7 +117,7 @@ public:\n> \t\t};\n> \t}\n> \n> -\t__nodiscard constexpr Size alignedUpTo(unsigned int hAlignment,\n> +\t[[nodiscard]] constexpr Size alignedUpTo(unsigned int hAlignment,\n> \t\t\t\t\t unsigned int vAlignment) const\n> \t{\n> \t\treturn {\n> @@ -128,7 +126,7 @@ public:\n> \t\t};\n> \t}\n> \n> -\t__nodiscard constexpr Size boundedTo(const Size &bound) const\n> +\t[[nodiscard]] constexpr Size boundedTo(const Size &bound) const\n> \t{\n> \t\treturn {\n> \t\t\tstd::min(width, bound.width),\n> @@ -136,7 +134,7 @@ public:\n> \t\t};\n> \t}\n> \n> -\t__nodiscard constexpr Size expandedTo(const Size &expand) const\n> +\t[[nodiscard]] constexpr Size expandedTo(const Size &expand) const\n> \t{\n> \t\treturn {\n> \t\t\tstd::max(width, expand.width),\n> @@ -144,7 +142,7 @@ public:\n> \t\t};\n> \t}\n> \n> -\t__nodiscard constexpr Size grownBy(const Size &margins) const\n> +\t[[nodiscard]] constexpr Size grownBy(const Size &margins) const\n> \t{\n> \t\treturn {\n> \t\t\twidth + margins.width,\n> @@ -152,7 +150,7 @@ public:\n> \t\t};\n> \t}\n> \n> -\t__nodiscard constexpr Size shrunkBy(const Size &margins) const\n> +\t[[nodiscard]] constexpr Size shrunkBy(const Size &margins) const\n> \t{\n> \t\treturn {\n> \t\t\twidth > margins.width ? width - margins.width : 0,\n> @@ -160,10 +158,10 @@ public:\n> \t\t};\n> \t}\n> \n> -\t__nodiscard Size boundedToAspectRatio(const Size &ratio) const;\n> -\t__nodiscard Size expandedToAspectRatio(const Size &ratio) const;\n> +\t[[nodiscard]] Size boundedToAspectRatio(const Size &ratio) const;\n> +\t[[nodiscard]] Size expandedToAspectRatio(const Size &ratio) const;\n> \n> -\t__nodiscard Rectangle centeredTo(const Point ¢er) const;\n> +\t[[nodiscard]] Rectangle centeredTo(const Point ¢er) const;\n> \n> \tSize operator*(float factor) const;\n> \tSize operator/(float factor) const;\n> @@ -294,11 +292,11 @@ public:\n> \tRectangle &scaleBy(const Size &numerator, const Size &denominator);\n> \tRectangle &translateBy(const Point &point);\n> \n> -\t__nodiscard Rectangle boundedTo(const Rectangle &bound) const;\n> -\t__nodiscard Rectangle enclosedIn(const Rectangle &boundary) const;\n> -\t__nodiscard Rectangle scaledBy(const Size &numerator,\n> +\t[[nodiscard]] Rectangle boundedTo(const Rectangle &bound) const;\n> +\t[[nodiscard]] Rectangle enclosedIn(const Rectangle &boundary) const;\n> +\t[[nodiscard]] Rectangle scaledBy(const Size &numerator,\n> \t\t\t\t const Size &denominator) const;\n> -\t__nodiscard Rectangle translatedBy(const Point &point) const;\n> +\t[[nodiscard]] Rectangle translatedBy(const Point &point) const;\n> \n> \tRectangle transformedBetween(const Rectangle &source,\n> \t\t\t\t const Rectangle &target) const;\n> \n> ---\n> base-commit: 35ed4b91291d9f3d08e4b51acfb51163e65df8f8\n> change-id: 20250103-nodiscard-redef-9158e8fdc3f5","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 17177C32DC\n\tfor ;\n\tMon, 6 Jan 2025 11:30:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 12D35684E2;\n\tMon, 6 Jan 2025 12:30:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C885F61891\n\tfor ;\n\tMon, 6 Jan 2025 12:30:41 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 155A04CE;\n\tMon, 6 Jan 2025 12:29:50 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HlVStytC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736162990;\n\tbh=CaEeUC9Ll/AFceY4W9wYGl7hjX5PlM1TyKIGLC3OD+w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HlVStytCNz5bX0RwlBCKyIhFAl9U+fOxqdTcqWt9m0IC8SPqqGCyCL0pGMN9I3D+j\n\tYraYYaZrkUDy8FESKCfIknYCpFRnJg6+eHj3qT58ZYPgL0ZADYTv5a4RAZiO2NYgxx\n\t2QMeDu1ka101VOf3heN2r+WjphoTrBk9SoVXZxHg=","Date":"Mon, 6 Jan 2025 13:30:40 +0200","From":"Laurent Pinchart ","To":"Mattijs Korpershoek ","Cc":"libcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque ","Subject":"Re: [PATCH v2] libcamera: base: Remove custom __nodiscard attribute","Message-ID":"<20250106113040.GD10308@pendragon.ideasonboard.com>","References":"<20250106-nodiscard-redef-v2-1-e4a9afd04c7c@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250106-nodiscard-redef-v2-1-e4a9afd04c7c@baylibre.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32970,"web_url":"https://patchwork.libcamera.org/comment/32970/","msgid":"<173634984411.1594000.12573332200126525598@ping.linuxembedded.co.uk>","date":"2025-01-08T15:24:04","subject":"Re: [PATCH v2] libcamera: base: Remove custom __nodiscard attribute","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2025-01-06 11:30:40)\n> Hi Mattijs,\n> \n> Thank you for the patch.\n> \n> On Mon, Jan 06, 2025 at 10:40:41AM +0100, Mattijs Korpershoek wrote:\n> > __nodiscard was introduced for compatibility with C++14.\n> > In C++17, there is an official attribute: [[nodiscard]].\n> > Moreover, some libc implementations (like bionic) already define the\n> > __nodiscard macro [1].\n> > \n> > Since:\n> > - libcamera builds with cpp_std=c++17\n> > - [[nodiscard]] is already used in the android HAL (exif)\n> > \n> > We should replace all usage __nodiscard of by [[nodiscard]] for\n> > consistency.\n> > \n> > Do the replacement and remove the no longer used compiler.h.\n> > \n> > [1] https://android-review.googlesource.com/c/platform/bionic/+/3254860\n> > \n> > Signed-off-by: Mattijs Korpershoek \n> \n> Reviewed-by: Laurent Pinchart \n\nSeems fine to me.\n\nThree indentation issues that can be fixed while applying. I'll probably\ndo that now.\n\nReviewed-by: Kieran Bingham \n\n> \n> > ---\n> > Hi, it's been a while. I've found a (trivial) build issue when\n> > building against recent bionic versions.\n> > \n> > After discussion in v1, dropping compiler.h seemed the right choice.\n> > ---\n> > Changes in v2:\n> > - Drop support of __nodiscard instead of adding another #ifdef (Laurent)\n> > - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2025-January/047922.html\n> > ---\n> > include/libcamera/base/compiler.h | 14 --------------\n> > include/libcamera/base/meson.build | 1 -\n> > include/libcamera/base/unique_fd.h | 3 +--\n> > include/libcamera/geometry.h | 28 +++++++++++++---------------\n> > 4 files changed, 14 insertions(+), 32 deletions(-)\n> > \n> > diff --git a/include/libcamera/base/compiler.h b/include/libcamera/base/compiler.h\n> > deleted file mode 100644\n> > index fda8fdfdc543f86c5554e38ef790c00d72d60389..0000000000000000000000000000000000000000\n> > --- a/include/libcamera/base/compiler.h\n> > +++ /dev/null\n> > @@ -1,14 +0,0 @@\n> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > -/*\n> > - * Copyright (C) 2021, Google Inc.\n> > - *\n> > - * Compiler support\n> > - */\n> > -\n> > -#pragma once\n> > -\n> > -#if __cplusplus >= 201703L\n> > -#define __nodiscard [[nodiscard]]\n> > -#else\n> > -#define __nodiscard\n> > -#endif\n> > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> > index 2a0cee317204b0d6b44276703fac229bfd7973b9..f28ae4d42a69c755710b51ffc92976bb6fb6a0d8 100644\n> > --- a/include/libcamera/base/meson.build\n> > +++ b/include/libcamera/base/meson.build\n> > @@ -5,7 +5,6 @@ libcamera_base_include_dir = libcamera_include_dir / 'base'\n> > libcamera_base_public_headers = files([\n> > 'bound_method.h',\n> > 'class.h',\n> > - 'compiler.h',\n> > 'flags.h',\n> > 'object.h',\n> > 'shared_fd.h',\n> > diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h\n> > index c9a3b5d0e8628533bd18649e156ba78da40ca200..3066bf28f745df1d26a74c20d21b607dba3d41f4 100644\n> > --- a/include/libcamera/base/unique_fd.h\n> > +++ b/include/libcamera/base/unique_fd.h\n> > @@ -10,7 +10,6 @@\n> > #include \n> > \n> > #include \n> > -#include \n> > \n> > namespace libcamera {\n> > \n> > @@ -43,7 +42,7 @@ public:\n> > return *this;\n> > }\n> > \n> > - __nodiscard int release()\n> > + [[nodiscard]] int release()\n> > {\n> > int fd = fd_;\n> > fd_ = -1;\n> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > index e5f0a843d3144d2086c42c11ab40b0a98438a7e2..0a7133f24c78d4b70075fdd02eabfa52857937dc 100644\n> > --- a/include/libcamera/geometry.h\n> > +++ b/include/libcamera/geometry.h\n> > @@ -11,8 +11,6 @@\n> > #include \n> > #include \n> > \n> > -#include \n> > -\n> > namespace libcamera {\n> > \n> > class Rectangle;\n> > @@ -110,7 +108,7 @@ public:\n> > return *this;\n> > }\n> > \n> > - __nodiscard constexpr Size alignedDownTo(unsigned int hAlignment,\n> > + [[nodiscard]] constexpr Size alignedDownTo(unsigned int hAlignment,\n> > unsigned int vAlignment) const\n\nNit for perhaps while applying, this second line is now 2 chars unaligned.\n\n> > {\n> > return {\n> > @@ -119,7 +117,7 @@ public:\n> > };\n> > }\n> > \n> > - __nodiscard constexpr Size alignedUpTo(unsigned int hAlignment,\n> > + [[nodiscard]] constexpr Size alignedUpTo(unsigned int hAlignment,\n> > unsigned int vAlignment) const\n\nNit for perhaps while applying, this second line is now 2 chars unaligned.\n\n> > {\n> > return {\n> > @@ -128,7 +126,7 @@ public:\n> > };\n> > }\n> > \n> > - __nodiscard constexpr Size boundedTo(const Size &bound) const\n> > + [[nodiscard]] constexpr Size boundedTo(const Size &bound) const\n> > {\n> > return {\n> > std::min(width, bound.width),\n> > @@ -136,7 +134,7 @@ public:\n> > };\n> > }\n> > \n> > - __nodiscard constexpr Size expandedTo(const Size &expand) const\n> > + [[nodiscard]] constexpr Size expandedTo(const Size &expand) const\n> > {\n> > return {\n> > std::max(width, expand.width),\n> > @@ -144,7 +142,7 @@ public:\n> > };\n> > }\n> > \n> > - __nodiscard constexpr Size grownBy(const Size &margins) const\n> > + [[nodiscard]] constexpr Size grownBy(const Size &margins) const\n> > {\n> > return {\n> > width + margins.width,\n> > @@ -152,7 +150,7 @@ public:\n> > };\n> > }\n> > \n> > - __nodiscard constexpr Size shrunkBy(const Size &margins) const\n> > + [[nodiscard]] constexpr Size shrunkBy(const Size &margins) const\n> > {\n> > return {\n> > width > margins.width ? width - margins.width : 0,\n> > @@ -160,10 +158,10 @@ public:\n> > };\n> > }\n> > \n> > - __nodiscard Size boundedToAspectRatio(const Size &ratio) const;\n> > - __nodiscard Size expandedToAspectRatio(const Size &ratio) const;\n> > + [[nodiscard]] Size boundedToAspectRatio(const Size &ratio) const;\n> > + [[nodiscard]] Size expandedToAspectRatio(const Size &ratio) const;\n> > \n> > - __nodiscard Rectangle centeredTo(const Point ¢er) const;\n> > + [[nodiscard]] Rectangle centeredTo(const Point ¢er) const;\n> > \n> > Size operator*(float factor) const;\n> > Size operator/(float factor) const;\n> > @@ -294,11 +292,11 @@ public:\n> > Rectangle &scaleBy(const Size &numerator, const Size &denominator);\n> > Rectangle &translateBy(const Point &point);\n> > \n> > - __nodiscard Rectangle boundedTo(const Rectangle &bound) const;\n> > - __nodiscard Rectangle enclosedIn(const Rectangle &boundary) const;\n> > - __nodiscard Rectangle scaledBy(const Size &numerator,\n> > + [[nodiscard]] Rectangle boundedTo(const Rectangle &bound) const;\n> > + [[nodiscard]] Rectangle enclosedIn(const Rectangle &boundary) const;\n> > + [[nodiscard]] Rectangle scaledBy(const Size &numerator,\n> > const Size &denominator) const;\n\nNit for perhaps while applying, this second line is now 2 chars unaligned.\n\n> > - __nodiscard Rectangle translatedBy(const Point &point) const;\n> > + [[nodiscard]] Rectangle translatedBy(const Point &point) const;\n> > \n> > Rectangle transformedBetween(const Rectangle &source,\n> > const Rectangle &target) const;\n> > \n> > ---\n> > base-commit: 35ed4b91291d9f3d08e4b51acfb51163e65df8f8\n> > change-id: 20250103-nodiscard-redef-9158e8fdc3f5\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4758AC32EF\n\tfor ;\n\tWed, 8 Jan 2025 15:24:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54B2F684E2;\n\tWed, 8 Jan 2025 16:24:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A7F08608AB\n\tfor ;\n\tWed, 8 Jan 2025 16:24:08 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B981BD53;\n\tWed, 8 Jan 2025 16:23:14 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cVoh4wFH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736349794;\n\tbh=5sBChFVx+NlrDnskgHyw4ep/wkimmaHmYkLHik+NDdA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=cVoh4wFHIS6Y9PYe9XsiOHevFDmLuHIiWln6nppQMLsv1qEIq+7f6+VWL10XkdVPZ\n\t1j0L0lwsPNJ83LmhSlmCqtMH/wMpbO55nrzL5bThUwv92XZcQloLFcPVqB0MoU1Dih\n\tBn1HgFIaVGOGQUN6SMeHr6G8ugEJrhftncrabZuU=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250106113040.GD10308@pendragon.ideasonboard.com>","References":"<20250106-nodiscard-redef-v2-1-e4a9afd04c7c@baylibre.com>\n\t<20250106113040.GD10308@pendragon.ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: base: Remove custom __nodiscard attribute","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque ","To":"Laurent Pinchart ,\n\tMattijs Korpershoek ","Date":"Wed, 08 Jan 2025 15:24:04 +0000","Message-ID":"<173634984411.1594000.12573332200126525598@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32971,"web_url":"https://patchwork.libcamera.org/comment/32971/","msgid":"<87r05dba16.fsf@baylibre.com>","date":"2025-01-08T15:27:49","subject":"Re: [PATCH v2] libcamera: base: Remove custom __nodiscard attribute","submitter":{"id":153,"url":"https://patchwork.libcamera.org/api/people/153/","name":"Mattijs Korpershoek","email":"mkorpershoek@baylibre.com"},"content":"Hi Kieran,\n\nOn mer., janv. 08, 2025 at 15:24, Kieran Bingham wrote:\n\n> Quoting Laurent Pinchart (2025-01-06 11:30:40)\n>> Hi Mattijs,\n>> \n>> Thank you for the patch.\n>> \n>> On Mon, Jan 06, 2025 at 10:40:41AM +0100, Mattijs Korpershoek wrote:\n>> > __nodiscard was introduced for compatibility with C++14.\n>> > In C++17, there is an official attribute: [[nodiscard]].\n>> > Moreover, some libc implementations (like bionic) already define the\n>> > __nodiscard macro [1].\n>> > \n>> > Since:\n>> > - libcamera builds with cpp_std=c++17\n>> > - [[nodiscard]] is already used in the android HAL (exif)\n>> > \n>> > We should replace all usage __nodiscard of by [[nodiscard]] for\n>> > consistency.\n>> > \n>> > Do the replacement and remove the no longer used compiler.h.\n>> > \n>> > [1] https://android-review.googlesource.com/c/platform/bionic/+/3254860\n>> > \n>> > Signed-off-by: Mattijs Korpershoek \n>> \n>> Reviewed-by: Laurent Pinchart \n>\n> Seems fine to me.\n>\n> Three indentation issues that can be fixed while applying. I'll probably\n> do that now.\n\nSorry about the indentation issues, I thought I ran utils/checkstyle.py.\n\nI'm happy if you fixup while applying.\n\nThanks!\n\n>\n> Reviewed-by: Kieran Bingham \n>\n>> \n>> > ---\n>> > Hi, it's been a while. I've found a (trivial) build issue when\n>> > building against recent bionic versions.\n>> > \n>> > After discussion in v1, dropping compiler.h seemed the right choice.\n>> > ---\n>> > Changes in v2:\n>> > - Drop support of __nodiscard instead of adding another #ifdef (Laurent)\n>> > - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2025-January/047922.html\n>> > ---\n>> > include/libcamera/base/compiler.h | 14 --------------\n>> > include/libcamera/base/meson.build | 1 -\n>> > include/libcamera/base/unique_fd.h | 3 +--\n>> > include/libcamera/geometry.h | 28 +++++++++++++---------------\n>> > 4 files changed, 14 insertions(+), 32 deletions(-)\n>> > \n>> > diff --git a/include/libcamera/base/compiler.h b/include/libcamera/base/compiler.h\n>> > deleted file mode 100644\n>> > index fda8fdfdc543f86c5554e38ef790c00d72d60389..0000000000000000000000000000000000000000\n>> > --- a/include/libcamera/base/compiler.h\n>> > +++ /dev/null\n>> > @@ -1,14 +0,0 @@\n>> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> > -/*\n>> > - * Copyright (C) 2021, Google Inc.\n>> > - *\n>> > - * Compiler support\n>> > - */\n>> > -\n>> > -#pragma once\n>> > -\n>> > -#if __cplusplus >= 201703L\n>> > -#define __nodiscard [[nodiscard]]\n>> > -#else\n>> > -#define __nodiscard\n>> > -#endif\n>> > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n>> > index 2a0cee317204b0d6b44276703fac229bfd7973b9..f28ae4d42a69c755710b51ffc92976bb6fb6a0d8 100644\n>> > --- a/include/libcamera/base/meson.build\n>> > +++ b/include/libcamera/base/meson.build\n>> > @@ -5,7 +5,6 @@ libcamera_base_include_dir = libcamera_include_dir / 'base'\n>> > libcamera_base_public_headers = files([\n>> > 'bound_method.h',\n>> > 'class.h',\n>> > - 'compiler.h',\n>> > 'flags.h',\n>> > 'object.h',\n>> > 'shared_fd.h',\n>> > diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h\n>> > index c9a3b5d0e8628533bd18649e156ba78da40ca200..3066bf28f745df1d26a74c20d21b607dba3d41f4 100644\n>> > --- a/include/libcamera/base/unique_fd.h\n>> > +++ b/include/libcamera/base/unique_fd.h\n>> > @@ -10,7 +10,6 @@\n>> > #include \n>> > \n>> > #include \n>> > -#include \n>> > \n>> > namespace libcamera {\n>> > \n>> > @@ -43,7 +42,7 @@ public:\n>> > return *this;\n>> > }\n>> > \n>> > - __nodiscard int release()\n>> > + [[nodiscard]] int release()\n>> > {\n>> > int fd = fd_;\n>> > fd_ = -1;\n>> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n>> > index e5f0a843d3144d2086c42c11ab40b0a98438a7e2..0a7133f24c78d4b70075fdd02eabfa52857937dc 100644\n>> > --- a/include/libcamera/geometry.h\n>> > +++ b/include/libcamera/geometry.h\n>> > @@ -11,8 +11,6 @@\n>> > #include \n>> > #include \n>> > \n>> > -#include \n>> > -\n>> > namespace libcamera {\n>> > \n>> > class Rectangle;\n>> > @@ -110,7 +108,7 @@ public:\n>> > return *this;\n>> > }\n>> > \n>> > - __nodiscard constexpr Size alignedDownTo(unsigned int hAlignment,\n>> > + [[nodiscard]] constexpr Size alignedDownTo(unsigned int hAlignment,\n>> > unsigned int vAlignment) const\n>\n> Nit for perhaps while applying, this second line is now 2 chars unaligned.\n>\n>> > {\n>> > return {\n>> > @@ -119,7 +117,7 @@ public:\n>> > };\n>> > }\n>> > \n>> > - __nodiscard constexpr Size alignedUpTo(unsigned int hAlignment,\n>> > + [[nodiscard]] constexpr Size alignedUpTo(unsigned int hAlignment,\n>> > unsigned int vAlignment) const\n>\n> Nit for perhaps while applying, this second line is now 2 chars unaligned.\n>\n>> > {\n>> > return {\n>> > @@ -128,7 +126,7 @@ public:\n>> > };\n>> > }\n>> > \n>> > - __nodiscard constexpr Size boundedTo(const Size &bound) const\n>> > + [[nodiscard]] constexpr Size boundedTo(const Size &bound) const\n>> > {\n>> > return {\n>> > std::min(width, bound.width),\n>> > @@ -136,7 +134,7 @@ public:\n>> > };\n>> > }\n>> > \n>> > - __nodiscard constexpr Size expandedTo(const Size &expand) const\n>> > + [[nodiscard]] constexpr Size expandedTo(const Size &expand) const\n>> > {\n>> > return {\n>> > std::max(width, expand.width),\n>> > @@ -144,7 +142,7 @@ public:\n>> > };\n>> > }\n>> > \n>> > - __nodiscard constexpr Size grownBy(const Size &margins) const\n>> > + [[nodiscard]] constexpr Size grownBy(const Size &margins) const\n>> > {\n>> > return {\n>> > width + margins.width,\n>> > @@ -152,7 +150,7 @@ public:\n>> > };\n>> > }\n>> > \n>> > - __nodiscard constexpr Size shrunkBy(const Size &margins) const\n>> > + [[nodiscard]] constexpr Size shrunkBy(const Size &margins) const\n>> > {\n>> > return {\n>> > width > margins.width ? width - margins.width : 0,\n>> > @@ -160,10 +158,10 @@ public:\n>> > };\n>> > }\n>> > \n>> > - __nodiscard Size boundedToAspectRatio(const Size &ratio) const;\n>> > - __nodiscard Size expandedToAspectRatio(const Size &ratio) const;\n>> > + [[nodiscard]] Size boundedToAspectRatio(const Size &ratio) const;\n>> > + [[nodiscard]] Size expandedToAspectRatio(const Size &ratio) const;\n>> > \n>> > - __nodiscard Rectangle centeredTo(const Point ¢er) const;\n>> > + [[nodiscard]] Rectangle centeredTo(const Point ¢er) const;\n>> > \n>> > Size operator*(float factor) const;\n>> > Size operator/(float factor) const;\n>> > @@ -294,11 +292,11 @@ public:\n>> > Rectangle &scaleBy(const Size &numerator, const Size &denominator);\n>> > Rectangle &translateBy(const Point &point);\n>> > \n>> > - __nodiscard Rectangle boundedTo(const Rectangle &bound) const;\n>> > - __nodiscard Rectangle enclosedIn(const Rectangle &boundary) const;\n>> > - __nodiscard Rectangle scaledBy(const Size &numerator,\n>> > + [[nodiscard]] Rectangle boundedTo(const Rectangle &bound) const;\n>> > + [[nodiscard]] Rectangle enclosedIn(const Rectangle &boundary) const;\n>> > + [[nodiscard]] Rectangle scaledBy(const Size &numerator,\n>> > const Size &denominator) const;\n>\n> Nit for perhaps while applying, this second line is now 2 chars unaligned.\n>\n>> > - __nodiscard Rectangle translatedBy(const Point &point) const;\n>> > + [[nodiscard]] Rectangle translatedBy(const Point &point) const;\n>> > \n>> > Rectangle transformedBetween(const Rectangle &source,\n>> > const Rectangle &target) const;\n>> > \n>> > ---\n>> > base-commit: 35ed4b91291d9f3d08e4b51acfb51163e65df8f8\n>> > change-id: 20250103-nodiscard-redef-9158e8fdc3f5\n>> \n>> -- \n>> Regards,\n>> \n>> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 96E14BD80A\n\tfor ;\n\tWed, 8 Jan 2025 15:27:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84272684E1;\n\tWed, 8 Jan 2025 16:27:52 +0100 (CET)","from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com\n\t[IPv6:2a00:1450:4864:20::42d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22623608AB\n\tfor ;\n\tWed, 8 Jan 2025 16:27:51 +0100 (CET)","by mail-wr1-x42d.google.com with SMTP id\n\tffacd0b85a97d-38633b5dbcfso16235354f8f.2\n\tfor ;\n\tWed, 08 Jan 2025 07:27:51 -0800 (PST)","from localhost ([2a01:cb19:95ba:5000:d6dd:417f:52ac:335b])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-38a1c832e8asm52450503f8f.37.2025.01.08.07.27.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 08 Jan 2025 07:27:50 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=baylibre-com.20230601.gappssmtp.com\n\theader.i=@baylibre-com.20230601.gappssmtp.com\n\theader.b=\"RQyU1kka\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1736350071;\n\tx=1736954871; darn=lists.libcamera.org; \n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:from:to:cc:subject:date:message-id:reply-to;\n\tbh=D+EmIPiRlsJgxcO0fSGySLohVHjSyOqT0uXnPhCthuI=;\n\tb=RQyU1kkarkYUwxmrj5GlbMPk1bpGEclEDBjuqOLxM9iw/MxLJL9iwanahF0qvSeLoK\n\tRnSzkE3jSqPfQBqK5zUfVc7bXCKq5S/IiM0ZBfhrnezW9Yj1CSjca229In5mp5AVPWv+\n\t/4+ktT7oA+pTos4FDGsdHMreUpEeK+f8Oc4EYQZcGtDUQGGXq+RhQZWZj70cqI6Rafar\n\tP8YO41wvWdDo+UVo3sq+vsY9KWMPxzg4LrDr0S1cTrhHWBJ1yCAKcQXe0umjpk7ch38u\n\tcDnouKuimUKQDK/USRj9c12DqlzHSyJ/FeqhY1//IMlNBJqOQ6z7qBoAgih9pnlinK3X\n\tO6ig==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1736350071; x=1736954871;\n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; \n\tbh=D+EmIPiRlsJgxcO0fSGySLohVHjSyOqT0uXnPhCthuI=;\n\tb=QmYv9ykVkLJwE0jEWQqXk05R0XsTz4+1vrvobFcnYd+cyRPRQaAa0U+phHSHsMZgjS\n\tY5gnb7JPSHCYjaurosmNb4mmjYNieHlMm14JHYl/yQwzauVnPhX6N+tzFfBk/Rt7meLM\n\t+CM2a2yqGaV8mAoajKpWsLV91uRAm0PuWQuZZT/7g8TNa7PzIAsq4niVjZdO4+tD7fRZ\n\tNeWQ0shEI35IjfHQQogw3FQbZ6bjzHIse7OVexmtIcu5ktMkn6MHtjH5e5kHFGROV7Si\n\tIUAEK+SAKtxfkAVGAPLOTHlTrCtuvTUMT8X5Ivz7lqcAhh9eNlK1SXrw/1kvGkaVwJXZ\n\tGJqg==","X-Gm-Message-State":"AOJu0YyF5XfSnOiBmRdm7Pnoxh6BU7AmfORCdU84WnCklWkrUEBtP3W1\n\t6TRzWO1Flm+dmpfbVFg25MCLJ9lFevwt4TSW7cllNOCJpGRiB4d3YsFyLswFcOo=","X-Gm-Gg":"ASbGncthQZhooI+fZupzEGt9dqicYecAOaAYqofsRhq61rUAfZ1bt80Ll6i1MaS8Tnc\n\tr+oReFKXpBda6mKJ8DZyjcUE4Y0h+fiYGzymoGoBnRagcjDW9biwan/RnAeaMeOno4gG+qLovC4\n\tXexEigGLuIUQ12SpEfNZj4Xu+2dx7fwKk6feIuYfNeqYvPaHHEqkQwCH/iUpp5p+StoVceKs+3m\n\tioRPCCVB8cjke0C/BHbTachqKvYmipepXBQ1RahNunO+BXvGZEc5B1Kemr4KQOGLA==","X-Google-Smtp-Source":"AGHT+IEpyarXRhw+Q+YzxkKPpWKLMvvhirs9jH1UxZNY9krD2jP7Yf7NlVjo698bJsJ6GtZhJXQpPw==","X-Received":"by 2002:a5d:6d02:0:b0:38a:50f7:24fa with SMTP id\n\tffacd0b85a97d-38a87357a07mr3001763f8f.54.1736350070602; \n\tWed, 08 Jan 2025 07:27:50 -0800 (PST)","From":"Mattijs Korpershoek ","To":"Kieran Bingham , Laurent Pinchart\n\t","Cc":"libcamera-devel@lists.libcamera.org, Guillaume La Roque\n\t","Subject":"Re: [PATCH v2] libcamera: base: Remove custom __nodiscard attribute","In-Reply-To":"<173634984411.1594000.12573332200126525598@ping.linuxembedded.co.uk>","References":"<20250106-nodiscard-redef-v2-1-e4a9afd04c7c@baylibre.com>\n\t<20250106113040.GD10308@pendragon.ideasonboard.com>\n\t<173634984411.1594000.12573332200126525598@ping.linuxembedded.co.uk>","Date":"Wed, 08 Jan 2025 16:27:49 +0100","Message-ID":"<87r05dba16.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]