[{"id":29558,"web_url":"https://patchwork.libcamera.org/comment/29558/","msgid":"<171619881645.2248009.10949398928201192657@ping.linuxembedded.co.uk>","date":"2024-05-20T09:53:36","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-05-20 05:02:50)\n> This way the construction of the default value of type `T`\n> can be delayed until it is really needed, which is useful,\n> for example when `T == std::string`, as the default value\n> string would always be constructed otherwise.\n> \n> Signed-off-by: Barnabás Pőcze \n\nThis looks and sounds good to me.\n\nReviewed-by: Kieran Bingham \n\n> ---\n> include/libcamera/internal/yaml_parser.h | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index b6979d73..3ac27e06 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -179,10 +179,10 @@ public:\n> #endif\n> std::optional get() const;\n> \n> - template\n> - T get(const T &defaultValue) const\n> + template\n> + T get(U &&defaultValue) const\n> {\n> - return get().value_or(defaultValue);\n> + return get().value_or(std::forward(defaultValue));\n> }\n> \n> #ifndef __DOXYGEN__\n> -- \n> 2.45.1\n> \n>","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 2FF77BDE6B\n\tfor ;\n\tMon, 20 May 2024 09:53:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F8666347E;\n\tMon, 20 May 2024 11:53:42 +0200 (CEST)","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 06CA261A55\n\tfor ;\n\tMon, 20 May 2024 11:53:40 +0200 (CEST)","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 B3DF5E39;\n\tMon, 20 May 2024 11:53:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ex327bez\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716198808;\n\tbh=9RgmiOnUjWFu8r+9tBFfskqMS0R1hfAdRffKIynYFVE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Ex327bezUaJbr8fB5zWcfSAQfLfDwdws6Nqj0EWIGuaXVL0xHLQGKgNiObPlLeFVB\n\tEeynt7LzURjrqCR6OVGREJaFIjdMd/kJjTVs2Y66llzN4WEjpsaHQ5t4Da8OEN9spE\n\tmIXCJllzRNtiCWNCENxOQcqnzcKAkIprl1ggvCq4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240520040249.209200-1-pobrn@protonmail.com>","References":"<20240520040249.209200-1-pobrn@protonmail.com>","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 20 May 2024 10:53:36 +0100","Message-ID":"<171619881645.2248009.10949398928201192657@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":29589,"web_url":"https://patchwork.libcamera.org/comment/29589/","msgid":"<20240521141835.GF6640@pendragon.ideasonboard.com>","date":"2024-05-21T14:18:35","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:\n> This way the construction of the default value of type `T`\n> can be delayed until it is really needed, which is useful,\n> for example when `T == std::string`, as the default value\n> string would always be constructed otherwise.\n\nThat's interesting. Is that all it takes to defer the construction of\nthe default value ?\n\nI implemented a different solution a while ago, which was merged in\n48c106429a19 (\"libcamera: base: utils: Provide defopt to simplify\nstd::optional::value_or() usage\") but had to be reverted due to\ncompilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to\nDebian 10 still being supported, although we may drop that once it\nreaches EOL at end of June this year).\n\n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/internal/yaml_parser.h | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index b6979d73..3ac27e06 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -179,10 +179,10 @@ public:\n> #endif\n> \tstd::optional get() const;\n> \n> -\ttemplate\n> -\tT get(const T &defaultValue) const\n> +\ttemplate\n> +\tT get(U &&defaultValue) const\n> \t{\n> -\t\treturn get().value_or(defaultValue);\n> +\t\treturn get().value_or(std::forward(defaultValue));\n> \t}\n> \n> #ifndef __DOXYGEN__","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 99EC2BDE6B\n\tfor ;\n\tTue, 21 May 2024 14:18:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A91B163487;\n\tTue, 21 May 2024 16:18:46 +0200 (CEST)","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 F3E0D61A55\n\tfor ;\n\tTue, 21 May 2024 16:18:44 +0200 (CEST)","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 AFD7CC8E;\n\tTue, 21 May 2024 16:18:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qqp/e647\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716301112;\n\tbh=PWEPqY3j/QrYaS0of1aalnz4xPxGLk0cCTQ/U4C0ykY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qqp/e6476NRZRdOVYMCarsLWF0vRWSS0A1D+YWWcCZacKhf8+WN8fnAyHWh0PQ1AL\n\ti6c7S9SzYmt/cH9/JmWyq95td9aO7AsSHG+csunYsr+6eeTncYBnCGSJhdEelvLelS\n\tTWVHGn4yvKz3FARVIqzANpjtWH/2zxViRL5oOaVo=","Date":"Tue, 21 May 2024 17:18:35 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","Message-ID":"<20240521141835.GF6640@pendragon.ideasonboard.com>","References":"<20240520040249.209200-1-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240520040249.209200-1-pobrn@protonmail.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":29591,"web_url":"https://patchwork.libcamera.org/comment/29591/","msgid":"","date":"2024-05-21T15:40:13","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. május 21., kedd 16:18 keltezéssel, Laurent Pinchart írta:\n\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:\n> > This way the construction of the default value of type `T`\n> > can be delayed until it is really needed, which is useful,\n> > for example when `T == std::string`, as the default value\n> > string would always be constructed otherwise.\n> \n> That's interesting. Is that all it takes to defer the construction of\n> the default value ?\n\nThe motivating example is std::string, previously\n\n x.get(\"asd\");\n\nwould force the construction of a temporary string object from the string literal\n\"asd\" since the argument type is `const std::string&`. Note that if the optional\nwas empty, it would also force the copy construction of another std::string object,\nwhich would be the one returned.\n\nSee the second overload on https://en.cppreference.com/w/cpp/utility/optional/value_or :\n\n Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n\nNow the value with type `T` is only constructed from `default_value` if the optional is empty.\n\nThis works nicely with std::string as now a string literal can be passed as argument,\nand an std::string object will only be constructed from it if needed.\n\n\n> \n> I implemented a different solution a while ago, which was merged in\n> 48c106429a19 (\"libcamera: base: utils: Provide defopt to simplify\n> std::optional::value_or() usage\") but had to be reverted due to\n> compilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to\n> Debian 10 still being supported, although we may drop that once it\n> reaches EOL at end of June this year).\n\nI am fairly sure it should work, although I haven't tested it on gcc 8,\nthis is a reasonably non-intrusive change in my opinion, the argument is simply\nforwarded to value_or() as is.\n\nI am wondering if there are any plans to accept contributions via gitlab merge\nrequests as in those cases the author could immediately see the result of\nCI and make the necessary changes.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > include/libcamera/internal/yaml_parser.h | 6 +++---\n> > 1 file changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index b6979d73..3ac27e06 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -179,10 +179,10 @@ public:\n> > #endif\n> > \tstd::optional get() const;\n> >\n> > -\ttemplate\n> > -\tT get(const T &defaultValue) const\n> > +\ttemplate\n> > +\tT get(U &&defaultValue) const\n> > \t{\n> > -\t\treturn get().value_or(defaultValue);\n> > +\t\treturn get().value_or(std::forward(defaultValue));\n> > \t}\n> >\n> > #ifndef __DOXYGEN__\n> \n> --\n> Regards,\n> \n> Laurent Pinchart\n>","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 0F8D6BD78E\n\tfor ;\n\tTue, 21 May 2024 15:40:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C88D6348E;\n\tTue, 21 May 2024 17:40:18 +0200 (CEST)","from mail-40131.protonmail.ch (mail-40131.protonmail.ch\n\t[185.70.40.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5CE0461A55\n\tfor ;\n\tTue, 21 May 2024 17:40:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"d7K4dur6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1716306015; x=1716565215;\n\tbh=zsiRvnWL3bHZKAbOPmV1z+/8G9g0H5u2JcTqAu/mUW0=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=d7K4dur6JgsctjpMgU8cpsOibtDta9Rn4BDH4LEJmhbnGzA/lQ6uy5pwWT1r3uEfE\n\tYd2lMhV3klaSnseNtoRktAQ7LwHzx1oj2W98XibMAiKwNJscE27W6eKX09gik2Yvj2\n\tiKgc6Z5gQr2/IFkq1nWx7MZoYv+DpQ1npp+tXkhfTir/Vpgm51vjaoEzLHdMz4sCKa\n\tEasOuaf5JQ4FCjG8+01+Ufv2Oi2GpNlH207krYCKm/7JRST+4BzDWgs3+Qw0RW1b8L\n\tV68HC2X0ghZp8J/tdqshGWnJ972hvuI4g24bET4zhiyzZbn8IsrFyHM1l2e0nZuv8O\n\tmPtOfK4p/QaPQ==","Date":"Tue, 21 May 2024 15:40:13 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","Message-ID":"","In-Reply-To":"<20240521141835.GF6640@pendragon.ideasonboard.com>","References":"<20240520040249.209200-1-pobrn@protonmail.com>\n\t<20240521141835.GF6640@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"f09d5d8a0e5157c84da274c909d06b9ceb1c401e","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":29847,"web_url":"https://patchwork.libcamera.org/comment/29847/","msgid":"","date":"2024-06-11T21:31:16","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. május 21., kedd 17:40 keltezéssel, Barnabás Pőcze írta:\n\n> 2024. május 21., kedd 16:18 keltezéssel, Laurent Pinchart írta:\n> \n> > Hi Barnabás,\n> >\n> > Thank you for the patch.\n> >\n> > On Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:\n> > > This way the construction of the default value of type `T`\n> > > can be delayed until it is really needed, which is useful,\n> > > for example when `T == std::string`, as the default value\n> > > string would always be constructed otherwise.\n> >\n> > That's interesting. Is that all it takes to defer the construction of\n> > the default value ?\n> \n> The motivating example is std::string, previously\n> \n> x.get(\"asd\");\n> \n> would force the construction of a temporary string object from the string literal\n> \"asd\" since the argument type is `const std::string&`. Note that if the optional\n> was empty, it would also force the copy construction of another std::string object,\n> which would be the one returned.\n> \n> See the second overload on https://en.cppreference.com/w/cpp/utility/optional/value_or :\n> \n> Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n> \n> Now the value with type `T` is only constructed from `default_value` if the optional is empty.\n> \n> This works nicely with std::string as now a string literal can be passed as argument,\n> and an std::string object will only be constructed from it if needed.\n> \n> \n> >\n> > I implemented a different solution a while ago, which was merged in\n> > 48c106429a19 (\"libcamera: base: utils: Provide defopt to simplify\n> > std::optional::value_or() usage\") but had to be reverted due to\n> > compilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to\n> > Debian 10 still being supported, although we may drop that once it\n> > reaches EOL at end of June this year).\n> \n> I am fairly sure it should work, although I haven't tested it on gcc 8,\n> this is a reasonably non-intrusive change in my opinion, the argument is simply\n> forwarded to value_or() as is.\n> \n> I am wondering if there are any plans to accept contributions via gitlab merge\n> requests as in those cases the author could immediately see the result of\n> CI and make the necessary changes.\n> [...]\n\nAnything else I should do here?\n\n\nRegards,\nBarnabás Pőcze","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 EB0F4C3237\n\tfor ;\n\tTue, 11 Jun 2024 21:31:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9D23365466;\n\tTue, 11 Jun 2024 23:31:21 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B91C361A26\n\tfor ;\n\tTue, 11 Jun 2024 23:31:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"FUAH5jCm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1718141479; x=1718400679;\n\tbh=JYrpXVXIpR3UU7N0sTxJacqkAWXHn6nOvnRgIOKPWek=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=FUAH5jCmizFz5bAA/tEijFVTIiehHDfOvQ1dL4fOrqLSHqZCHlS8XZPlxwn2cfgh5\n\t4vSjNQmcfXI1vkBFdDKxI3iXkfnZ/02/PyfEUQLpOp3f9zebYgZyxZ1p7JwSoPIG+r\n\tGrwPSN8Q9w++RLMpRbwiHaeEMemP3cqpxSF4BGomJJBOLgVjPLRnk44FRqcqB95z5x\n\tRYOfu4flUHA4HqP60cGhq/Six9lECm/c5t3yyAMWE6TrnbQw1HMPgJbxSEBKTIk0MU\n\tcdbGXKeDx4V0n0ohZv3EfDlNo+IIf+VtoQCvPbjJroCuiDe0f646nB4BKXUHndYNPD\n\tHWjZO76Y/COJg==","Date":"Tue, 11 Jun 2024 21:31:16 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","Message-ID":"","In-Reply-To":"","References":"<20240520040249.209200-1-pobrn@protonmail.com>\n\t<20240521141835.GF6640@pendragon.ideasonboard.com>\n\t","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"2254e9765f0642ee2813972c7b7a7942e7feb65d","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":29876,"web_url":"https://patchwork.libcamera.org/comment/29876/","msgid":"<171823109406.2248009.10563184290630835403@ping.linuxembedded.co.uk>","date":"2024-06-12T22:24:54","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-06-11 22:31:16)\n> 2024. május 21., kedd 17:40 keltezéssel, Barnabás Pőcze írta:\n> \n> > 2024. május 21., kedd 16:18 keltezéssel, Laurent Pinchart írta:\n> > \n> > > Hi Barnabás,\n> > >\n> > > Thank you for the patch.\n> > >\n> > > On Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:\n> > > > This way the construction of the default value of type `T`\n> > > > can be delayed until it is really needed, which is useful,\n> > > > for example when `T == std::string`, as the default value\n> > > > string would always be constructed otherwise.\n> > >\n> > > That's interesting. Is that all it takes to defer the construction of\n> > > the default value ?\n> > \n> > The motivating example is std::string, previously\n> > \n> > x.get(\"asd\");\n> > \n> > would force the construction of a temporary string object from the string literal\n> > \"asd\" since the argument type is `const std::string&`. Note that if the optional\n> > was empty, it would also force the copy construction of another std::string object,\n> > which would be the one returned.\n> > \n> > See the second overload on https://en.cppreference.com/w/cpp/utility/optional/value_or :\n> > \n> > Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n> > \n> > Now the value with type `T` is only constructed from `default_value` if the optional is empty.\n> > \n> > This works nicely with std::string as now a string literal can be passed as argument,\n> > and an std::string object will only be constructed from it if needed.\n> > \n> > \n> > >\n> > > I implemented a different solution a while ago, which was merged in\n> > > 48c106429a19 (\"libcamera: base: utils: Provide defopt to simplify\n> > > std::optional::value_or() usage\") but had to be reverted due to\n> > > compilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to\n> > > Debian 10 still being supported, although we may drop that once it\n> > > reaches EOL at end of June this year).\n> > \n> > I am fairly sure it should work, although I haven't tested it on gcc 8,\n> > this is a reasonably non-intrusive change in my opinion, the argument is simply\n> > forwarded to value_or() as is.\n> > \n> > I am wondering if there are any plans to accept contributions via gitlab merge\n> > requests as in those cases the author could immediately see the result of\n> > CI and make the necessary changes.\n\nI'm working on getting the CI kicked from patches sent to the list\n'automatically'*\n\n*Automatically - with approval from someone with commit access to\nprevent potential abuses.\n\nBut meanwhile:\n\n./send-for-testing.sh 4319\n - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1199972 : pending : patchwork/4319\n\nDoes report some documentation issues:\n - https://gitlab.freedesktop.org/camera/libcamera/-/jobs/59823569#L899\n\nI believe we can also give access to regular contributors to either be\nable to enable the CI on their own forks or have push rights to a CI\nbuild on gitlab. An option like that could be arranged for you perhaps?\n\n--\nKieran\n\n\n> > [...]\n> \n> Anything else I should do here?\n> \n> \n> Regards,\n> Barnabás Pőcze","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 B6D9FC3237\n\tfor ;\n\tWed, 12 Jun 2024 22:24:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 240E16545D;\n\tThu, 13 Jun 2024 00:24:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A26EE65446\n\tfor ;\n\tThu, 13 Jun 2024 00:24:56 +0200 (CEST)","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 D4BC37E0;\n\tThu, 13 Jun 2024 00:24:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OoPIMxQR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718231082;\n\tbh=F0p4JNttIbQtJN7ovwEcgRqP1/zImxvMEBxEgwXY8xQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=OoPIMxQRRet1awF1x2Cso/xKO6vnGCBBrCcQA+244Y0jtehqUr5+N8re9gKgMDLZH\n\tXqqUzO/v7m2X3ahL0C5WQ3he4xM1Nnj6JZgDRxfJ0HAv4z+NrjdIyanyKjattRhoAY\n\t1WrB991/8MKidybBtSNrgV+ldDTgy7NP5CmIqC6A=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240520040249.209200-1-pobrn@protonmail.com>\n\t<20240521141835.GF6640@pendragon.ideasonboard.com>\n\t\n\t","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart ","Date":"Wed, 12 Jun 2024 23:24:54 +0100","Message-ID":"<171823109406.2248009.10563184290630835403@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":29877,"web_url":"https://patchwork.libcamera.org/comment/29877/","msgid":"","date":"2024-06-12T22:41:33","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. június 13., csütörtök 0:24 keltezéssel, Kieran Bingham írta:\n\n> Quoting Barnabás Pőcze (2024-06-11 22:31:16)\n> > 2024. május 21., kedd 17:40 keltezéssel, Barnabás Pőcze írta:\n> >\n> > > 2024. május 21., kedd 16:18 keltezéssel, Laurent Pinchart írta:\n> > >\n> > > > Hi Barnabás,\n> > > >\n> > > > Thank you for the patch.\n> > > >\n> > > > On Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:\n> > > > > This way the construction of the default value of type `T`\n> > > > > can be delayed until it is really needed, which is useful,\n> > > > > for example when `T == std::string`, as the default value\n> > > > > string would always be constructed otherwise.\n> > > >\n> > > > That's interesting. Is that all it takes to defer the construction of\n> > > > the default value ?\n> > >\n> > > The motivating example is std::string, previously\n> > >\n> > > x.get(\"asd\");\n> > >\n> > > would force the construction of a temporary string object from the string literal\n> > > \"asd\" since the argument type is `const std::string&`. Note that if the optional\n> > > was empty, it would also force the copy construction of another std::string object,\n> > > which would be the one returned.\n> > >\n> > > See the second overload on https://en.cppreference.com/w/cpp/utility/optional/value_or :\n> > >\n> > > Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n> > >\n> > > Now the value with type `T` is only constructed from `default_value` if the optional is empty.\n> > >\n> > > This works nicely with std::string as now a string literal can be passed as argument,\n> > > and an std::string object will only be constructed from it if needed.\n> > >\n> > >\n> > > >\n> > > > I implemented a different solution a while ago, which was merged in\n> > > > 48c106429a19 (\"libcamera: base: utils: Provide defopt to simplify\n> > > > std::optional::value_or() usage\") but had to be reverted due to\n> > > > compilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to\n> > > > Debian 10 still being supported, although we may drop that once it\n> > > > reaches EOL at end of June this year).\n> > >\n> > > I am fairly sure it should work, although I haven't tested it on gcc 8,\n> > > this is a reasonably non-intrusive change in my opinion, the argument is simply\n> > > forwarded to value_or() as is.\n> > >\n> > > I am wondering if there are any plans to accept contributions via gitlab merge\n> > > requests as in those cases the author could immediately see the result of\n> > > CI and make the necessary changes.\n> \n> I'm working on getting the CI kicked from patches sent to the list\n> 'automatically'*\n> \n> *Automatically - with approval from someone with commit access to\n> prevent potential abuses.\n> \n> But meanwhile:\n> \n> ./send-for-testing.sh 4319\n> - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1199972 : pending : patchwork/4319\n> \n> Does report some documentation issues:\n> - https://gitlab.freedesktop.org/camera/libcamera/-/jobs/59823569#L899\n\nOops, I forgot to add the documentation changes to the commit... I will send a new version.\n\n\n> \n> I believe we can also give access to regular contributors to either be\n> able to enable the CI on their own forks or have push rights to a CI\n> build on gitlab. An option like that could be arranged for you perhaps?\n\nWell, my preference is being able to send changes via gitlab. I understand\nthis is not the project's preference, but so far nothing has shaken me in my belief\nthat a merge request with automatic CI is most convenient for contributors.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> [...]","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 CA3DABD87C\n\tfor ;\n\tWed, 12 Jun 2024 22:41:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBAD965463;\n\tThu, 13 Jun 2024 00:41:39 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3566265446\n\tfor ;\n\tThu, 13 Jun 2024 00:41:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"NxZpAJ4I\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1718232097; x=1718491297;\n\tbh=kthH24FTDcNCsHZlnn+yPytz4CXdjFuuX8BxmvMu7C4=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=NxZpAJ4Il3K0jVXwep4ceul5k+E+yrM40C8WrFYjKZExjLfrkrdY1TKLUOWsTEebN\n\t7QVCEYJo76UCdV2SHXBfE0oBDfyvFKrHTSpyYANGB4r3TgTp/id6pg6HXeIsthk3fJ\n\tjjM3MD4nzbaisQcUqxueYGqOedytCxm3y4oGGH8a4nWfo1lH/B/+2EZjfaGLJ4ftQi\n\tSIIU+Fu/wxo2FznH4po3sdMyJBF5x9cjt84mUP5gMbsA6JTtJnKEfjoKm7Va4JMmKu\n\tS55scsj+y3HykYoeGF0znPg4wkt+nF5Eg0AuQyS7nS6u06lCKpVbaq617rWKVgkk3k\n\tqXuQ1mry7H/WQ==","Date":"Wed, 12 Jun 2024 22:41:33 +0000","To":"Kieran Bingham ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","Message-ID":"","In-Reply-To":"<171823109406.2248009.10563184290630835403@ping.linuxembedded.co.uk>","References":"<20240520040249.209200-1-pobrn@protonmail.com>\n\t<20240521141835.GF6640@pendragon.ideasonboard.com>\n\t\n\t\n\t<171823109406.2248009.10563184290630835403@ping.linuxembedded.co.uk>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"ca29a2816e14ac4a76721988daea860f4379c016","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":29879,"web_url":"https://patchwork.libcamera.org/comment/29879/","msgid":"<20240612230703.GA7291@pendragon.ideasonboard.com>","date":"2024-06-12T23:07:03","subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nOn Tue, May 21, 2024 at 03:40:13PM +0000, Barnabás Pőcze wrote:\n> 2024. május 21., kedd 16:18 keltezéssel, Laurent Pinchart írta:\n> > On Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:\n> > > This way the construction of the default value of type `T`\n> > > can be delayed until it is really needed, which is useful,\n> > > for example when `T == std::string`, as the default value\n> > > string would always be constructed otherwise.\n> > \n> > That's interesting. Is that all it takes to defer the construction of\n> > the default value ?\n> \n> The motivating example is std::string, previously\n> \n> x.get(\"asd\");\n> \n> would force the construction of a temporary string object from the string literal\n> \"asd\" since the argument type is `const std::string&`. Note that if the optional\n> was empty, it would also force the copy construction of another std::string object,\n> which would be the one returned.\n> \n> See the second overload on https://en.cppreference.com/w/cpp/utility/optional/value_or :\n> \n> Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n> \n> Now the value with type `T` is only constructed from `default_value` if the optional is empty.\n> \n> This works nicely with std::string as now a string literal can be passed as argument,\n> and an std::string object will only be constructed from it if needed.\n\nAahhhh thanks for the explanation. So this won't prevent the std::string\nconstruction for\n\n\tx.get(std::string{\"abc\"})\n\nbut it will for\n\n\tx.get(\"abc\")\n\n(assuming x is not empty of course).\n\nThat's a useful change I think.\n\n> > I implemented a different solution a while ago, which was merged in\n> > 48c106429a19 (\"libcamera: base: utils: Provide defopt to simplify\n> > std::optional::value_or() usage\") but had to be reverted due to\n> > compilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to\n> > Debian 10 still being supported, although we may drop that once it\n> > reaches EOL at end of June this year).\n> \n> I am fairly sure it should work, although I haven't tested it on gcc 8,\n> this is a reasonably non-intrusive change in my opinion, the argument is simply\n> forwarded to value_or() as is.\n\nI think it's valid C++ code, but gcc 8.0.0 to 8.3.0 unfortunately didn't\nagree :-( I'm tempted to revive the code at some point this year, if we\ndecide to drop support for Debian 10.\n\n> I am wondering if there are any plans to accept contributions via gitlab merge\n> requests as in those cases the author could immediately see the result of\n> CI and make the necessary changes.\n\nWe are considering different options to improve the contribution model,\nbut for the time being we want to keep the mailing list workflow for\nreviews. One idea that was floated around was a merge request to e-mail\ngateway (one direction only, reviews would then move to e-mail, they\nwouldn't be forwarded to gitlab).\n\nDue to the amount of CI abuse on gitlab.fdo, I don't think we'll be able\nto automatically trigger pipeline runs when a merge request is submitted\nby a non-member though. Manual pre-review and approval of the pipeline\nrun will likely be needed. If you're a member of the camera CI group (or\nwould like to become one) then you could already have access to CI\nthrough your own clone.\n\nAnother thing we are considering is automating the creation of CI\npipelines from patch series posted to the list. It will still require\nmanual approval from maintainers due to the issue listed above, but it\nwould still be a good step forward. There's a high chance this will get\nimplemented in the not too distant future.\n\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > include/libcamera/internal/yaml_parser.h | 6 +++---\n> > > 1 file changed, 3 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > index b6979d73..3ac27e06 100644\n> > > --- a/include/libcamera/internal/yaml_parser.h\n> > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > @@ -179,10 +179,10 @@ public:\n> > > #endif\n> > > \tstd::optional get() const;\n> > >\n> > > -\ttemplate\n> > > -\tT get(const T &defaultValue) const\n> > > +\ttemplate\n> > > +\tT get(U &&defaultValue) const\n> > > \t{\n> > > -\t\treturn get().value_or(defaultValue);\n> > > +\t\treturn get().value_or(std::forward(defaultValue));\n> > > \t}\n> > >\n> > > #ifndef __DOXYGEN__","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 08FDAC3237\n\tfor ;\n\tWed, 12 Jun 2024 23:07:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F212A65463;\n\tThu, 13 Jun 2024 01:07:24 +0200 (CEST)","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 C93D565446\n\tfor ;\n\tThu, 13 Jun 2024 01:07:23 +0200 (CEST)","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 9F755E4;\n\tThu, 13 Jun 2024 01:07:09 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OAy9I4y1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718233629;\n\tbh=qTgVq6tjLCMTxHht+KfwqtiZyduQU2uw544p4z+QMuI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OAy9I4y1NeyCwx2hYZIGpTwJ+G+/CN0me7zerCTLdtrekUQqX0Q7J1+Ic47WGKq4w\n\tohp/DfptKj6r80BsjkQHCsX5k3hKlGcA816W5SMFke5ncMCMNktpolljx7SQHpIH/N\n\t+DSpdnz025L88J0WOZQxGrh6Ja4yiMBbBOBIkuHg=","Date":"Thu, 13 Jun 2024 02:07:03 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: yaml_parser: Make default value templated\n\tin `get()`","Message-ID":"<20240612230703.GA7291@pendragon.ideasonboard.com>","References":"<20240520040249.209200-1-pobrn@protonmail.com>\n\t<20240521141835.GF6640@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}}]