[{"id":15145,"web_url":"https://patchwork.libcamera.org/comment/15145/","msgid":"","date":"2021-02-12T14:07:33","subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move\n\tand copy disablers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Fri, Feb 12, 2021 at 01:30:51PM +0000, Kieran Bingham wrote:\n> It can be difficult to correctly parse the syntax for copy/move and the\n> associated assignment operators.\n> \n> Provide helpers as syntactic sugar to facilitate disabling either the\n> copy constructor, and copy assignment operator, and the move constructor\n> and move assignment operator in a way which is explicit and clear.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/class.h | 18 +++++++++++++\n> src/libcamera/class.cpp | 57 +++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 75 insertions(+)\n> \n> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n> index cb278e58204a..920624d8e726 100644\n> --- a/include/libcamera/class.h\n> +++ b/include/libcamera/class.h\n> @@ -11,6 +11,24 @@\n> \n> namespace libcamera {\n> \n> +#ifndef __DOXYGEN__\n> +#define LIBCAMERA_DISABLE_COPY(klass) \\\n> +\tklass(const klass &) = delete; \\\n\nPerhaps the \\ could be aligned with tabs, with the same indentation for\nall macros ? We don't have a coding style rule for this, so you can pick\nwhat you like best :-)\n\n> +\tklass &operator=(const klass &) = delete;\n> +\n> +#define LIBCAMERA_DISABLE_MOVE(klass) \\\n> +\tklass(klass &&) = delete; \\\n> +\tklass &operator=(klass &&) = delete;\n> +\n> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass) \\\n> +\tLIBCAMERA_DISABLE_COPY(klass) \\\n> +\tLIBCAMERA_DISABLE_MOVE(klass)\n> +#else\n> +#define LIBCAMERA_DISABLE_COPY(klass)\n> +#define LIBCAMERA_DISABLE_MOVE(klass)\n> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass)\n> +#endif\n\nYou could merge this with the next block to avoid two #ifndef\n__DOXYGEN__, up to you.\n\n> +\n> #ifndef __DOXYGEN__\n> #define LIBCAMERA_DECLARE_PRIVATE(klass)\t\t\t\t\\\n> public:\t\t\t\t\t\t\t\t\t\\\n> diff --git a/src/libcamera/class.cpp b/src/libcamera/class.cpp\n> index ce230be91e61..c1db14197b4e 100644\n> --- a/src/libcamera/class.cpp\n> +++ b/src/libcamera/class.cpp\n> @@ -17,6 +17,63 @@\n> \n> namespace libcamera {\n> \n> +/**\n> + * \\def LIBCAMERA_DISABLE_COPY\n> + * \\brief Disable copy construction and assignment of the \\a klass\n> + * \\param klass The name of the class\n> + *\n> + * Example usage:\n> + * \\code{.cpp}\n> + * class NonCopyable\n> + * {\n> + * public:\n> + * \tNonCopyable();\n> + * \t...\n> + *\n> + * private:\n> + * \tLIBCAMERA_DISABLE_COPY(NonCopyable)\n> + * };\n> + * \\endcode\n> + */\n> +\n> +/**\n> + * \\def LIBCAMERA_DISABLE_MOVE\n> + * \\brief Disable move construction and assignment of the \\a klass\n> + * \\param klass The name of the class\n> + *\n> + * Example usage:\n> + * \\code{.cpp}\n> + * class NonMoveable\n> + * {\n> + * public:\n> + * \tNonMoveable();\n> + * \t...\n> + *\n> + * private:\n> + * \tLIBCAMERA_DISABLE_MOVE(NonMoveable)\n> + * };\n> + * \\endcode\n> + */\n> +\n> +/**\n> + * \\def LIBCAMERA_DISABLE_COPY_AND_MOVE\n> + * \\brief Disable copy and move construction and assignment of the \\a klass\n> +* \\param klass The name of the class\n\nMissing space at the beginning of the line.\n\nReviewed-by: Laurent Pinchart \n\n> + *\n> + * Example usage:\n> + * \\code{.cpp}\n> + * class NonCopyableNonMoveable\n> + * {\n> + * public:\n> + * \tNonCopyableNonMoveable();\n> + * \t...\n> + *\n> + * private:\n> + * \tLIBCAMERA_DISABLE_COPY_AND_MOVE(NonCopyableNonMoveable)\n> + * };\n> + * \\endcode\n> + */\n> +\n> /**\n> * \\def LIBCAMERA_DECLARE_PRIVATE\n> * \\brief Declare private data for a public class","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 AC4D2BD160\n\tfor ;\n\tFri, 12 Feb 2021 14:08:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 362236378A;\n\tFri, 12 Feb 2021 15:08:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96C8463781\n\tfor ;\n\tFri, 12 Feb 2021 15:07:59 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F36EB8B5;\n\tFri, 12 Feb 2021 15:07:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IMMqRrHX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613138879;\n\tbh=5SKeA2kFgPD/dHg2bUDYQOsmgeD5Z+3kW++Y5xRf+Og=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IMMqRrHXZoHJ7dNedHwzRI7J6GUn2uFr5Jsez3hL+ie1EKEDdUZWoCfc/KzbSpDPY\n\t0ySTNKjTTWd5zHVv8GFzAsc6+ZFXDSrKMHIJgJcIe4z9raEsUfjxHUjjIwQ/vO3M7U\n\txEJYbWxEYZW8T0OSwLK3LafUOplhI2I0FbLNn794=","Date":"Fri, 12 Feb 2021 16:07:33 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210212133056.873230-1-kieran.bingham@ideasonboard.com>\n\t<20210212133056.873230-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210212133056.873230-3-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move\n\tand copy disablers","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15150,"web_url":"https://patchwork.libcamera.org/comment/15150/","msgid":"<98bf5882-0af1-2918-6d62-d20a2cf85482@ideasonboard.com>","date":"2021-02-12T14:34:25","subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move\n\tand copy disablers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 12/02/2021 14:07, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Fri, Feb 12, 2021 at 01:30:51PM +0000, Kieran Bingham wrote:\n>> It can be difficult to correctly parse the syntax for copy/move and the\n>> associated assignment operators.\n>>\n>> Provide helpers as syntactic sugar to facilitate disabling either the\n>> copy constructor, and copy assignment operator, and the move constructor\n>> and move assignment operator in a way which is explicit and clear.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> include/libcamera/class.h | 18 +++++++++++++\n>> src/libcamera/class.cpp | 57 +++++++++++++++++++++++++++++++++++++++\n>> 2 files changed, 75 insertions(+)\n>>\n>> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n>> index cb278e58204a..920624d8e726 100644\n>> --- a/include/libcamera/class.h\n>> +++ b/include/libcamera/class.h\n>> @@ -11,6 +11,24 @@\n>> \n>> namespace libcamera {\n>> \n>> +#ifndef __DOXYGEN__\n>> +#define LIBCAMERA_DISABLE_COPY(klass) \\\n>> +\tklass(const klass &) = delete; \\\n> \n> Perhaps the \\ could be aligned with tabs, with the same indentation for\n> all macros ? We don't have a coding style rule for this, so you can pick\n> what you like best :-)\n\nCheckstyle/clang-format really seems to want to pull those in ;-)\nWhich implies we 'do' have a coding style already.\n\nIt seems that although we don't have\n AlignEscapedNewlines\n\ndefined in .clang-format (it's commented out), it is defaulting to 'left'.\n\nI've also tried to set this to 'right' expecting it to match as you have\nthe other macros but it seems identical to left.\n\nPersonally, for anything whitespace/trivial, I'd really like the tools\nto define the rules. So I'd like to go with what clang-format generates,\nand that way further contributions will be consistent.\n\n\n\n\n\n>> +\tklass &operator=(const klass &) = delete;\n>> +\n>> +#define LIBCAMERA_DISABLE_MOVE(klass) \\\n>> +\tklass(klass &&) = delete; \\\n>> +\tklass &operator=(klass &&) = delete;\n>> +\n>> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass) \\\n>> +\tLIBCAMERA_DISABLE_COPY(klass) \\\n>> +\tLIBCAMERA_DISABLE_MOVE(klass)\n>> +#else\n>> +#define LIBCAMERA_DISABLE_COPY(klass)\n>> +#define LIBCAMERA_DISABLE_MOVE(klass)\n>> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass)\n>> +#endif\n> \n> You could merge this with the next block to avoid two #ifndef\n> __DOXYGEN__, up to you.\n> \n\nI specifically chose this to keep them in separate groups.\n\n\n>> +\n>> #ifndef __DOXYGEN__\n>> #define LIBCAMERA_DECLARE_PRIVATE(klass)\t\t\t\t\\\n>> public:\t\t\t\t\t\t\t\t\t\\\n>> diff --git a/src/libcamera/class.cpp b/src/libcamera/class.cpp\n>> index ce230be91e61..c1db14197b4e 100644\n>> --- a/src/libcamera/class.cpp\n>> +++ b/src/libcamera/class.cpp\n>> @@ -17,6 +17,63 @@\n>> \n>> namespace libcamera {\n>> \n>> +/**\n>> + * \\def LIBCAMERA_DISABLE_COPY\n>> + * \\brief Disable copy construction and assignment of the \\a klass\n>> + * \\param klass The name of the class\n>> + *\n>> + * Example usage:\n>> + * \\code{.cpp}\n>> + * class NonCopyable\n>> + * {\n>> + * public:\n>> + * \tNonCopyable();\n>> + * \t...\n>> + *\n>> + * private:\n>> + * \tLIBCAMERA_DISABLE_COPY(NonCopyable)\n>> + * };\n>> + * \\endcode\n>> + */\n>> +\n>> +/**\n>> + * \\def LIBCAMERA_DISABLE_MOVE\n>> + * \\brief Disable move construction and assignment of the \\a klass\n>> + * \\param klass The name of the class\n>> + *\n>> + * Example usage:\n>> + * \\code{.cpp}\n>> + * class NonMoveable\n>> + * {\n>> + * public:\n>> + * \tNonMoveable();\n>> + * \t...\n>> + *\n>> + * private:\n>> + * \tLIBCAMERA_DISABLE_MOVE(NonMoveable)\n>> + * };\n>> + * \\endcode\n>> + */\n>> +\n>> +/**\n>> + * \\def LIBCAMERA_DISABLE_COPY_AND_MOVE\n>> + * \\brief Disable copy and move construction and assignment of the \\a klass\n>> +* \\param klass The name of the class\n> \n> Missing space at the beginning of the line.\n\nArgh. added.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n>> + *\n>> + * Example usage:\n>> + * \\code{.cpp}\n>> + * class NonCopyableNonMoveable\n>> + * {\n>> + * public:\n>> + * \tNonCopyableNonMoveable();\n>> + * \t...\n>> + *\n>> + * private:\n>> + * \tLIBCAMERA_DISABLE_COPY_AND_MOVE(NonCopyableNonMoveable)\n>> + * };\n>> + * \\endcode\n>> + */\n>> +\n>> /**\n>> * \\def LIBCAMERA_DECLARE_PRIVATE\n>> * \\brief Declare private data for a public class\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 5D327BD162\n\tfor ;\n\tFri, 12 Feb 2021 14:34:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1A946378A;\n\tFri, 12 Feb 2021 15:34:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6ECA663781\n\tfor ;\n\tFri, 12 Feb 2021 15:34:28 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DC1828B5;\n\tFri, 12 Feb 2021 15:34:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZHwuf6j2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613140468;\n\tbh=7lpjMDZmrBSdMXIMs5Wcd8C9l8wFHYGn17bu75kq+n4=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=ZHwuf6j2DmPkzmDns3DmfPPzZ/RIPryDeNjxLIACic9wMetKLsKCiGtdY8rM8rZIR\n\tOOwvdI9unsreGwStOUvabmqSCwMCx9Cxx1iJFBI2NnTxDHzoWUaWAkQLHPEoPZLNwn\n\txsSn6mh9LbcexXyqORA6ZBFJ9FkzWjx5v1bKIOZE=","To":"Laurent Pinchart ","References":"<20210212133056.873230-1-kieran.bingham@ideasonboard.com>\n\t<20210212133056.873230-3-kieran.bingham@ideasonboard.com>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<98bf5882-0af1-2918-6d62-d20a2cf85482@ideasonboard.com>","Date":"Fri, 12 Feb 2021 14:34:25 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move\n\tand copy disablers","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15151,"web_url":"https://patchwork.libcamera.org/comment/15151/","msgid":"","date":"2021-02-12T14:52:24","subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move\n\tand copy disablers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Feb 12, 2021 at 02:34:25PM +0000, Kieran Bingham wrote:\n> On 12/02/2021 14:07, Laurent Pinchart wrote:\n> > On Fri, Feb 12, 2021 at 01:30:51PM +0000, Kieran Bingham wrote:\n> >> It can be difficult to correctly parse the syntax for copy/move and the\n> >> associated assignment operators.\n> >>\n> >> Provide helpers as syntactic sugar to facilitate disabling either the\n> >> copy constructor, and copy assignment operator, and the move constructor\n> >> and move assignment operator in a way which is explicit and clear.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> include/libcamera/class.h | 18 +++++++++++++\n> >> src/libcamera/class.cpp | 57 +++++++++++++++++++++++++++++++++++++++\n> >> 2 files changed, 75 insertions(+)\n> >>\n> >> diff --git a/include/libcamera/class.h b/include/libcamera/class.h\n> >> index cb278e58204a..920624d8e726 100644\n> >> --- a/include/libcamera/class.h\n> >> +++ b/include/libcamera/class.h\n> >> @@ -11,6 +11,24 @@\n> >> \n> >> namespace libcamera {\n> >> \n> >> +#ifndef __DOXYGEN__\n> >> +#define LIBCAMERA_DISABLE_COPY(klass) \\\n> >> +\tklass(const klass &) = delete; \\\n> > \n> > Perhaps the \\ could be aligned with tabs, with the same indentation for\n> > all macros ? We don't have a coding style rule for this, so you can pick\n> > what you like best :-)\n> \n> Checkstyle/clang-format really seems to want to pull those in ;-)\n> Which implies we 'do' have a coding style already.\n> \n> It seems that although we don't have\n> AlignEscapedNewlines\n> \n> defined in .clang-format (it's commented out), it is defaulting to 'left'.\n> \n> I've also tried to set this to 'right' expecting it to match as you have\n> the other macros but it seems identical to left.\n\nThat's because we don't set ColumnLimit.\n\nThere are quite a few new option for clang-format, our configuration\nfile needs a brush up. I'll give it a go, you can ignore the issue here\nfor now.\n\n> Personally, for anything whitespace/trivial, I'd really like the tools\n> to define the rules. So I'd like to go with what clang-format generates,\n> and that way further contributions will be consistent.\n> \n> >> +\tklass &operator=(const klass &) = delete;\n> >> +\n> >> +#define LIBCAMERA_DISABLE_MOVE(klass) \\\n> >> +\tklass(klass &&) = delete; \\\n> >> +\tklass &operator=(klass &&) = delete;\n> >> +\n> >> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass) \\\n> >> +\tLIBCAMERA_DISABLE_COPY(klass) \\\n> >> +\tLIBCAMERA_DISABLE_MOVE(klass)\n> >> +#else\n> >> +#define LIBCAMERA_DISABLE_COPY(klass)\n> >> +#define LIBCAMERA_DISABLE_MOVE(klass)\n> >> +#define LIBCAMERA_DISABLE_COPY_AND_MOVE(klass)\n> >> +#endif\n> > \n> > You could merge this with the next block to avoid two #ifndef\n> > __DOXYGEN__, up to you.\n> > \n> \n> I specifically chose this to keep them in separate groups.\n> \n> >> +\n> >> #ifndef __DOXYGEN__\n> >> #define LIBCAMERA_DECLARE_PRIVATE(klass)\t\t\t\t\\\n> >> public:\t\t\t\t\t\t\t\t\t\\\n> >> diff --git a/src/libcamera/class.cpp b/src/libcamera/class.cpp\n> >> index ce230be91e61..c1db14197b4e 100644\n> >> --- a/src/libcamera/class.cpp\n> >> +++ b/src/libcamera/class.cpp\n> >> @@ -17,6 +17,63 @@\n> >> \n> >> namespace libcamera {\n> >> \n> >> +/**\n> >> + * \\def LIBCAMERA_DISABLE_COPY\n> >> + * \\brief Disable copy construction and assignment of the \\a klass\n> >> + * \\param klass The name of the class\n> >> + *\n> >> + * Example usage:\n> >> + * \\code{.cpp}\n> >> + * class NonCopyable\n> >> + * {\n> >> + * public:\n> >> + * \tNonCopyable();\n> >> + * \t...\n> >> + *\n> >> + * private:\n> >> + * \tLIBCAMERA_DISABLE_COPY(NonCopyable)\n> >> + * };\n> >> + * \\endcode\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def LIBCAMERA_DISABLE_MOVE\n> >> + * \\brief Disable move construction and assignment of the \\a klass\n> >> + * \\param klass The name of the class\n> >> + *\n> >> + * Example usage:\n> >> + * \\code{.cpp}\n> >> + * class NonMoveable\n> >> + * {\n> >> + * public:\n> >> + * \tNonMoveable();\n> >> + * \t...\n> >> + *\n> >> + * private:\n> >> + * \tLIBCAMERA_DISABLE_MOVE(NonMoveable)\n> >> + * };\n> >> + * \\endcode\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def LIBCAMERA_DISABLE_COPY_AND_MOVE\n> >> + * \\brief Disable copy and move construction and assignment of the \\a klass\n> >> +* \\param klass The name of the class\n> > \n> > Missing space at the beginning of the line.\n> \n> Argh. added.\n> \n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> >> + *\n> >> + * Example usage:\n> >> + * \\code{.cpp}\n> >> + * class NonCopyableNonMoveable\n> >> + * {\n> >> + * public:\n> >> + * \tNonCopyableNonMoveable();\n> >> + * \t...\n> >> + *\n> >> + * private:\n> >> + * \tLIBCAMERA_DISABLE_COPY_AND_MOVE(NonCopyableNonMoveable)\n> >> + * };\n> >> + * \\endcode\n> >> + */\n> >> +\n> >> /**\n> >> * \\def LIBCAMERA_DECLARE_PRIVATE\n> >> * \\brief Declare private data for a public class","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 2B9AABD160\n\tfor ;\n\tFri, 12 Feb 2021 14:52:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A8F2463790;\n\tFri, 12 Feb 2021 15:52:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EA88663786\n\tfor ;\n\tFri, 12 Feb 2021 15:52:50 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 685DC8B5;\n\tFri, 12 Feb 2021 15:52:50 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UqRvx/zE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613141570;\n\tbh=A4Cwtu9N2VZCFAOWa1Ylw+nIspyL2s3lxZcklc92Lo8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UqRvx/zE0UMogPkuvOpXkoWz9PVP390J7oUe17ZcA6mygsKRYVMofouoeDJmgjf+Y\n\tDAKtp1YR83AYM5gofpM8Ew07Ka82xChGIbLU/fyWfKGeR11HgZ8rSZkTld2P7EgD3W\n\tCRB3oSCrMWXcQl6bdZceqQJ/Yh5SWUDwmPLZO/t4=","Date":"Fri, 12 Feb 2021 16:52:24 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210212133056.873230-1-kieran.bingham@ideasonboard.com>\n\t<20210212133056.873230-3-kieran.bingham@ideasonboard.com>\n\t\n\t<98bf5882-0af1-2918-6d62-d20a2cf85482@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<98bf5882-0af1-2918-6d62-d20a2cf85482@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: class: Provide move\n\tand copy disablers","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]