{"id":12087,"url":"https://patchwork.libcamera.org/api/1.1/patches/12087/?format=json","web_url":"https://patchwork.libcamera.org/patch/12087/","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":"<20210423020932.2760-3-laurent.pinchart@ideasonboard.com>","date":"2021-04-23T02:09:31","name":"[libcamera-devel,PATCH/RFC,2/3] libcamera: utils: enumerate: Use named fields for result","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"e3b89ade3a3a5eeceef806b4b916c14704a38570","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/12087/mbox/","series":[{"id":1966,"url":"https://patchwork.libcamera.org/api/1.1/series/1966/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1966","date":"2021-04-23T02:09:29","name":"libcamera: Simplify range-based for loop counters","version":1,"mbox":"https://patchwork.libcamera.org/series/1966/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12087/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12087/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 2D8E0BDB70\n\tfor ;\n\tFri, 23 Apr 2021 02:09:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A39468876;\n\tFri, 23 Apr 2021 04:09:45 +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 D1D7968861\n\tfor ;\n\tFri, 23 Apr 2021 04:09:41 +0200 (CEST)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 69090BB4\n\tfor ;\n\tFri, 23 Apr 2021 04:09:41 +0200 (CEST)"],"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=\"WdVg2FZh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619143781;\n\tbh=fOCXAOCwyja2xqpQr1Uu+VhZq21pbLFFRj4MerDiJSc=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=WdVg2FZho6e6R/SsJKZ6759slxGynfDY//piOn6ZKmWs13mL5seh7OUIqusA8ztuC\n\tp3FTKS/2C+9P1ogsc7dxQ8ErZsV8AW1JCEXHM4nTGvEkmZzXFih026L8NLZt3eqpd4\n\taincSHwhwm6Omv1uKx6Mafzuo3zXBxu0lE0nYT+E=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 23 Apr 2021 05:09:31 +0300","Message-Id":"<20210423020932.2760-3-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.28.1","In-Reply-To":"<20210423020932.2760-1-laurent.pinchart@ideasonboard.com>","References":"<20210423020932.2760-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH/RFC 2/3] libcamera: utils: enumerate: Use\n\tnamed fields for result","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":"Returning a pair of { index, value } from container enumeration is\nerror-prone, as the index and value can easily be swapped. Use a\nstructure with named index and value fields instead.\n\nSigned-off-by: Laurent Pinchart \n---\n include/libcamera/internal/utils.h | 16 +++++++++++-----\n src/libcamera/utils.cpp | 13 +++++++------\n test/utils.cpp | 24 ++++++++++++------------\n 3 files changed, 30 insertions(+), 23 deletions(-)","diff":"diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h\nindex 9372a75889ac..54a6b6d283fb 100644\n--- a/include/libcamera/internal/utils.h\n+++ b/include/libcamera/internal/utils.h\n@@ -16,7 +16,6 @@\n #include \n #include \n #include \n-#include \n #include \n \n #ifndef __DOXYGEN__\n@@ -237,12 +236,19 @@ namespace details {\n template\n class enumerate_iterator\n {\n-private:\n-\tusing base_reference = typename std::iterator_traits::reference;\n-\n public:\n \tusing difference_type = typename std::iterator_traits::difference_type;\n-\tusing value_type = std::pair;\n+\n+private:\n+\tusing base_reference = typename std::iterator_traits::reference;\n+\n+\tstruct result {\n+\t\tconst difference_type index;\n+\t\tbase_reference value;\n+\t};\n+\n+public:\n+\tusing value_type = result;\n \tusing pointer = value_type *;\n \tusing reference = value_type &;\n \tusing iterator_category = typename std::iterator_traits::iterator_category;\ndiff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\nindex ff9a5832b10e..b4b4180c1337 100644\n--- a/src/libcamera/utils.cpp\n+++ b/src/libcamera/utils.cpp\n@@ -481,20 +481,21 @@ std::string libcameraSourcePath()\n * in their ability to replace for loops that require access to a loop counter.\n * The enumerate() function solves this problem by wrapping the \\a iterable in\n * an adapter that, when used as a range-expression, will provide iterators\n- * whose value_type is a pair of index and value reference.\n+ * whose value_type exposes both the element's value and its index.\n *\n * The iterable must support std::begin() and std::end(). This includes all\n * containers provided by the standard C++ library, as well as C-style arrays.\n *\n- * A typical usage pattern would use structured binding to store the index and\n- * value in two separate variables:\n+ * The iterator's value_type is a structure that aggregates the index and value\n+ * reference in two named members. The index is always const, and the value\n+ * reference is conditionally const depending on the iterable. A typical usage\n+ * pattern would be:\n *\n * \\code{.cpp}\n * std::vector values = ...;\n *\n- * for (auto [index, value] : utils::enumerate(values)) {\n- * \t...\n- * }\n+ * for (const auto v : utils::enumerate(values))\n+ * \tstd::cout << \"- index \" << v.index << \", value \" << v.value << std::endl;\n * \\endcode\n *\n * \\return A value of unspecified type that, when used in a range-based for\ndiff --git a/test/utils.cpp b/test/utils.cpp\nindex 7e24c71e4775..06ce5301a74e 100644\n--- a/test/utils.cpp\n+++ b/test/utils.cpp\n@@ -79,16 +79,16 @@ protected:\n \t\tstd::vector integers{ 1, 2, 3, 4, 5 };\n \t\tint i = 0;\n \n-\t\tfor (auto [index, value] : utils::enumerate(integers)) {\n-\t\t\tif (index != i || value != i + 1) {\n+\t\tfor (const auto v : utils::enumerate(integers)) {\n+\t\t\tif (v.index != i || v.value != i + 1) {\n \t\t\t\tcerr << \"utils::enumerate() test failed: i=\" << i\n-\t\t\t\t << \", index=\" << index << \", value=\" << value\n-\t\t\t\t << std::endl;\n+\t\t\t\t << \", index=\" << v.index << \", value=\" << v.value\n+\t\t\t\t << endl;\n \t\t\t\treturn TestFail;\n \t\t\t}\n \n \t\t\t/* Verify that we can modify the value. */\n-\t\t\t--value;\n+\t\t\t--v.value;\n \t\t\t++i;\n \t\t}\n \n@@ -100,10 +100,10 @@ protected:\n \t\tSpan span{ integers };\n \t\ti = 0;\n \n-\t\tfor (auto [index, value] : utils::enumerate(span)) {\n-\t\t\tif (index != i || value != i) {\n+\t\tfor (const auto v : utils::enumerate(span)) {\n+\t\t\tif (v.index != i || v.value != i) {\n \t\t\t\tcerr << \"utils::enumerate() test failed: i=\" << i\n-\t\t\t\t << \", index=\" << index << \", value=\" << value\n+\t\t\t\t << \", index=\" << v.index << \", value=\" << v.value\n \t\t\t\t << std::endl;\n \t\t\t\treturn TestFail;\n \t\t\t}\n@@ -114,11 +114,11 @@ protected:\n \t\tconst int array[] = { 0, 2, 4, 6, 8 };\n \t\ti = 0;\n \n-\t\tfor (auto [index, value] : utils::enumerate(array)) {\n-\t\t\tif (index != i || value != i * 2) {\n+\t\tfor (const auto v : utils::enumerate(array)) {\n+\t\t\tif (v.index != i || v.value != i * 2) {\n \t\t\t\tcerr << \"utils::enumerate() test failed: i=\" << i\n-\t\t\t\t << \", index=\" << index << \", value=\" << value\n-\t\t\t\t << std::endl;\n+\t\t\t\t << \", index=\" << v.index << \", value=\" << v.value\n+\t\t\t\t << endl;\n \t\t\t\treturn TestFail;\n \t\t\t}\n \n","prefixes":["libcamera-devel","PATCH/RFC","2/3"]}