{"id":11362,"url":"https://patchwork.libcamera.org/api/1.1/patches/11362/?format=json","web_url":"https://patchwork.libcamera.org/patch/11362/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210222111531.25845-1-laurent.pinchart@ideasonboard.com>","date":"2021-02-22T11:15:31","name":"[libcamera-devel,PATCH/RFC] libcamera: controls: Disable const move constructor and assignement","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"edd0e56d9b341f3eb1c61190163f24965b1ea45c","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":{"id":14,"url":"https://patchwork.libcamera.org/api/1.1/users/14/?format=json","username":"pinchartl","first_name":"Laurent","last_name":"Pinchart","email":"laurent.pinchart@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/11362/mbox/","series":[{"id":1718,"url":"https://patchwork.libcamera.org/api/1.1/series/1718/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1718","date":"2021-02-22T11:15:31","name":"[libcamera-devel,PATCH/RFC] libcamera: controls: Disable const move constructor and assignement","version":1,"mbox":"https://patchwork.libcamera.org/series/1718/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/11362/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/11362/checks/","tags":{},"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 31AA4BD1F6\n\tfor ;\n\tMon, 22 Feb 2021 11:16:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 911A168A04;\n\tMon, 22 Feb 2021 12:16:02 +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 17435637DA\n\tfor ;\n\tMon, 22 Feb 2021 12:16:01 +0100 (CET)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B1BD36B5; \n\tMon, 22 Feb 2021 12:16:00 +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=\"lVxH4tfN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613992560;\n\tbh=C8j6BEt5RObCy9Qwbi9q3ZIrHzyI0XIRHb6/GIXwxBQ=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=lVxH4tfNinFkDa5fvefYsj6hWK3XeuJkU4SNMcU0KpULVUq04SPhjWjoDcEgrLrY+\n\tB5rYtCbxaSZ3jfzgI+kvdA13j8lX1ga0baKVgoCh//LSz4L9KUrTdDrth+oohwReok\n\ttaDEPeK0Hb2jE8d0giK+EBctYF90TWTCcvbFxybo=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 22 Feb 2021 13:15:31 +0200","Message-Id":"<20210222111531.25845-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.28.0","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH/RFC] libcamera: controls: Disable const\n\tmove constructor and assignement","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"C++ supports const rvalue references. They are of dubious use, as the\npurpose of rvalue references is to express move semantics, and moving a\nconst object is not possible, but they nonetheless exist. This means\nthat calling std::move() on a const reference will not produce an error,\nbut return a const rvalue reference.\n\nFurthermore, a const lvalue reference can bind to a const rvalue. This\nmeans that overload resolution will select an overload of a function\nthat takes a const lvalue reference (such as the copy constructor) when\npassed a const rvalue reference (such as the return value of std::move()\ncalled on a const reference) if no overload taking a const rvalue\nreference exists.\n\nThe consequence of this is that the following code\n\n\tconst ControlList &ctrls = ...;\n\t...\n\tControlList copy(std::move(ctrls));\n\nwill select the ControlList copy constructor and thus compile, when one\nmay expect the object to be moved from the use of std::move() (there is,\non a side note, no guarantee in the language that a move constructor\nwill move anything).\n\nTo make the compiler reject usage of std::move() in this case, define\nthe copy and move constructor and assignment operators explicitly,\ndefaulting the non-const version of the move constructor and assignment\noperator, but deleting the const version. The compiler will select the\nconst move version as it is declared, and generate an error as it is\ndeleted.\n\nSigned-off-by: Laurent Pinchart \n---\n include/libcamera/controls.h | 8 ++++++++\n src/libcamera/controls.cpp | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\nI'm sending this as an RFC as I'm not sure such an explicit approach is\nworth it. We'd have to apply it to all classes that can be copied and\nmoved, which will generate quite a bit of churn (especially in the\ndocumentation, although we could surround the functions in the header\nwith an #ifndef __DOXYGEN__). Maybe a macro would make sense, something\nalong the lines of\n\n#define LIBCAMERA_COPYABLE_MOVEABLE(klass) \\\n\tklass(const klass &other) = default; \\\n\tklass(klass &&other) = default; \\\n\tklass(const klass &&other) = delete; \\\n\tklass &operator=(const klass &other) = default; \\\n\tklass &operator=(klass &&other) = default; \\\n\tklass &operator=(const klass &&other) = delete;\n\n(that would however not work with #ifndef __DOXYGEN__, but it may be\npossible to create a macro for the documentation too).\n\nThere's of course also the possibility that I'm missing something, and\nthat the silent use of the copy constructor when called with a const\nrvalue is actually a useful feature.","diff":"diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 1a5690a5ccbe..fe4b159eb172 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -353,6 +353,14 @@ public:\n \tControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr);\n \tControlList(const ControlInfoMap &infoMap, ControlValidator *validator = nullptr);\n \n+\tControlList(const ControlList &other) = default;\n+\tControlList(ControlList &&other) = default;\n+\tControlList(const ControlList &&other) = delete;\n+\n+\tControlList &operator=(const ControlList &other) = default;\n+\tControlList &operator=(ControlList &&other) = default;\n+\tControlList &operator=(const ControlList &&other) = delete;\n+\n \tusing iterator = ControlListMap::iterator;\n \tusing const_iterator = ControlListMap::const_iterator;\n \ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex c58ed3946f3b..53957322c505 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -821,6 +821,36 @@ ControlList::ControlList(const ControlInfoMap &infoMap, ControlValidator *valida\n {\n }\n \n+/**\n+ * \\fn ControlList::ControlList(const ControlList &other)\n+ * \\brief Copy constructor, construct a ControlList by copying \\a other\n+ * \\param[in] other The ControlList to copy content from\n+ */\n+\n+/**\n+ * \\fn ControlList::ControlList(ControlList &&other)\n+ * \\brief Move constructor, construct a ControlList by moving \\a other\n+ * \\param[in] other The ControlList to move content from\n+ *\n+ * Upon return the \\a other ControlList is empty.\n+ */\n+\n+/**\n+ * \\fn ControlList::operator=(const ControlList &other)\n+ * \\brief Copy assignment operator, replace the content of the ControlList with\n+ * a copy of \\a other\n+ * \\param[in] other The ControlList to copy content from\n+ */\n+\n+/**\n+ * \\fn ControlList::operator=(ControlList &&other)\n+ * \\brief Move assignment operator, replace the content of the ControlList with\n+ * the content of \\a other\n+ * \\param[in] other The ControlList to move content from\n+ *\n+ * Upon return the \\a other ControlList is empty.\n+ */\n+\n /**\n * \\typedef ControlList::iterator\n * \\brief Iterator for the controls contained within the list\n","prefixes":["libcamera-devel","PATCH/RFC"]}