[{"id":35495,"web_url":"https://patchwork.libcamera.org/comment/35495/","msgid":"<20250819024148.GI5862@pendragon.ideasonboard.com>","date":"2025-08-19T02:41:48","subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","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 Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote:\n> Forcing the \"non-pod\" members to be initialized from `const T&` is not the\n> ideal solution because it disallows e.g. move constructors. So generate a\n> templated constructor, which members to be initialized more freely, e.g.\n> using move constructors.\n> \n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n> changes in v2:\n> \t* use `std::is_convertible_v` to avoid accidental use of explicit\n> \t  constructors/conversion operators\n> ---\n>  .../libcamera_templates/core_ipa_interface.h.tmpl |  2 ++\n>  .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++----\n>  .../module_ipa_interface.h.tmpl                   |  2 ++\n>  3 files changed, 15 insertions(+), 4 deletions(-)\n> \n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> index 3942e5708..93f988cd9 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> @@ -16,6 +16,8 @@\n> \n>  {% if has_map %}#include <map>{% endif %}\n>  {% if has_string %}#include <string>{% endif %}\n> +#include <type_traits>\n> +#include <utility>\n>  {% if has_array %}#include <vector>{% endif %}\n> \n>  #include <libcamera/controls.h>\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> index 3a57a3c2c..dec25558f 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> @@ -28,14 +28,21 @@ public:\n>  #ifndef __DOXYGEN__\n>  \t{{struct.mojom_name}}() = default;\n> \n> +\ttemplate<\n> +\t{%- for field in struct.fields %}\n> +\t\ttypename T{{loop.index}} = {{field|name}},\n> +\t{%- endfor %}\n> +\t{%- for field in struct.fields %}\n> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n> +\t{%- endfor %}\n> +\t>\n\nIsn't this overkill for the pod members, shouldn't we restrict this to non-pod ?\n\n>  \t{{struct.mojom_name}}(\n>  {%- for field in struct.fields -%}\n> -{{\"const \" if not field|is_pod}}{{field|name}} {{\"&\" if not field|is_pod}}_{{field.mojom_name}}{{\", \" if not loop.last}}\n> +\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n>  {%- endfor -%}\n\nThere must be something I'm missing. Looking for instance at the\nconstructor of IPABuffer, we get\n\n\ttemplate<\n\t\ttypename T1 = uint32_t,\n\t\ttypename T2 = std::vector<FrameBuffer::Plane>,\n\t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n\t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n\t>\n\tIPABuffer(T1 &&_id, T2 &&_planes)\n\t\t: id(std::forward<T1>(_id))\n\t\t, planes(std::forward<T2>(_planes))\n\t{\n\t}\n\nT1 and T2 are defaulted, and I don't expect to see any code create an\nIPABuffer with explicit template parameter types. How is that therefore\ndifferent from\n\n\tIPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes)\n\t\t: id(std::forward<uint32_t>(_id))\n\t\t, planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes))\n\t{\n\t}\n\n? There's clearly a difference as the latter doesn't compile. Is the\ncompiler allowed to deduce types even when they have a default value ?\nThe following compiles too:\n\n\ttemplate<\n\t\ttypename T1,\n\t\ttypename T2,\n\t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n\t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n\t>\n\tIPABuffer(T1 &&_id, T2 &&_planes)\n\t\t: id(std::forward<T1>(_id))\n\t\t, planes(std::forward<T2>(_planes))\n\t{\n\t}\n\nWhat's the difference ?\n\n>  )\n> -\t\t:\n> -{%- for field in struct.fields -%}\n> -{{\" \" if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{\", \" if not loop.last}}\n> +{%- for field in struct.fields %}\n> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n>  {%- endfor %}\n>  \t{\n>  \t}\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> index 5d70ea6a2..3913eb1fb 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> @@ -16,6 +16,8 @@\n> \n>  {% if has_map %}#include <map>{% endif %}\n>  {% if has_string %}#include <string>{% endif %}\n> +#include <type_traits>\n> +#include <utility>\n>  {% if has_array %}#include <vector>{% endif %}\n> \n>  #include <libcamera/base/flags.h>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4FBF5BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 02:42:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 17248613C5;\n\tTue, 19 Aug 2025 04:42:15 +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 03CE8613C5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 04:42:12 +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 UTF8SMTPSA id 038C6F6;\n\tTue, 19 Aug 2025 04:41:14 +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=\"FGNTnuE0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755571275;\n\tbh=79cMe3w0SUWQdqpEzO+1P5rUA3HtGi2DMQ68TbyJpjg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FGNTnuE0uWWOUufm/PuY5CN9T4IEK4/qIQx9HFyD2axPmIlGopNP2O12PhNnoM31b\n\tRYaK1HlNuXS+XLfE8A9cCVSwyJE2ce2T/QmbUQvdNz8r74BxuGxM4AchHPmEESEJvs\n\ttMmItjalbE+MxAIXLOa1GPim7RvG4deZjuKdPZuU=","Date":"Tue, 19 Aug 2025 05:41:48 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","Message-ID":"<20250819024148.GI5862@pendragon.ideasonboard.com>","References":"<20250815123138.2213654-1-barnabas.pocze@ideasonboard.com>\n\t<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35497,"web_url":"https://patchwork.libcamera.org/comment/35497/","msgid":"<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>","date":"2025-08-19T07:25:43","subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta:\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote:\n>> Forcing the \"non-pod\" members to be initialized from `const T&` is not the\n>> ideal solution because it disallows e.g. move constructors. So generate a\n>> templated constructor, which members to be initialized more freely, e.g.\n>> using move constructors.\n>>\n>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n>> ---\n>> changes in v2:\n>> \t* use `std::is_convertible_v` to avoid accidental use of explicit\n>> \t  constructors/conversion operators\n>> ---\n>>   .../libcamera_templates/core_ipa_interface.h.tmpl |  2 ++\n>>   .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++----\n>>   .../module_ipa_interface.h.tmpl                   |  2 ++\n>>   3 files changed, 15 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>> index 3942e5708..93f988cd9 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>> @@ -16,6 +16,8 @@\n>>\n>>   {% if has_map %}#include <map>{% endif %}\n>>   {% if has_string %}#include <string>{% endif %}\n>> +#include <type_traits>\n>> +#include <utility>\n>>   {% if has_array %}#include <vector>{% endif %}\n>>\n>>   #include <libcamera/controls.h>\n>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>> index 3a57a3c2c..dec25558f 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>> @@ -28,14 +28,21 @@ public:\n>>   #ifndef __DOXYGEN__\n>>   \t{{struct.mojom_name}}() = default;\n>>\n>> +\ttemplate<\n>> +\t{%- for field in struct.fields %}\n>> +\t\ttypename T{{loop.index}} = {{field|name}},\n>> +\t{%- endfor %}\n>> +\t{%- for field in struct.fields %}\n>> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n>> +\t{%- endfor %}\n>> +\t>\n> \n> Isn't this overkill for the pod members, shouldn't we restrict this to non-pod ?\n\nPossibly, but I thought it simpler to handle everything uniformly.\n\n\n> \n>>   \t{{struct.mojom_name}}(\n>>   {%- for field in struct.fields -%}\n>> -{{\"const \" if not field|is_pod}}{{field|name}} {{\"&\" if not field|is_pod}}_{{field.mojom_name}}{{\", \" if not loop.last}}\n>> +\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n>>   {%- endfor -%}\n> \n> There must be something I'm missing. Looking for instance at the\n> constructor of IPABuffer, we get\n> \n> \ttemplate<\n> \t\ttypename T1 = uint32_t,\n> \t\ttypename T2 = std::vector<FrameBuffer::Plane>,\n> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n> \t>\n> \tIPABuffer(T1 &&_id, T2 &&_planes)\n> \t\t: id(std::forward<T1>(_id))\n> \t\t, planes(std::forward<T2>(_planes))\n> \t{\n> \t}\n> \n> T1 and T2 are defaulted, and I don't expect to see any code create an\n> IPABuffer with explicit template parameter types. How is that therefore\n> different from\n> \n> \tIPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes)\n> \t\t: id(std::forward<uint32_t>(_id))\n> \t\t, planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes))\n> \t{\n> \t}\n> \n> ? There's clearly a difference as the latter doesn't compile. Is the\n> compiler allowed to deduce types even when they have a default value ?\n\nThe difference is that in the proposed change, those references are forwarding\nreferences[0]. While in the above code, they are just normal rvalue references,\nso they won't bind, for example, to `const T&`, while the forwarding references will.\n\nThe default template parameters are there in case the type cannot be deduced from\nthe call, such as when an initializer list or similar is used.\n\n[0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references\n\n\nRegards,\nBarnabás Pőcze\n\n> The following compiles too:\n> \n> \ttemplate<\n> \t\ttypename T1,\n> \t\ttypename T2,\n> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n> \t>\n> \tIPABuffer(T1 &&_id, T2 &&_planes)\n> \t\t: id(std::forward<T1>(_id))\n> \t\t, planes(std::forward<T2>(_planes))\n> \t{\n> \t}\n> \n> What's the difference ?\n> \n>>   )\n>> -\t\t:\n>> -{%- for field in struct.fields -%}\n>> -{{\" \" if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{\", \" if not loop.last}}\n>> +{%- for field in struct.fields %}\n>> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n>>   {%- endfor %}\n>>   \t{\n>>   \t}\n>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>> index 5d70ea6a2..3913eb1fb 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>> @@ -16,6 +16,8 @@\n>>\n>>   {% if has_map %}#include <map>{% endif %}\n>>   {% if has_string %}#include <string>{% endif %}\n>> +#include <type_traits>\n>> +#include <utility>\n>>   {% if has_array %}#include <vector>{% endif %}\n>>\n>>   #include <libcamera/base/flags.h>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B948FBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 07:25:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 776C269266;\n\tTue, 19 Aug 2025 09:25:52 +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 3BEA96923C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 09:25:50 +0200 (CEST)","from [192.168.33.20] (185.221.141.188.nat.pool.zt.hu\n\t[185.221.141.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CCA18596;\n\tTue, 19 Aug 2025 09:24:50 +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=\"ArwH6S4z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755588292;\n\tbh=QAQuh8EETXeqSdvdmr/va5Yd1Jmgb//dRgasckUH0i4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=ArwH6S4zclSsfsfdqtoFlijMElwteJppC6mL79MsOgh+JNeOqxM4VCW+C/5UWyALU\n\t6HUrKTP7zqhrNpleJRj9uT+RPm+jnthUrdFTfLIRRrfv0G8cdkkT3Hxw9Rh0SaXJaS\n\tiyepp2FYTYr0MRY1Z+7wo9fJHj3BGnVE/IhOXwto=","Message-ID":"<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>","Date":"Tue, 19 Aug 2025 09:25:43 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","References":"<20250815123138.2213654-1-barnabas.pocze@ideasonboard.com>\n\t<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>\n\t<20250819024148.GI5862@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250819024148.GI5862@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35501,"web_url":"https://patchwork.libcamera.org/comment/35501/","msgid":"<20250819083553.GO5862@pendragon.ideasonboard.com>","date":"2025-08-19T08:35:53","subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote:\n> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta:\n> > On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote:\n> >> Forcing the \"non-pod\" members to be initialized from `const T&` is not the\n> >> ideal solution because it disallows e.g. move constructors. So generate a\n> >> templated constructor, which members to be initialized more freely, e.g.\n> >> using move constructors.\n> >>\n> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> >> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> >> ---\n> >> changes in v2:\n> >> \t* use `std::is_convertible_v` to avoid accidental use of explicit\n> >> \t  constructors/conversion operators\n> >> ---\n> >>   .../libcamera_templates/core_ipa_interface.h.tmpl |  2 ++\n> >>   .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++----\n> >>   .../module_ipa_interface.h.tmpl                   |  2 ++\n> >>   3 files changed, 15 insertions(+), 4 deletions(-)\n> >>\n> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> >> index 3942e5708..93f988cd9 100644\n> >> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> >> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> >> @@ -16,6 +16,8 @@\n> >>\n> >>   {% if has_map %}#include <map>{% endif %}\n> >>   {% if has_string %}#include <string>{% endif %}\n> >> +#include <type_traits>\n> >> +#include <utility>\n> >>   {% if has_array %}#include <vector>{% endif %}\n> >>\n> >>   #include <libcamera/controls.h>\n> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> >> index 3a57a3c2c..dec25558f 100644\n> >> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> >> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> >> @@ -28,14 +28,21 @@ public:\n> >>   #ifndef __DOXYGEN__\n> >>   \t{{struct.mojom_name}}() = default;\n> >>\n> >> +\ttemplate<\n> >> +\t{%- for field in struct.fields %}\n> >> +\t\ttypename T{{loop.index}} = {{field|name}},\n> >> +\t{%- endfor %}\n> >> +\t{%- for field in struct.fields %}\n> >> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n> >> +\t{%- endfor %}\n> >> +\t>\n> > \n> > Isn't this overkill for the pod members, shouldn't we restrict this\n> > to non-pod ?\n> \n> Possibly, but I thought it simpler to handle everything uniformly.\n\nAny impact on either the complexity of the binary resulting from this,\nor the compilation time ? If not, I suppose most people won't look at\nthe generated code, so we could ignore this.\n\n> >>   \t{{struct.mojom_name}}(\n> >>   {%- for field in struct.fields -%}\n> >> -{{\"const \" if not field|is_pod}}{{field|name}} {{\"&\" if not field|is_pod}}_{{field.mojom_name}}{{\", \" if not loop.last}}\n> >> +\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n> >>   {%- endfor -%}\n> > \n> > There must be something I'm missing. Looking for instance at the\n> > constructor of IPABuffer, we get\n> > \n> > \ttemplate<\n> > \t\ttypename T1 = uint32_t,\n> > \t\ttypename T2 = std::vector<FrameBuffer::Plane>,\n> > \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n> > \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n> > \t>\n> > \tIPABuffer(T1 &&_id, T2 &&_planes)\n> > \t\t: id(std::forward<T1>(_id))\n> > \t\t, planes(std::forward<T2>(_planes))\n> > \t{\n> > \t}\n> > \n> > T1 and T2 are defaulted, and I don't expect to see any code create an\n> > IPABuffer with explicit template parameter types. How is that therefore\n> > different from\n> > \n> > \tIPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes)\n> > \t\t: id(std::forward<uint32_t>(_id))\n> > \t\t, planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes))\n> > \t{\n> > \t}\n> > \n> > ? There's clearly a difference as the latter doesn't compile. Is the\n> > compiler allowed to deduce types even when they have a default value ?\n> \n> The difference is that in the proposed change, those references are forwarding\n> references[0]. While in the above code, they are just normal rvalue references,\n> so they won't bind, for example, to `const T&`, while the forwarding references will.\n\nYes, that part I understood.\n\n> The default template parameters are there in case the type cannot be deduced from\n> the call, such as when an initializer list or similar is used.\n\nSo providing default template parameters is just a hint, compilers are\nfree to deduce a different type ? That's the part I wasn't aware of.\n\n> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references\n> \n> > The following compiles too:\n> > \n> > \ttemplate<\n> > \t\ttypename T1,\n> > \t\ttypename T2,\n> > \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n> > \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n> > \t>\n> > \tIPABuffer(T1 &&_id, T2 &&_planes)\n> > \t\t: id(std::forward<T1>(_id))\n> > \t\t, planes(std::forward<T2>(_planes))\n> > \t{\n> > \t}\n> > \n> > What's the difference ?\n> > \n> >>   )\n> >> -\t\t:\n> >> -{%- for field in struct.fields -%}\n> >> -{{\" \" if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{\", \" if not loop.last}}\n> >> +{%- for field in struct.fields %}\n> >> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n> >>   {%- endfor %}\n> >>   \t{\n> >>   \t}\n> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> >> index 5d70ea6a2..3913eb1fb 100644\n> >> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> >> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> >> @@ -16,6 +16,8 @@\n> >>\n> >>   {% if has_map %}#include <map>{% endif %}\n> >>   {% if has_string %}#include <string>{% endif %}\n> >> +#include <type_traits>\n> >> +#include <utility>\n> >>   {% if has_array %}#include <vector>{% endif %}\n> >>\n> >>   #include <libcamera/base/flags.h>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B7A61BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 08:36:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8405769269;\n\tTue, 19 Aug 2025 10:36:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C278661421\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 10:36:15 +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 UTF8SMTPSA id BFBD11A37; \n\tTue, 19 Aug 2025 10:35:17 +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=\"H3UVdOtg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755592518;\n\tbh=7FZRC3vTLy/2qvwuLggRykvJf5weKiZOa1jT19eai8M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H3UVdOtg8k9GmMJmBGYm3bd20N9nrLM0FlLSxyZJh/NdDmbDs2ZZYisZhPfGVRtfR\n\tLS5WhlFt+xikuYJUtr4RGNwCQ74YQ5tdxUAEy4jOUSaaPkzjpdohiXtF54+Dvhyuij\n\tRDKqYobcM/SJMjpqwoCdQh9FPexT6DOW+ZodMVeo=","Date":"Tue, 19 Aug 2025 11:35:53 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","Message-ID":"<20250819083553.GO5862@pendragon.ideasonboard.com>","References":"<20250815123138.2213654-1-barnabas.pocze@ideasonboard.com>\n\t<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>\n\t<20250819024148.GI5862@pendragon.ideasonboard.com>\n\t<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35503,"web_url":"https://patchwork.libcamera.org/comment/35503/","msgid":"<53eac9a9-11dd-4623-bb53-689d472b57c0@ideasonboard.com>","date":"2025-08-19T08:43:38","subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 08. 19. 10:35 keltezéssel, Laurent Pinchart írta:\n> On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote:\n>> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta:\n>>> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote:\n>>>> Forcing the \"non-pod\" members to be initialized from `const T&` is not the\n>>>> ideal solution because it disallows e.g. move constructors. So generate a\n>>>> templated constructor, which members to be initialized more freely, e.g.\n>>>> using move constructors.\n>>>>\n>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>>>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n>>>> ---\n>>>> changes in v2:\n>>>> \t* use `std::is_convertible_v` to avoid accidental use of explicit\n>>>> \t  constructors/conversion operators\n>>>> ---\n>>>>    .../libcamera_templates/core_ipa_interface.h.tmpl |  2 ++\n>>>>    .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++----\n>>>>    .../module_ipa_interface.h.tmpl                   |  2 ++\n>>>>    3 files changed, 15 insertions(+), 4 deletions(-)\n>>>>\n>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>>>> index 3942e5708..93f988cd9 100644\n>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>>>> @@ -16,6 +16,8 @@\n>>>>\n>>>>    {% if has_map %}#include <map>{% endif %}\n>>>>    {% if has_string %}#include <string>{% endif %}\n>>>> +#include <type_traits>\n>>>> +#include <utility>\n>>>>    {% if has_array %}#include <vector>{% endif %}\n>>>>\n>>>>    #include <libcamera/controls.h>\n>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>>>> index 3a57a3c2c..dec25558f 100644\n>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>>>> @@ -28,14 +28,21 @@ public:\n>>>>    #ifndef __DOXYGEN__\n>>>>    \t{{struct.mojom_name}}() = default;\n>>>>\n>>>> +\ttemplate<\n>>>> +\t{%- for field in struct.fields %}\n>>>> +\t\ttypename T{{loop.index}} = {{field|name}},\n>>>> +\t{%- endfor %}\n>>>> +\t{%- for field in struct.fields %}\n>>>> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n>>>> +\t{%- endfor %}\n>>>> +\t>\n>>>\n>>> Isn't this overkill for the pod members, shouldn't we restrict this\n>>> to non-pod ?\n>>\n>> Possibly, but I thought it simpler to handle everything uniformly.\n> \n> Any impact on either the complexity of the binary resulting from this,\n> or the compilation time ? If not, I suppose most people won't look at\n> the generated code, so we could ignore this.\n> \n>>>>    \t{{struct.mojom_name}}(\n>>>>    {%- for field in struct.fields -%}\n>>>> -{{\"const \" if not field|is_pod}}{{field|name}} {{\"&\" if not field|is_pod}}_{{field.mojom_name}}{{\", \" if not loop.last}}\n>>>> +\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n>>>>    {%- endfor -%}\n>>>\n>>> There must be something I'm missing. Looking for instance at the\n>>> constructor of IPABuffer, we get\n>>>\n>>> \ttemplate<\n>>> \t\ttypename T1 = uint32_t,\n>>> \t\ttypename T2 = std::vector<FrameBuffer::Plane>,\n>>> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n>>> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n>>> \t>\n>>> \tIPABuffer(T1 &&_id, T2 &&_planes)\n>>> \t\t: id(std::forward<T1>(_id))\n>>> \t\t, planes(std::forward<T2>(_planes))\n>>> \t{\n>>> \t}\n>>>\n>>> T1 and T2 are defaulted, and I don't expect to see any code create an\n>>> IPABuffer with explicit template parameter types. How is that therefore\n>>> different from\n>>>\n>>> \tIPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes)\n>>> \t\t: id(std::forward<uint32_t>(_id))\n>>> \t\t, planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes))\n>>> \t{\n>>> \t}\n>>>\n>>> ? There's clearly a difference as the latter doesn't compile. Is the\n>>> compiler allowed to deduce types even when they have a default value ?\n>>\n>> The difference is that in the proposed change, those references are forwarding\n>> references[0]. While in the above code, they are just normal rvalue references,\n>> so they won't bind, for example, to `const T&`, while the forwarding references will.\n> \n> Yes, that part I understood.\n> \n>> The default template parameters are there in case the type cannot be deduced from\n>> the call, such as when an initializer list or similar is used.\n> \n> So providing default template parameters is just a hint, compilers are\n> free to deduce a different type ? That's the part I wasn't aware of.\n\nSorry, yes, that's correct.\n\n\n> \n>> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references\n>>\n>>> The following compiles too:\n>>>\n>>> \ttemplate<\n>>> \t\ttypename T1,\n>>> \t\ttypename T2,\n>>> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n>>> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n>>> \t>\n>>> \tIPABuffer(T1 &&_id, T2 &&_planes)\n>>> \t\t: id(std::forward<T1>(_id))\n>>> \t\t, planes(std::forward<T2>(_planes))\n>>> \t{\n>>> \t}\n>>>\n>>> What's the difference ?\n>>>\n>>>>    )\n>>>> -\t\t:\n>>>> -{%- for field in struct.fields -%}\n>>>> -{{\" \" if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{\", \" if not loop.last}}\n>>>> +{%- for field in struct.fields %}\n>>>> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n>>>>    {%- endfor %}\n>>>>    \t{\n>>>>    \t}\n>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>>>> index 5d70ea6a2..3913eb1fb 100644\n>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>>>> @@ -16,6 +16,8 @@\n>>>>\n>>>>    {% if has_map %}#include <map>{% endif %}\n>>>>    {% if has_string %}#include <string>{% endif %}\n>>>> +#include <type_traits>\n>>>> +#include <utility>\n>>>>    {% if has_array %}#include <vector>{% endif %}\n>>>>\n>>>>    #include <libcamera/base/flags.h>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AE6FFBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 08:43:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AECA6926D;\n\tTue, 19 Aug 2025 10:43:43 +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 B3F1861421\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 10:43:41 +0200 (CEST)","from [192.168.33.20] (185.221.141.188.nat.pool.zt.hu\n\t[185.221.141.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4A5A1A37;\n\tTue, 19 Aug 2025 10:42:43 +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=\"jqu47Lb3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755592964;\n\tbh=PaI2+wCBUseF8xq4ihnGdbHjpo7/bAljvYueGVD1stA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=jqu47Lb3JVYAHWmuebyxGA8dxawRQpz5PLB8tzX4efGYcrqjcW2OsGJ9l7QEMw9lA\n\tCi5vHGhsyNBb6LW+qfAmMvetn0XgS+Af7GPokmtkpBHux9Vfqr53w+BirEvMdl6mE9\n\tiHUPzrjiXmKnuPLyxsE44f5vPY/Km3mgmHPCUSOI=","Message-ID":"<53eac9a9-11dd-4623-bb53-689d472b57c0@ideasonboard.com>","Date":"Tue, 19 Aug 2025 10:43:38 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","References":"<20250815123138.2213654-1-barnabas.pocze@ideasonboard.com>\n\t<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>\n\t<20250819024148.GI5862@pendragon.ideasonboard.com>\n\t<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>\n\t<20250819083553.GO5862@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250819083553.GO5862@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35674,"web_url":"https://patchwork.libcamera.org/comment/35674/","msgid":"<20250901135506.GA7124@pendragon.ideasonboard.com>","date":"2025-09-01T13:55:06","subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 19, 2025 at 10:43:38AM +0200, Barnabás Pőcze wrote:\n> 2025. 08. 19. 10:35 keltezéssel, Laurent Pinchart írta:\n> > On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote:\n> >> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta:\n> >>> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote:\n> >>>> Forcing the \"non-pod\" members to be initialized from `const T&` is not the\n> >>>> ideal solution because it disallows e.g. move constructors. So generate a\n> >>>> templated constructor, which members to be initialized more freely, e.g.\n> >>>> using move constructors.\n> >>>>\n> >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> >>>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> >>>> ---\n> >>>> changes in v2:\n> >>>> \t* use `std::is_convertible_v` to avoid accidental use of explicit\n> >>>> \t  constructors/conversion operators\n> >>>> ---\n> >>>>    .../libcamera_templates/core_ipa_interface.h.tmpl |  2 ++\n> >>>>    .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++----\n> >>>>    .../module_ipa_interface.h.tmpl                   |  2 ++\n> >>>>    3 files changed, 15 insertions(+), 4 deletions(-)\n> >>>>\n> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> >>>> index 3942e5708..93f988cd9 100644\n> >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> >>>> @@ -16,6 +16,8 @@\n> >>>>\n> >>>>    {% if has_map %}#include <map>{% endif %}\n> >>>>    {% if has_string %}#include <string>{% endif %}\n> >>>> +#include <type_traits>\n> >>>> +#include <utility>\n> >>>>    {% if has_array %}#include <vector>{% endif %}\n> >>>>\n> >>>>    #include <libcamera/controls.h>\n> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> >>>> index 3a57a3c2c..dec25558f 100644\n> >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> >>>> @@ -28,14 +28,21 @@ public:\n> >>>>    #ifndef __DOXYGEN__\n> >>>>    \t{{struct.mojom_name}}() = default;\n> >>>>\n> >>>> +\ttemplate<\n> >>>> +\t{%- for field in struct.fields %}\n> >>>> +\t\ttypename T{{loop.index}} = {{field|name}},\n> >>>> +\t{%- endfor %}\n> >>>> +\t{%- for field in struct.fields %}\n> >>>> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n> >>>> +\t{%- endfor %}\n> >>>> +\t>\n> >>>\n> >>> Isn't this overkill for the pod members, shouldn't we restrict this\n> >>> to non-pod ?\n> >>\n> >> Possibly, but I thought it simpler to handle everything uniformly.\n> > \n> > Any impact on either the complexity of the binary resulting from this,\n> > or the compilation time ? If not, I suppose most people won't look at\n> > the generated code, so we could ignore this.\n\nI gave it a try. When using rvalue references and std::forward for\nnon-POD arguments only compared to your version, I get a reduction of 54\nbytes in .text and an increase of 32 bytes in .rodata for libcamera.so.\nThe impact on the binary isn't significant.\n\nEither way,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nIf anyone is interested, here's how I implemented the change:\n\ndiff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\nindex fd9a61df100f..22e0af570485 100644\n--- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n+++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n@@ -29,20 +29,34 @@ public:\n \t{{struct.mojom_name}}() = default;\n \n \ttemplate<\n-\t{%- for field in struct.fields %}\n+{%- for field in struct.fields -%}\n+\t{%- if not field|is_pod %}\n \t\ttypename T{{loop.index}} = {{field|name}},\n-\t{%- endfor %}\n-\t{%- for field in struct.fields %}\n-\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n-\t{%- endfor %}\n+\t{%- endif -%}\n+{%- endfor %}\n+{%- for field in struct.fields %}\n+\t{%- if not field|is_pod %}\n+\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr,\n+\t{%- endif -%}\n+{%- endfor %}\n+\t\tbool B = true\n \t>\n \t{{struct.mojom_name}}(\n {%- for field in struct.fields -%}\n-\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n+\t{%- if field|is_pod -%}\n+\t\t{{field|name}} _{{field.mojom_name}}\n+\t{%- else -%}\n+\t\tT{{loop.index}} &&_{{field.mojom_name}}\n+\t{%- endif -%}\n+\t{{\", \" if not loop.last}}\n {%- endfor -%}\n )\n-{%- for field in struct.fields %}\n+{%- for field in struct.fields -%}\n+\t{%- if field|is_pod %}\n+\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(_{{field.mojom_name}})\n+\t{%- else %}\n \t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n+\t{%- endif -%}\n {%- endfor %}\n \t{\n \t}\n\n> >>>>    \t{{struct.mojom_name}}(\n> >>>>    {%- for field in struct.fields -%}\n> >>>> -{{\"const \" if not field|is_pod}}{{field|name}} {{\"&\" if not field|is_pod}}_{{field.mojom_name}}{{\", \" if not loop.last}}\n> >>>> +\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n> >>>>    {%- endfor -%}\n> >>>\n> >>> There must be something I'm missing. Looking for instance at the\n> >>> constructor of IPABuffer, we get\n> >>>\n> >>> \ttemplate<\n> >>> \t\ttypename T1 = uint32_t,\n> >>> \t\ttypename T2 = std::vector<FrameBuffer::Plane>,\n> >>> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n> >>> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n> >>> \t>\n> >>> \tIPABuffer(T1 &&_id, T2 &&_planes)\n> >>> \t\t: id(std::forward<T1>(_id))\n> >>> \t\t, planes(std::forward<T2>(_planes))\n> >>> \t{\n> >>> \t}\n> >>>\n> >>> T1 and T2 are defaulted, and I don't expect to see any code create an\n> >>> IPABuffer with explicit template parameter types. How is that therefore\n> >>> different from\n> >>>\n> >>> \tIPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes)\n> >>> \t\t: id(std::forward<uint32_t>(_id))\n> >>> \t\t, planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes))\n> >>> \t{\n> >>> \t}\n> >>>\n> >>> ? There's clearly a difference as the latter doesn't compile. Is the\n> >>> compiler allowed to deduce types even when they have a default value ?\n> >>\n> >> The difference is that in the proposed change, those references are forwarding\n> >> references[0]. While in the above code, they are just normal rvalue references,\n> >> so they won't bind, for example, to `const T&`, while the forwarding references will.\n> > \n> > Yes, that part I understood.\n> > \n> >> The default template parameters are there in case the type cannot be deduced from\n> >> the call, such as when an initializer list or similar is used.\n> > \n> > So providing default template parameters is just a hint, compilers are\n> > free to deduce a different type ? That's the part I wasn't aware of.\n> \n> Sorry, yes, that's correct.\n> \n> >> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references\n> >>\n> >>> The following compiles too:\n> >>>\n> >>> \ttemplate<\n> >>> \t\ttypename T1,\n> >>> \t\ttypename T2,\n> >>> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n> >>> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n> >>> \t>\n> >>> \tIPABuffer(T1 &&_id, T2 &&_planes)\n> >>> \t\t: id(std::forward<T1>(_id))\n> >>> \t\t, planes(std::forward<T2>(_planes))\n> >>> \t{\n> >>> \t}\n> >>>\n> >>> What's the difference ?\n> >>>\n> >>>>    )\n> >>>> -\t\t:\n> >>>> -{%- for field in struct.fields -%}\n> >>>> -{{\" \" if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{\", \" if not loop.last}}\n> >>>> +{%- for field in struct.fields %}\n> >>>> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n> >>>>    {%- endfor %}\n> >>>>    \t{\n> >>>>    \t}\n> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> >>>> index 5d70ea6a2..3913eb1fb 100644\n> >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> >>>> @@ -16,6 +16,8 @@\n> >>>>\n> >>>>    {% if has_map %}#include <map>{% endif %}\n> >>>>    {% if has_string %}#include <string>{% endif %}\n> >>>> +#include <type_traits>\n> >>>> +#include <utility>\n> >>>>    {% if has_array %}#include <vector>{% endif %}\n> >>>>\n> >>>>    #include <libcamera/base/flags.h>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7EB63BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  1 Sep 2025 13:55:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7919469324;\n\tMon,  1 Sep 2025 15:55:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20D8269323\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Sep 2025 15:55:27 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(230.215-178-91.adsl-dyn.isp.belgacom.be [91.178.215.230])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B79DDE92;\n\tMon,  1 Sep 2025 15:54:19 +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=\"ZMjllzmx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756734859;\n\tbh=YjHioV2ZWVRWH4Dc/MRY9SeGNkojfcKO17LF3acSrCw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZMjllzmxHbD+L8j0C8k2wKH2f1mPLQIOmT8ktxZv+kW+5LLTrEFga388QsA46WP2s\n\tbOXZIqdG9qbo5H8DsOam7btYjql8vnCdqTdmjnUdyW9bTIieDVedYctbX00eezGoaJ\n\tBqil21wFCG+0Kld5HZ2b5k18RaKAyp39LGa/L0yU=","Date":"Mon, 1 Sep 2025 15:55:06 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","Message-ID":"<20250901135506.GA7124@pendragon.ideasonboard.com>","References":"<20250815123138.2213654-1-barnabas.pocze@ideasonboard.com>\n\t<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>\n\t<20250819024148.GI5862@pendragon.ideasonboard.com>\n\t<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>\n\t<20250819083553.GO5862@pendragon.ideasonboard.com>\n\t<53eac9a9-11dd-4623-bb53-689d472b57c0@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<53eac9a9-11dd-4623-bb53-689d472b57c0@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35680,"web_url":"https://patchwork.libcamera.org/comment/35680/","msgid":"<27918376-e2b4-4ce4-a371-75735107c126@ideasonboard.com>","date":"2025-09-01T15:07:18","subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 09. 01. 15:55 keltezéssel, Laurent Pinchart írta:\n> On Tue, Aug 19, 2025 at 10:43:38AM +0200, Barnabás Pőcze wrote:\n>> 2025. 08. 19. 10:35 keltezéssel, Laurent Pinchart írta:\n>>> On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote:\n>>>> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta:\n>>>>> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote:\n>>>>>> Forcing the \"non-pod\" members to be initialized from `const T&` is not the\n>>>>>> ideal solution because it disallows e.g. move constructors. So generate a\n>>>>>> templated constructor, which members to be initialized more freely, e.g.\n>>>>>> using move constructors.\n>>>>>>\n>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>>>>>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n>>>>>> ---\n>>>>>> changes in v2:\n>>>>>> \t* use `std::is_convertible_v` to avoid accidental use of explicit\n>>>>>> \t  constructors/conversion operators\n>>>>>> ---\n>>>>>>     .../libcamera_templates/core_ipa_interface.h.tmpl |  2 ++\n>>>>>>     .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++----\n>>>>>>     .../module_ipa_interface.h.tmpl                   |  2 ++\n>>>>>>     3 files changed, 15 insertions(+), 4 deletions(-)\n>>>>>>\n>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>>>>>> index 3942e5708..93f988cd9 100644\n>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n>>>>>> @@ -16,6 +16,8 @@\n>>>>>>\n>>>>>>     {% if has_map %}#include <map>{% endif %}\n>>>>>>     {% if has_string %}#include <string>{% endif %}\n>>>>>> +#include <type_traits>\n>>>>>> +#include <utility>\n>>>>>>     {% if has_array %}#include <vector>{% endif %}\n>>>>>>\n>>>>>>     #include <libcamera/controls.h>\n>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>>>>>> index 3a57a3c2c..dec25558f 100644\n>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n>>>>>> @@ -28,14 +28,21 @@ public:\n>>>>>>     #ifndef __DOXYGEN__\n>>>>>>     \t{{struct.mojom_name}}() = default;\n>>>>>>\n>>>>>> +\ttemplate<\n>>>>>> +\t{%- for field in struct.fields %}\n>>>>>> +\t\ttypename T{{loop.index}} = {{field|name}},\n>>>>>> +\t{%- endfor %}\n>>>>>> +\t{%- for field in struct.fields %}\n>>>>>> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n>>>>>> +\t{%- endfor %}\n>>>>>> +\t>\n>>>>>\n>>>>> Isn't this overkill for the pod members, shouldn't we restrict this\n>>>>> to non-pod ?\n>>>>\n>>>> Possibly, but I thought it simpler to handle everything uniformly.\n>>>\n>>> Any impact on either the complexity of the binary resulting from this,\n>>> or the compilation time ? If not, I suppose most people won't look at\n>>> the generated code, so we could ignore this.\n> \n> I gave it a try. When using rvalue references and std::forward for\n> non-POD arguments only compared to your version, I get a reduction of 54\n> bytes in .text and an increase of 32 bytes in .rodata for libcamera.so.\n> The impact on the binary isn't significant.\n> \n> Either way,\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nIn that case I think I will go ahead with the version using perfect forwarding\nunconditionally as that is shorter.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> If anyone is interested, here's how I implemented the change:\n> \n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> index fd9a61df100f..22e0af570485 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl\n> @@ -29,20 +29,34 @@ public:\n>   \t{{struct.mojom_name}}() = default;\n>   \n>   \ttemplate<\n> -\t{%- for field in struct.fields %}\n> +{%- for field in struct.fields -%}\n> +\t{%- if not field|is_pod %}\n>   \t\ttypename T{{loop.index}} = {{field|name}},\n> -\t{%- endfor %}\n> -\t{%- for field in struct.fields %}\n> -\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{\",\" if not loop.last}}\n> -\t{%- endfor %}\n> +\t{%- endif -%}\n> +{%- endfor %}\n> +{%- for field in struct.fields %}\n> +\t{%- if not field|is_pod %}\n> +\t\tstd::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr,\n> +\t{%- endif -%}\n> +{%- endfor %}\n> +\t\tbool B = true\n>   \t>\n>   \t{{struct.mojom_name}}(\n>   {%- for field in struct.fields -%}\n> -\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n> +\t{%- if field|is_pod -%}\n> +\t\t{{field|name}} _{{field.mojom_name}}\n> +\t{%- else -%}\n> +\t\tT{{loop.index}} &&_{{field.mojom_name}}\n> +\t{%- endif -%}\n> +\t{{\", \" if not loop.last}}\n>   {%- endfor -%}\n>   )\n> -{%- for field in struct.fields %}\n> +{%- for field in struct.fields -%}\n> +\t{%- if field|is_pod %}\n> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(_{{field.mojom_name}})\n> +\t{%- else %}\n>   \t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n> +\t{%- endif -%}\n>   {%- endfor %}\n>   \t{\n>   \t}\n> \n>>>>>>     \t{{struct.mojom_name}}(\n>>>>>>     {%- for field in struct.fields -%}\n>>>>>> -{{\"const \" if not field|is_pod}}{{field|name}} {{\"&\" if not field|is_pod}}_{{field.mojom_name}}{{\", \" if not loop.last}}\n>>>>>> +\t\tT{{loop.index}} &&_{{field.mojom_name}}{{ \", \" if not loop.last }}\n>>>>>>     {%- endfor -%}\n>>>>>\n>>>>> There must be something I'm missing. Looking for instance at the\n>>>>> constructor of IPABuffer, we get\n>>>>>\n>>>>> \ttemplate<\n>>>>> \t\ttypename T1 = uint32_t,\n>>>>> \t\ttypename T2 = std::vector<FrameBuffer::Plane>,\n>>>>> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n>>>>> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n>>>>> \t>\n>>>>> \tIPABuffer(T1 &&_id, T2 &&_planes)\n>>>>> \t\t: id(std::forward<T1>(_id))\n>>>>> \t\t, planes(std::forward<T2>(_planes))\n>>>>> \t{\n>>>>> \t}\n>>>>>\n>>>>> T1 and T2 are defaulted, and I don't expect to see any code create an\n>>>>> IPABuffer with explicit template parameter types. How is that therefore\n>>>>> different from\n>>>>>\n>>>>> \tIPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes)\n>>>>> \t\t: id(std::forward<uint32_t>(_id))\n>>>>> \t\t, planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes))\n>>>>> \t{\n>>>>> \t}\n>>>>>\n>>>>> ? There's clearly a difference as the latter doesn't compile. Is the\n>>>>> compiler allowed to deduce types even when they have a default value ?\n>>>>\n>>>> The difference is that in the proposed change, those references are forwarding\n>>>> references[0]. While in the above code, they are just normal rvalue references,\n>>>> so they won't bind, for example, to `const T&`, while the forwarding references will.\n>>>\n>>> Yes, that part I understood.\n>>>\n>>>> The default template parameters are there in case the type cannot be deduced from\n>>>> the call, such as when an initializer list or similar is used.\n>>>\n>>> So providing default template parameters is just a hint, compilers are\n>>> free to deduce a different type ? That's the part I wasn't aware of.\n>>\n>> Sorry, yes, that's correct.\n>>\n>>>> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references\n>>>>\n>>>>> The following compiles too:\n>>>>>\n>>>>> \ttemplate<\n>>>>> \t\ttypename T1,\n>>>>> \t\ttypename T2,\n>>>>> \t\tstd::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr,\n>>>>> \t\tstd::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr\n>>>>> \t>\n>>>>> \tIPABuffer(T1 &&_id, T2 &&_planes)\n>>>>> \t\t: id(std::forward<T1>(_id))\n>>>>> \t\t, planes(std::forward<T2>(_planes))\n>>>>> \t{\n>>>>> \t}\n>>>>>\n>>>>> What's the difference ?\n>>>>>\n>>>>>>     )\n>>>>>> -\t\t:\n>>>>>> -{%- for field in struct.fields -%}\n>>>>>> -{{\" \" if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{\", \" if not loop.last}}\n>>>>>> +{%- for field in struct.fields %}\n>>>>>> +\t\t{{\": \" if loop.first else \", \"}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}}))\n>>>>>>     {%- endfor %}\n>>>>>>     \t{\n>>>>>>     \t}\n>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>>>>>> index 5d70ea6a2..3913eb1fb 100644\n>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n>>>>>> @@ -16,6 +16,8 @@\n>>>>>>\n>>>>>>     {% if has_map %}#include <map>{% endif %}\n>>>>>>     {% if has_string %}#include <string>{% endif %}\n>>>>>> +#include <type_traits>\n>>>>>> +#include <utility>\n>>>>>>     {% if has_array %}#include <vector>{% endif %}\n>>>>>>\n>>>>>>     #include <libcamera/base/flags.h>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 99E17BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  1 Sep 2025 15:07:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6953E69332;\n\tMon,  1 Sep 2025 17: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 A42C369323\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Sep 2025 17:07:22 +0200 (CEST)","from [192.168.33.23] (185.221.143.232.nat.pool.zt.hu\n\t[185.221.143.232])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C6216DF3;\n\tMon,  1 Sep 2025 17:06:14 +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=\"QgV0RAun\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756739174;\n\tbh=1FBEGuBYW4KSm5BV7CKQH0ldPTHQkLzdvnww0dTOtOk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=QgV0RAun/OkBeq5WcfoC4Zacd9qwgTC9rngk7t7Zbhrbrc8pMxIxX2JFm6xBDCmEF\n\t9M1hfgUmVdZO/9DHMfrcMog68pz1+oO84h0PESUKFYTKIvJy4LV6MacRjiH5bVLnoR\n\tOwUv/F0SfM9Y9QEtN1dgs5HHnVGpb279tcCW6eOQ=","Message-ID":"<27918376-e2b4-4ce4-a371-75735107c126@ideasonboard.com>","Date":"Mon, 1 Sep 2025 17:07:18 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 3/3] utils: codegen: ipc: Generate templated\n\tconstructor","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","References":"<20250815123138.2213654-1-barnabas.pocze@ideasonboard.com>\n\t<20250815123138.2213654-3-barnabas.pocze@ideasonboard.com>\n\t<20250819024148.GI5862@pendragon.ideasonboard.com>\n\t<762f3bdc-706a-434d-bbf8-72bdbc1e381d@ideasonboard.com>\n\t<20250819083553.GO5862@pendragon.ideasonboard.com>\n\t<53eac9a9-11dd-4623-bb53-689d472b57c0@ideasonboard.com>\n\t<20250901135506.GA7124@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250901135506.GA7124@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]