[{"id":23447,"web_url":"https://patchwork.libcamera.org/comment/23447/","msgid":"<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>","date":"2022-06-17T13:45:05","subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Jun 16, 2022 at 05:23:58PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Allow using range-based for loops over YamlObject instances by\n> implementing iterators. New YamlObject::DictAdapter and\n> YamlObject::ListAdapter adapter classes are introduced to provide\n> different iterators depending on the object type.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/internal/yaml_parser.h | 117 ++++++++++++++++++++++-\n>  src/libcamera/yaml_parser.cpp            |  39 ++++++++\n>  2 files changed, 154 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index 29b7b218f0d9..8edd32354184 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -7,6 +7,7 @@\n>\n>  #pragma once\n>\n> +#include <iterator>\n>  #include <map>\n>  #include <string>\n>  #include <vector>\n> @@ -22,7 +23,116 @@ class YamlParserContext;\n>\n>  class YamlObject\n>  {\n> +private:\n> +\tusing DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;\n> +\tusing ListContainer = std::vector<std::unique_ptr<YamlObject>>;\n> +\n>  public:\n> +#ifndef __DOXYGEN__\n> +\ttemplate<typename Container, typename Derived>\n> +\tclass Iterator\n> +\t{\n> +\tpublic:\n> +\t\tusing difference_type = std::ptrdiff_t;\n> +\t\tusing iterator_category = std::bidirectional_iterator_tag;\n\nDoes it make any different the fact you only implement operator++ ?\nShould this be just a forward_iterator ?\n\n> +\n> +\t\tIterator(typename Container::const_iterator it)\n> +\t\t\t: it_(it)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tDerived &operator++()\n\nBeing it_ a ::const_iterator, should the returned type be a const too  ?\n\n> +\t\t{\n> +\t\t\t++it_;\n> +\t\t\treturn *static_cast<Derived *>(this);\n\nWeird, I was expecting this to be\n\t\t\treturn *static_cast<Derived *>(it);\n\nBut I've surely missed something as the class clearly works :)\n\n> +\t\t}\n> +\n> +\t\tDerived operator++(int)\n\nIs the 'int' tag to allow random access ?\n\n> +\t\t{\n> +\t\t\tDerived it = *static_cast<Derived *>(this);\n> +\t\t\tit_++;\n> +\t\t\treturn it;\n> +\t\t}\n> +\n> +\t\tfriend bool operator==(const Iterator &a, const Iterator &b)\n> +\t\t{\n> +\t\t\treturn a.it_ == b.it_;\n> +\t\t}\n> +\n> +\t\tfriend bool operator!=(const Iterator &a, const Iterator &b)\n> +\t\t{\n> +\t\t\treturn a.it_ != b.it_;\n> +\t\t}\n> +\n> +\tprotected:\n> +\t\ttypename Container::const_iterator it_;\n> +\t};\n> +\n> +\ttemplate<typename Container, typename Iterator>\n> +\tclass Adapter\n> +\t{\n> +\tpublic:\n> +\t\tAdapter(const Container &container)\n> +\t\t\t: container_(container)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tIterator begin() const\n> +\t\t{\n> +\t\t\treturn Iterator{ container_.begin() };\n> +\t\t}\n> +\n> +\t\tIterator end() const\n> +\t\t{\n> +\t\t\treturn Iterator{ container_.end() };\n> +\t\t}\n> +\n> +\tprotected:\n> +\t\tconst Container &container_;\n> +\t};\n> +\n> +\tclass ListIterator : public Iterator<ListContainer, ListIterator>\n> +\t{\n> +\tpublic:\n> +\t\tusing value_type = const YamlObject &;\n> +\t\tusing pointer = const YamlObject *;\n> +\t\tusing reference = value_type;\n> +\n> +\t\tvalue_type operator*() const\n> +\t\t{\n> +\t\t\treturn *it_->get();\n> +\t\t}\n> +\n> +\t\tpointer operator->() const\n> +\t\t{\n> +\t\t\treturn it_->get();\n> +\t\t}\n> +\t};\n> +\n> +\tclass DictIterator : public Iterator<DictContainer, DictIterator>\n> +\t{\n> +\tpublic:\n> +\t\tusing value_type = std::pair<const std::string &, const YamlObject &>;\n> +\t\tusing pointer = value_type *;\n> +\t\tusing reference = value_type &;\n> +\n> +\t\tvalue_type operator*() const\n> +\t\t{\n> +\t\t\treturn { it_->first, *it_->second.get() };\n> +\t\t}\n> +\t};\n> +\n> +\tclass DictAdapter : public Adapter<DictContainer, DictIterator>\n> +\t{\n> +\tpublic:\n> +\t\tusing key_type = std::string;\n> +\t};\n> +\n> +\tclass ListAdapter : public Adapter<ListContainer, ListIterator>\n> +\t{\n> +\t};\n> +#endif /* __DOXYGEN__ */\n> +\n>  \tYamlObject();\n>  \t~YamlObject();\n>\n> @@ -55,6 +165,9 @@ public:\n>  #endif\n>  \tT get(const T &defaultValue, bool *ok = nullptr) const;\n>\n> +\tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> +\tListAdapter asList() const { return ListAdapter{ list_ }; }\n\nWhen I first thought about making this iterable I was expecting\nsimple wrappers around std::list iterator functions. That would not\nallow to have it as dict or as list. I wonder if providing both\nabstraction is necessary (my bad, I never played very much with this\nclass).\n\nMore questions than comments, so I would say the patch looks good (as\nfar as I understand it :p )\n\nThanks\n   j\n\n> +\n>  \tconst YamlObject &operator[](std::size_t index) const;\n>\n>  \tbool contains(const std::string &key) const;\n> @@ -75,8 +188,8 @@ private:\n>  \tType type_;\n>\n>  \tstd::string value_;\n> -\tstd::vector<std::unique_ptr<YamlObject>> list_;\n> -\tstd::map<const std::string, std::unique_ptr<YamlObject>> dictionary_;\n> +\tListContainer list_;\n> +\tDictContainer dictionary_;\n>  };\n>\n>  class YamlParser final\n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> index 85f6694f5fde..4df7e5a33d47 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/yaml_parser.cpp\n> @@ -259,6 +259,45 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const\n>\n>  #endif /* __DOXYGEN__ */\n>\n> +/**\n> + * \\fn YamlObject::asDict() const\n> + * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> + *\n> + * The YamlObject class doesn't directly implement iterators, as the iterator\n> + * type depends on whether the object is a Dictionary or List. This function\n> + * wraps a YamlObject of Dictionary type into an adapter that exposes\n> + * iterators, as well as begin() and end() functions, allowing usage of\n> + * range-based for loops with YamlObject. As YAML mappings are not ordered, the\n> + * iteration order is not specified.\n> + *\n> + * The iterator's value_type is a\n> + * <em>std::pair<const std::string &, const \\ref YamlObject &></em>.\n> + *\n> + * If the YamlObject is not of Dictionary type, the returned adapter operates\n> + * as an empty container.\n> + *\n> + * \\return An adapter of unspecified type compatible with range-based for loops\n> + */\n> +\n> +/**\n> + * \\fn YamlObject::asList() const\n> + * \\brief Wrap a list YamlObject in an adapter that exposes iterators\n> + *\n> + * The YamlObject class doesn't directly implement iterators, as the iterator\n> + * type depends on whether the object is a Dictionary or List. This function\n> + * wraps a YamlObject of List type into an adapter that exposes iterators, as\n> + * well as begin() and end() functions, allowing usage of range-based for loops\n> + * with YamlObject. As YAML lists are ordered, the iteration order is identical\n> + * to the list order in the YAML data.\n> + *\n> + * The iterator's value_type is a <em>const YamlObject &</em>.\n> + *\n> + * If the YamlObject is not of List type, the returned adapter operates as an\n> + * empty container.\n> + *\n> + * \\return An adapter of unspecified type compatible with range-based for loops\n> + */\n> +\n>  /**\n>   * \\fn YamlObject::operator[](std::size_t index) const\n>   * \\brief Retrieve the element from list YamlObject by index\n> --\n> Regards,\n>\n> Laurent Pinchart\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 14B7CBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 17 Jun 2022 13:45:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 499B365635;\n\tFri, 17 Jun 2022 15:45:08 +0200 (CEST)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 99DC265632\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jun 2022 15:45:07 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 34E3B20000E;\n\tFri, 17 Jun 2022 13:45:06 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655473508;\n\tbh=/RZy8HVqoSGNQqHjFT16RtgX3kk08V6W97/O+eyhFlI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tJLEU2S7Oxb22pWGQY89bOyYqeGLLNP1ol4xC1jyN66bdIPGtFuw/sH/p5MKx5KTj\n\tuf6wOAw3SHYyIK9jqNlUaQxWTaxW3I12oeNY5KPaEqPGA9NaTqS203twTmMmOq+oD2\n\t4iPHA+/UWuCPYeRPwh683VkPWIELkGYgwKDc5L8btfrfj8ACZ1XTM6hJDbuIsGqBje\n\tZX2xRsVNYQ8tXqWThJeVBSLQ6oaB0O5mn0YcjASgaovlrAOVgV+M1YlLJH3OozOu/d\n\t7GJdBNzuaoAaMlSV++o0MG87nGyrzHFYIzjFCtO/G0DynN9OeI1ev13ooCprvwQjyk\n\tCqckEGb06pZhQ==","Date":"Fri, 17 Jun 2022 15:45:05 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>","References":"<20220616142403.20723-1-laurent.pinchart@ideasonboard.com>\n\t<20220616142403.20723-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220616142403.20723-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23455,"web_url":"https://patchwork.libcamera.org/comment/23455/","msgid":"<YqyLDB6Vq8rqqjuJ@pendragon.ideasonboard.com>","date":"2022-06-17T14:09:16","subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Jun 17, 2022 at 03:45:05PM +0200, Jacopo Mondi wrote:\n> On Thu, Jun 16, 2022 at 05:23:58PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Allow using range-based for loops over YamlObject instances by\n> > implementing iterators. New YamlObject::DictAdapter and\n> > YamlObject::ListAdapter adapter classes are introduced to provide\n> > different iterators depending on the object type.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/internal/yaml_parser.h | 117 ++++++++++++++++++++++-\n> >  src/libcamera/yaml_parser.cpp            |  39 ++++++++\n> >  2 files changed, 154 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index 29b7b218f0d9..8edd32354184 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -7,6 +7,7 @@\n> >\n> >  #pragma once\n> >\n> > +#include <iterator>\n> >  #include <map>\n> >  #include <string>\n> >  #include <vector>\n> > @@ -22,7 +23,116 @@ class YamlParserContext;\n> >\n> >  class YamlObject\n> >  {\n> > +private:\n> > +\tusing DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;\n> > +\tusing ListContainer = std::vector<std::unique_ptr<YamlObject>>;\n> > +\n> >  public:\n> > +#ifndef __DOXYGEN__\n> > +\ttemplate<typename Container, typename Derived>\n> > +\tclass Iterator\n> > +\t{\n> > +\tpublic:\n> > +\t\tusing difference_type = std::ptrdiff_t;\n> > +\t\tusing iterator_category = std::bidirectional_iterator_tag;\n> \n> Does it make any different the fact you only implement operator++ ?\n> Should this be just a forward_iterator ?\n\nVery good point. I'll go for a forward_iterator as the goal is really to\nsupport a for range loop. If someone wants to iterator backwards in the\nfuture, this can then be extended.\n\n> > +\n> > +\t\tIterator(typename Container::const_iterator it)\n> > +\t\t\t: it_(it)\n> > +\t\t{\n> > +\t\t}\n> > +\n> > +\t\tDerived &operator++()\n> \n> Being it_ a ::const_iterator, should the returned type be a const too  ?\n\nNo, a const_iterator isn't const itself. The \"const\" is related to the\nfact that the iterator doesn't allow modifying the element it points to,\nbut the iterator itself can be modified (incremented in this case).\n\n> > +\t\t{\n> > +\t\t\t++it_;\n> > +\t\t\treturn *static_cast<Derived *>(this);\n> \n> Weird, I was expecting this to be\n> \t\t\treturn *static_cast<Derived *>(it);\n> \n> But I've surely missed something as the class clearly works :)\n\nDerived is the type of the class deriving from Iterator (DictIterator or\nListIterator), while it_ is the iterator on the underlying container.\nThe pre-increment operator returns a reference to itself, the only\nreason why we need a cast is that DictIterator::operator++() should\nreturn a DictIterator & and ListIterator::operator++() a ListIterator &.\nAs operator++() is implemented once in the base class only, we pass the\nDerived type as a template argument, so the right type can be returned.\n\n> > +\t\t}\n> > +\n> > +\t\tDerived operator++(int)\n> \n> Is the 'int' tag to allow random access ?\n\nIt's used to differentiate the pre-increment (++x) and post-increment\n(x++) operators, as C++ doesn't allow overloads to differ by they return type\nonly. It's a C++ convention, operator++() is the pre-increment operator,\nand operator++(int) the post-increment operator.\n\n> > +\t\t{\n> > +\t\t\tDerived it = *static_cast<Derived *>(this);\n> > +\t\t\tit_++;\n> > +\t\t\treturn it;\n> > +\t\t}\n> > +\n> > +\t\tfriend bool operator==(const Iterator &a, const Iterator &b)\n> > +\t\t{\n> > +\t\t\treturn a.it_ == b.it_;\n> > +\t\t}\n> > +\n> > +\t\tfriend bool operator!=(const Iterator &a, const Iterator &b)\n> > +\t\t{\n> > +\t\t\treturn a.it_ != b.it_;\n> > +\t\t}\n> > +\n> > +\tprotected:\n> > +\t\ttypename Container::const_iterator it_;\n> > +\t};\n> > +\n> > +\ttemplate<typename Container, typename Iterator>\n> > +\tclass Adapter\n> > +\t{\n> > +\tpublic:\n> > +\t\tAdapter(const Container &container)\n> > +\t\t\t: container_(container)\n> > +\t\t{\n> > +\t\t}\n> > +\n> > +\t\tIterator begin() const\n> > +\t\t{\n> > +\t\t\treturn Iterator{ container_.begin() };\n> > +\t\t}\n> > +\n> > +\t\tIterator end() const\n> > +\t\t{\n> > +\t\t\treturn Iterator{ container_.end() };\n> > +\t\t}\n> > +\n> > +\tprotected:\n> > +\t\tconst Container &container_;\n> > +\t};\n> > +\n> > +\tclass ListIterator : public Iterator<ListContainer, ListIterator>\n> > +\t{\n> > +\tpublic:\n> > +\t\tusing value_type = const YamlObject &;\n> > +\t\tusing pointer = const YamlObject *;\n> > +\t\tusing reference = value_type;\n> > +\n> > +\t\tvalue_type operator*() const\n> > +\t\t{\n> > +\t\t\treturn *it_->get();\n> > +\t\t}\n> > +\n> > +\t\tpointer operator->() const\n> > +\t\t{\n> > +\t\t\treturn it_->get();\n> > +\t\t}\n> > +\t};\n> > +\n> > +\tclass DictIterator : public Iterator<DictContainer, DictIterator>\n> > +\t{\n> > +\tpublic:\n> > +\t\tusing value_type = std::pair<const std::string &, const YamlObject &>;\n> > +\t\tusing pointer = value_type *;\n> > +\t\tusing reference = value_type &;\n> > +\n> > +\t\tvalue_type operator*() const\n> > +\t\t{\n> > +\t\t\treturn { it_->first, *it_->second.get() };\n> > +\t\t}\n> > +\t};\n> > +\n> > +\tclass DictAdapter : public Adapter<DictContainer, DictIterator>\n> > +\t{\n> > +\tpublic:\n> > +\t\tusing key_type = std::string;\n> > +\t};\n> > +\n> > +\tclass ListAdapter : public Adapter<ListContainer, ListIterator>\n> > +\t{\n> > +\t};\n> > +#endif /* __DOXYGEN__ */\n> > +\n> >  \tYamlObject();\n> >  \t~YamlObject();\n> >\n> > @@ -55,6 +165,9 @@ public:\n> >  #endif\n> >  \tT get(const T &defaultValue, bool *ok = nullptr) const;\n> >\n> > +\tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > +\tListAdapter asList() const { return ListAdapter{ list_ }; }\n> \n> When I first thought about making this iterable I was expecting\n> simple wrappers around std::list iterator functions. That would not\n> allow to have it as dict or as list. I wonder if providing both\n> abstraction is necessary (my bad, I never played very much with this\n> class).\n\nI would have liked a way to avoid asDict() and asList(). It's possible,\nbut then we would need the values to be stored in a single container\n(done in the RFC v3 of the RPi YamlParser series), and the value_type of\nthe iterator to always be a pair, with the first member containing empty\nstrings when the YamlObject is a list. That's not very nice from an API\npoint of view.\n\n> More questions than comments, so I would say the patch looks good (as\n> far as I understand it :p )\n\nThank you :-)\n\n> > +\n> >  \tconst YamlObject &operator[](std::size_t index) const;\n> >\n> >  \tbool contains(const std::string &key) const;\n> > @@ -75,8 +188,8 @@ private:\n> >  \tType type_;\n> >\n> >  \tstd::string value_;\n> > -\tstd::vector<std::unique_ptr<YamlObject>> list_;\n> > -\tstd::map<const std::string, std::unique_ptr<YamlObject>> dictionary_;\n> > +\tListContainer list_;\n> > +\tDictContainer dictionary_;\n> >  };\n> >\n> >  class YamlParser final\n> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > index 85f6694f5fde..4df7e5a33d47 100644\n> > --- a/src/libcamera/yaml_parser.cpp\n> > +++ b/src/libcamera/yaml_parser.cpp\n> > @@ -259,6 +259,45 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const\n> >\n> >  #endif /* __DOXYGEN__ */\n> >\n> > +/**\n> > + * \\fn YamlObject::asDict() const\n> > + * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > + *\n> > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > + * type depends on whether the object is a Dictionary or List. This function\n> > + * wraps a YamlObject of Dictionary type into an adapter that exposes\n> > + * iterators, as well as begin() and end() functions, allowing usage of\n> > + * range-based for loops with YamlObject. As YAML mappings are not ordered, the\n> > + * iteration order is not specified.\n> > + *\n> > + * The iterator's value_type is a\n> > + * <em>std::pair<const std::string &, const \\ref YamlObject &></em>.\n> > + *\n> > + * If the YamlObject is not of Dictionary type, the returned adapter operates\n> > + * as an empty container.\n> > + *\n> > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > + */\n> > +\n> > +/**\n> > + * \\fn YamlObject::asList() const\n> > + * \\brief Wrap a list YamlObject in an adapter that exposes iterators\n> > + *\n> > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > + * type depends on whether the object is a Dictionary or List. This function\n> > + * wraps a YamlObject of List type into an adapter that exposes iterators, as\n> > + * well as begin() and end() functions, allowing usage of range-based for loops\n> > + * with YamlObject. As YAML lists are ordered, the iteration order is identical\n> > + * to the list order in the YAML data.\n> > + *\n> > + * The iterator's value_type is a <em>const YamlObject &</em>.\n> > + *\n> > + * If the YamlObject is not of List type, the returned adapter operates as an\n> > + * empty container.\n> > + *\n> > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > + */\n> > +\n> >  /**\n> >   * \\fn YamlObject::operator[](std::size_t index) const\n> >   * \\brief Retrieve the element from list YamlObject by index","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 1540CC3273\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 17 Jun 2022 14:09:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 967646563B;\n\tFri, 17 Jun 2022 16:09:30 +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 C1C5A65632\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jun 2022 16:09:28 +0200 (CEST)","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 3085B741;\n\tFri, 17 Jun 2022 16:09:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655474970;\n\tbh=axRhyfiMPIgxo6cSaoQopQz8rkx0GH1b16s2NCTwEJI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=oN8rmNUbie3V/jePqWoOYKJUVTWA30SNGRnqIr/H2YDp2Bo9jnaygDAGzy62wEa2I\n\tcuT8QfAYWP+sb+nCZRY11RK2Sh6STNTmaTdO8hCy8xsOT4OIWfUA5iRdNAxSAwktqF\n\tPRYTnTVF/4i0g+4RN5UVMAFkzqkPwuFP3MleN1DMIXHuHrv/B33JXwEzNXjhPXJ2Lh\n\t1p5q+oG//R0Z8hro/KdPQ3jUOVxrqlVEVKlcxlkxJW33D8crb4rkIB986Q6TCtqtn3\n\twL0t19ke95NqlFzTgeIYblBjoKMyz+Z1z6giBiVt8ShSpTDJNw/R0RyWUthNhXp5BT\n\tX6mopXOd++2+A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655474968;\n\tbh=axRhyfiMPIgxo6cSaoQopQz8rkx0GH1b16s2NCTwEJI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cwUp5v0VKzU+Y8czGa7jrVL3JTQV9xIO6rTunVUi2MQH0jHk2KtxPH91AKk7FGiSi\n\tN12KZ1WDuIiFCkB3Pw8wsMbFBR/oLVqT8JkXLnOoymZ2r7mwgsIQcPzviZTWrBnBcr\n\tbbLgtiJevUyk2B7EUSZ0C0EkdAfkQHCCHEpEfojs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cwUp5v0V\"; dkim-atps=neutral","Date":"Fri, 17 Jun 2022 17:09:16 +0300","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YqyLDB6Vq8rqqjuJ@pendragon.ideasonboard.com>","References":"<20220616142403.20723-1-laurent.pinchart@ideasonboard.com>\n\t<20220616142403.20723-3-laurent.pinchart@ideasonboard.com>\n\t<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23457,"web_url":"https://patchwork.libcamera.org/comment/23457/","msgid":"<20220617142058.mbxyjxxamf5kvxvq@uno.localdomain>","date":"2022-06-17T14:20:58","subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\n  thanks for the clarifications, with the minor issue of the iterator\ntype changed\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n     j\n\nOn Fri, Jun 17, 2022 at 05:09:16PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Fri, Jun 17, 2022 at 03:45:05PM +0200, Jacopo Mondi wrote:\n> > On Thu, Jun 16, 2022 at 05:23:58PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > Allow using range-based for loops over YamlObject instances by\n> > > implementing iterators. New YamlObject::DictAdapter and\n> > > YamlObject::ListAdapter adapter classes are introduced to provide\n> > > different iterators depending on the object type.\n> > >\n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > >  include/libcamera/internal/yaml_parser.h | 117 ++++++++++++++++++++++-\n> > >  src/libcamera/yaml_parser.cpp            |  39 ++++++++\n> > >  2 files changed, 154 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > index 29b7b218f0d9..8edd32354184 100644\n> > > --- a/include/libcamera/internal/yaml_parser.h\n> > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > @@ -7,6 +7,7 @@\n> > >\n> > >  #pragma once\n> > >\n> > > +#include <iterator>\n> > >  #include <map>\n> > >  #include <string>\n> > >  #include <vector>\n> > > @@ -22,7 +23,116 @@ class YamlParserContext;\n> > >\n> > >  class YamlObject\n> > >  {\n> > > +private:\n> > > +\tusing DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;\n> > > +\tusing ListContainer = std::vector<std::unique_ptr<YamlObject>>;\n> > > +\n> > >  public:\n> > > +#ifndef __DOXYGEN__\n> > > +\ttemplate<typename Container, typename Derived>\n> > > +\tclass Iterator\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tusing difference_type = std::ptrdiff_t;\n> > > +\t\tusing iterator_category = std::bidirectional_iterator_tag;\n> >\n> > Does it make any different the fact you only implement operator++ ?\n> > Should this be just a forward_iterator ?\n>\n> Very good point. I'll go for a forward_iterator as the goal is really to\n> support a for range loop. If someone wants to iterator backwards in the\n> future, this can then be extended.\n>\n> > > +\n> > > +\t\tIterator(typename Container::const_iterator it)\n> > > +\t\t\t: it_(it)\n> > > +\t\t{\n> > > +\t\t}\n> > > +\n> > > +\t\tDerived &operator++()\n> >\n> > Being it_ a ::const_iterator, should the returned type be a const too  ?\n>\n> No, a const_iterator isn't const itself. The \"const\" is related to the\n> fact that the iterator doesn't allow modifying the element it points to,\n> but the iterator itself can be modified (incremented in this case).\n>\n> > > +\t\t{\n> > > +\t\t\t++it_;\n> > > +\t\t\treturn *static_cast<Derived *>(this);\n> >\n> > Weird, I was expecting this to be\n> > \t\t\treturn *static_cast<Derived *>(it);\n> >\n> > But I've surely missed something as the class clearly works :)\n>\n> Derived is the type of the class deriving from Iterator (DictIterator or\n> ListIterator), while it_ is the iterator on the underlying container.\n> The pre-increment operator returns a reference to itself, the only\n> reason why we need a cast is that DictIterator::operator++() should\n> return a DictIterator & and ListIterator::operator++() a ListIterator &.\n> As operator++() is implemented once in the base class only, we pass the\n> Derived type as a template argument, so the right type can be returned.\n>\n> > > +\t\t}\n> > > +\n> > > +\t\tDerived operator++(int)\n> >\n> > Is the 'int' tag to allow random access ?\n>\n> It's used to differentiate the pre-increment (++x) and post-increment\n> (x++) operators, as C++ doesn't allow overloads to differ by they return type\n> only. It's a C++ convention, operator++() is the pre-increment operator,\n> and operator++(int) the post-increment operator.\n>\n> > > +\t\t{\n> > > +\t\t\tDerived it = *static_cast<Derived *>(this);\n> > > +\t\t\tit_++;\n> > > +\t\t\treturn it;\n> > > +\t\t}\n> > > +\n> > > +\t\tfriend bool operator==(const Iterator &a, const Iterator &b)\n> > > +\t\t{\n> > > +\t\t\treturn a.it_ == b.it_;\n> > > +\t\t}\n> > > +\n> > > +\t\tfriend bool operator!=(const Iterator &a, const Iterator &b)\n> > > +\t\t{\n> > > +\t\t\treturn a.it_ != b.it_;\n> > > +\t\t}\n> > > +\n> > > +\tprotected:\n> > > +\t\ttypename Container::const_iterator it_;\n> > > +\t};\n> > > +\n> > > +\ttemplate<typename Container, typename Iterator>\n> > > +\tclass Adapter\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tAdapter(const Container &container)\n> > > +\t\t\t: container_(container)\n> > > +\t\t{\n> > > +\t\t}\n> > > +\n> > > +\t\tIterator begin() const\n> > > +\t\t{\n> > > +\t\t\treturn Iterator{ container_.begin() };\n> > > +\t\t}\n> > > +\n> > > +\t\tIterator end() const\n> > > +\t\t{\n> > > +\t\t\treturn Iterator{ container_.end() };\n> > > +\t\t}\n> > > +\n> > > +\tprotected:\n> > > +\t\tconst Container &container_;\n> > > +\t};\n> > > +\n> > > +\tclass ListIterator : public Iterator<ListContainer, ListIterator>\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tusing value_type = const YamlObject &;\n> > > +\t\tusing pointer = const YamlObject *;\n> > > +\t\tusing reference = value_type;\n> > > +\n> > > +\t\tvalue_type operator*() const\n> > > +\t\t{\n> > > +\t\t\treturn *it_->get();\n> > > +\t\t}\n> > > +\n> > > +\t\tpointer operator->() const\n> > > +\t\t{\n> > > +\t\t\treturn it_->get();\n> > > +\t\t}\n> > > +\t};\n> > > +\n> > > +\tclass DictIterator : public Iterator<DictContainer, DictIterator>\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tusing value_type = std::pair<const std::string &, const YamlObject &>;\n> > > +\t\tusing pointer = value_type *;\n> > > +\t\tusing reference = value_type &;\n> > > +\n> > > +\t\tvalue_type operator*() const\n> > > +\t\t{\n> > > +\t\t\treturn { it_->first, *it_->second.get() };\n> > > +\t\t}\n> > > +\t};\n> > > +\n> > > +\tclass DictAdapter : public Adapter<DictContainer, DictIterator>\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tusing key_type = std::string;\n> > > +\t};\n> > > +\n> > > +\tclass ListAdapter : public Adapter<ListContainer, ListIterator>\n> > > +\t{\n> > > +\t};\n> > > +#endif /* __DOXYGEN__ */\n> > > +\n> > >  \tYamlObject();\n> > >  \t~YamlObject();\n> > >\n> > > @@ -55,6 +165,9 @@ public:\n> > >  #endif\n> > >  \tT get(const T &defaultValue, bool *ok = nullptr) const;\n> > >\n> > > +\tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > > +\tListAdapter asList() const { return ListAdapter{ list_ }; }\n> >\n> > When I first thought about making this iterable I was expecting\n> > simple wrappers around std::list iterator functions. That would not\n> > allow to have it as dict or as list. I wonder if providing both\n> > abstraction is necessary (my bad, I never played very much with this\n> > class).\n>\n> I would have liked a way to avoid asDict() and asList(). It's possible,\n> but then we would need the values to be stored in a single container\n> (done in the RFC v3 of the RPi YamlParser series), and the value_type of\n> the iterator to always be a pair, with the first member containing empty\n> strings when the YamlObject is a list. That's not very nice from an API\n> point of view.\n>\n> > More questions than comments, so I would say the patch looks good (as\n> > far as I understand it :p )\n>\n> Thank you :-)\n>\n> > > +\n> > >  \tconst YamlObject &operator[](std::size_t index) const;\n> > >\n> > >  \tbool contains(const std::string &key) const;\n> > > @@ -75,8 +188,8 @@ private:\n> > >  \tType type_;\n> > >\n> > >  \tstd::string value_;\n> > > -\tstd::vector<std::unique_ptr<YamlObject>> list_;\n> > > -\tstd::map<const std::string, std::unique_ptr<YamlObject>> dictionary_;\n> > > +\tListContainer list_;\n> > > +\tDictContainer dictionary_;\n> > >  };\n> > >\n> > >  class YamlParser final\n> > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > > index 85f6694f5fde..4df7e5a33d47 100644\n> > > --- a/src/libcamera/yaml_parser.cpp\n> > > +++ b/src/libcamera/yaml_parser.cpp\n> > > @@ -259,6 +259,45 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const\n> > >\n> > >  #endif /* __DOXYGEN__ */\n> > >\n> > > +/**\n> > > + * \\fn YamlObject::asDict() const\n> > > + * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > > + *\n> > > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > > + * type depends on whether the object is a Dictionary or List. This function\n> > > + * wraps a YamlObject of Dictionary type into an adapter that exposes\n> > > + * iterators, as well as begin() and end() functions, allowing usage of\n> > > + * range-based for loops with YamlObject. As YAML mappings are not ordered, the\n> > > + * iteration order is not specified.\n> > > + *\n> > > + * The iterator's value_type is a\n> > > + * <em>std::pair<const std::string &, const \\ref YamlObject &></em>.\n> > > + *\n> > > + * If the YamlObject is not of Dictionary type, the returned adapter operates\n> > > + * as an empty container.\n> > > + *\n> > > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn YamlObject::asList() const\n> > > + * \\brief Wrap a list YamlObject in an adapter that exposes iterators\n> > > + *\n> > > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > > + * type depends on whether the object is a Dictionary or List. This function\n> > > + * wraps a YamlObject of List type into an adapter that exposes iterators, as\n> > > + * well as begin() and end() functions, allowing usage of range-based for loops\n> > > + * with YamlObject. As YAML lists are ordered, the iteration order is identical\n> > > + * to the list order in the YAML data.\n> > > + *\n> > > + * The iterator's value_type is a <em>const YamlObject &</em>.\n> > > + *\n> > > + * If the YamlObject is not of List type, the returned adapter operates as an\n> > > + * empty container.\n> > > + *\n> > > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > > + */\n> > > +\n> > >  /**\n> > >   * \\fn YamlObject::operator[](std::size_t index) const\n> > >   * \\brief Retrieve the element from list YamlObject by index\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 77912BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 17 Jun 2022 14:21:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9ACA65633;\n\tFri, 17 Jun 2022 16:21:02 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C65E365632\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jun 2022 16:21:00 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 5D648240003;\n\tFri, 17 Jun 2022 14:21:00 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655475662;\n\tbh=jv19XiBwTxypkrM7pIM2Jo3k/yx0SIksiQbk/EeL6W4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=mFWRotxyw57QhAUrw0HH8jPbENwzImXKz/UUTLFEqiksGlXMAnpAs5UtFaKeWeSeO\n\tM4t2SA2A5PrmEtVE6SKcJM4HO6RNrhOHDkXUc3Eu31c5EA1kQCN0SLyW3+kaIpAXwm\n\tt+HTTXM/zaVIoA/bH2p4tSRdet4DuRG4e3qf+7v6CByTwhSFF8BEjxyLj1qho0Lglu\n\trxtD+ekj/5ePB0L8Jhplz84dP2NsieR2TkJP44XdXU08s6mq9tkgweYaqiRms9cq2m\n\t2Ryj1lrjAKNhbRIThaEuralHixGUfGluxJ3gob67PpwiS6bVVdPmc2uTru1Eio+KBI\n\txhMOR71cPUAsQ==","Date":"Fri, 17 Jun 2022 16:20:58 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20220617142058.mbxyjxxamf5kvxvq@uno.localdomain>","References":"<20220616142403.20723-1-laurent.pinchart@ideasonboard.com>\n\t<20220616142403.20723-3-laurent.pinchart@ideasonboard.com>\n\t<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>\n\t<YqyLDB6Vq8rqqjuJ@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<YqyLDB6Vq8rqqjuJ@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23483,"web_url":"https://patchwork.libcamera.org/comment/23483/","msgid":"<CAJAuwM=VgRciUh3OcvrroV-6ehXj1OUriGMc9L6mucCL=bTDbg@mail.gmail.com>","date":"2022-06-20T12:02:13","subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Laurent,\n\nReviewed-by: Han-Lin Chen <hanlinchen@chromium.org>\n\nThanks for the extension. Looks great to me.\nJust to confirm, I suppose the adding an adaptor instead of returning\nthe const version of map and list is to hide the underlying types?\n\nOn Fri, Jun 17, 2022 at 10:21 PM Jacopo Mondi via libcamera-devel\n<libcamera-devel@lists.libcamera.org> wrote:\n>\n> Hi Laurent\n>\n>   thanks for the clarifications, with the minor issue of the iterator\n> type changed\n>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Thanks\n>      j\n>\n> On Fri, Jun 17, 2022 at 05:09:16PM +0300, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > On Fri, Jun 17, 2022 at 03:45:05PM +0200, Jacopo Mondi wrote:\n> > > On Thu, Jun 16, 2022 at 05:23:58PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > Allow using range-based for loops over YamlObject instances by\n> > > > implementing iterators. New YamlObject::DictAdapter and\n> > > > YamlObject::ListAdapter adapter classes are introduced to provide\n> > > > different iterators depending on the object type.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > > ---\n> > > >  include/libcamera/internal/yaml_parser.h | 117 ++++++++++++++++++++++-\n> > > >  src/libcamera/yaml_parser.cpp            |  39 ++++++++\n> > > >  2 files changed, 154 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > > index 29b7b218f0d9..8edd32354184 100644\n> > > > --- a/include/libcamera/internal/yaml_parser.h\n> > > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > > @@ -7,6 +7,7 @@\n> > > >\n> > > >  #pragma once\n> > > >\n> > > > +#include <iterator>\n> > > >  #include <map>\n> > > >  #include <string>\n> > > >  #include <vector>\n> > > > @@ -22,7 +23,116 @@ class YamlParserContext;\n> > > >\n> > > >  class YamlObject\n> > > >  {\n> > > > +private:\n> > > > + using DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;\n> > > > + using ListContainer = std::vector<std::unique_ptr<YamlObject>>;\n> > > > +\n> > > >  public:\n> > > > +#ifndef __DOXYGEN__\n> > > > + template<typename Container, typename Derived>\n> > > > + class Iterator\n> > > > + {\n> > > > + public:\n> > > > +         using difference_type = std::ptrdiff_t;\n> > > > +         using iterator_category = std::bidirectional_iterator_tag;\n> > >\n> > > Does it make any different the fact you only implement operator++ ?\n> > > Should this be just a forward_iterator ?\n> >\n> > Very good point. I'll go for a forward_iterator as the goal is really to\n> > support a for range loop. If someone wants to iterator backwards in the\n> > future, this can then be extended.\n> >\n> > > > +\n> > > > +         Iterator(typename Container::const_iterator it)\n> > > > +                 : it_(it)\n> > > > +         {\n> > > > +         }\n> > > > +\n> > > > +         Derived &operator++()\n> > >\n> > > Being it_ a ::const_iterator, should the returned type be a const too  ?\n> >\n> > No, a const_iterator isn't const itself. The \"const\" is related to the\n> > fact that the iterator doesn't allow modifying the element it points to,\n> > but the iterator itself can be modified (incremented in this case).\n> >\n> > > > +         {\n> > > > +                 ++it_;\n> > > > +                 return *static_cast<Derived *>(this);\n> > >\n> > > Weird, I was expecting this to be\n> > >                     return *static_cast<Derived *>(it);\n> > >\n> > > But I've surely missed something as the class clearly works :)\n> >\n> > Derived is the type of the class deriving from Iterator (DictIterator or\n> > ListIterator), while it_ is the iterator on the underlying container.\n> > The pre-increment operator returns a reference to itself, the only\n> > reason why we need a cast is that DictIterator::operator++() should\n> > return a DictIterator & and ListIterator::operator++() a ListIterator &.\n> > As operator++() is implemented once in the base class only, we pass the\n> > Derived type as a template argument, so the right type can be returned.\n> >\n> > > > +         }\n> > > > +\n> > > > +         Derived operator++(int)\n> > >\n> > > Is the 'int' tag to allow random access ?\n> >\n> > It's used to differentiate the pre-increment (++x) and post-increment\n> > (x++) operators, as C++ doesn't allow overloads to differ by they return type\n> > only. It's a C++ convention, operator++() is the pre-increment operator,\n> > and operator++(int) the post-increment operator.\n> >\n> > > > +         {\n> > > > +                 Derived it = *static_cast<Derived *>(this);\n> > > > +                 it_++;\n> > > > +                 return it;\n> > > > +         }\n> > > > +\n> > > > +         friend bool operator==(const Iterator &a, const Iterator &b)\n> > > > +         {\n> > > > +                 return a.it_ == b.it_;\n> > > > +         }\n> > > > +\n> > > > +         friend bool operator!=(const Iterator &a, const Iterator &b)\n> > > > +         {\n> > > > +                 return a.it_ != b.it_;\n> > > > +         }\n> > > > +\n> > > > + protected:\n> > > > +         typename Container::const_iterator it_;\n> > > > + };\n> > > > +\n> > > > + template<typename Container, typename Iterator>\n> > > > + class Adapter\n> > > > + {\n> > > > + public:\n> > > > +         Adapter(const Container &container)\n> > > > +                 : container_(container)\n> > > > +         {\n> > > > +         }\n> > > > +\n> > > > +         Iterator begin() const\n> > > > +         {\n> > > > +                 return Iterator{ container_.begin() };\n> > > > +         }\n> > > > +\n> > > > +         Iterator end() const\n> > > > +         {\n> > > > +                 return Iterator{ container_.end() };\n> > > > +         }\n> > > > +\n> > > > + protected:\n> > > > +         const Container &container_;\n> > > > + };\n> > > > +\n> > > > + class ListIterator : public Iterator<ListContainer, ListIterator>\n> > > > + {\n> > > > + public:\n> > > > +         using value_type = const YamlObject &;\n> > > > +         using pointer = const YamlObject *;\n> > > > +         using reference = value_type;\n> > > > +\n> > > > +         value_type operator*() const\n> > > > +         {\n> > > > +                 return *it_->get();\n> > > > +         }\n> > > > +\n> > > > +         pointer operator->() const\n> > > > +         {\n> > > > +                 return it_->get();\n> > > > +         }\n> > > > + };\n> > > > +\n> > > > + class DictIterator : public Iterator<DictContainer, DictIterator>\n> > > > + {\n> > > > + public:\n> > > > +         using value_type = std::pair<const std::string &, const YamlObject &>;\n> > > > +         using pointer = value_type *;\n> > > > +         using reference = value_type &;\n> > > > +\n> > > > +         value_type operator*() const\n> > > > +         {\n> > > > +                 return { it_->first, *it_->second.get() };\n> > > > +         }\n> > > > + };\n> > > > +\n> > > > + class DictAdapter : public Adapter<DictContainer, DictIterator>\n> > > > + {\n> > > > + public:\n> > > > +         using key_type = std::string;\n> > > > + };\n> > > > +\n> > > > + class ListAdapter : public Adapter<ListContainer, ListIterator>\n> > > > + {\n> > > > + };\n> > > > +#endif /* __DOXYGEN__ */\n> > > > +\n> > > >   YamlObject();\n> > > >   ~YamlObject();\n> > > >\n> > > > @@ -55,6 +165,9 @@ public:\n> > > >  #endif\n> > > >   T get(const T &defaultValue, bool *ok = nullptr) const;\n> > > >\n> > > > + DictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > > > + ListAdapter asList() const { return ListAdapter{ list_ }; }\n> > >\n> > > When I first thought about making this iterable I was expecting\n> > > simple wrappers around std::list iterator functions. That would not\n> > > allow to have it as dict or as list. I wonder if providing both\n> > > abstraction is necessary (my bad, I never played very much with this\n> > > class).\n> >\n> > I would have liked a way to avoid asDict() and asList(). It's possible,\n> > but then we would need the values to be stored in a single container\n> > (done in the RFC v3 of the RPi YamlParser series), and the value_type of\n> > the iterator to always be a pair, with the first member containing empty\n> > strings when the YamlObject is a list. That's not very nice from an API\n> > point of view.\n> >\n> > > More questions than comments, so I would say the patch looks good (as\n> > > far as I understand it :p )\n> >\n> > Thank you :-)\n> >\n> > > > +\n> > > >   const YamlObject &operator[](std::size_t index) const;\n> > > >\n> > > >   bool contains(const std::string &key) const;\n> > > > @@ -75,8 +188,8 @@ private:\n> > > >   Type type_;\n> > > >\n> > > >   std::string value_;\n> > > > - std::vector<std::unique_ptr<YamlObject>> list_;\n> > > > - std::map<const std::string, std::unique_ptr<YamlObject>> dictionary_;\n> > > > + ListContainer list_;\n> > > > + DictContainer dictionary_;\n> > > >  };\n> > > >\n> > > >  class YamlParser final\n> > > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > > > index 85f6694f5fde..4df7e5a33d47 100644\n> > > > --- a/src/libcamera/yaml_parser.cpp\n> > > > +++ b/src/libcamera/yaml_parser.cpp\n> > > > @@ -259,6 +259,45 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const\n> > > >\n> > > >  #endif /* __DOXYGEN__ */\n> > > >\n> > > > +/**\n> > > > + * \\fn YamlObject::asDict() const\n> > > > + * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > > > + *\n> > > > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > > > + * type depends on whether the object is a Dictionary or List. This function\n> > > > + * wraps a YamlObject of Dictionary type into an adapter that exposes\n> > > > + * iterators, as well as begin() and end() functions, allowing usage of\n> > > > + * range-based for loops with YamlObject. As YAML mappings are not ordered, the\n> > > > + * iteration order is not specified.\n> > > > + *\n> > > > + * The iterator's value_type is a\n> > > > + * <em>std::pair<const std::string &, const \\ref YamlObject &></em>.\n> > > > + *\n> > > > + * If the YamlObject is not of Dictionary type, the returned adapter operates\n> > > > + * as an empty container.\n> > > > + *\n> > > > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\fn YamlObject::asList() const\n> > > > + * \\brief Wrap a list YamlObject in an adapter that exposes iterators\n> > > > + *\n> > > > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > > > + * type depends on whether the object is a Dictionary or List. This function\n> > > > + * wraps a YamlObject of List type into an adapter that exposes iterators, as\n> > > > + * well as begin() and end() functions, allowing usage of range-based for loops\n> > > > + * with YamlObject. As YAML lists are ordered, the iteration order is identical\n> > > > + * to the list order in the YAML data.\n> > > > + *\n> > > > + * The iterator's value_type is a <em>const YamlObject &</em>.\n> > > > + *\n> > > > + * If the YamlObject is not of List type, the returned adapter operates as an\n> > > > + * empty container.\n> > > > + *\n> > > > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > > > + */\n> > > > +\n> > > >  /**\n> > > >   * \\fn YamlObject::operator[](std::size_t index) const\n> > > >   * \\brief Retrieve the element from list YamlObject by index\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","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 5CEA7BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jun 2022 12:02:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF79765635;\n\tMon, 20 Jun 2022 14:02:27 +0200 (CEST)","from mail-oa1-x2a.google.com (mail-oa1-x2a.google.com\n\t[IPv6:2001:4860:4864:20::2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 814B160498\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jun 2022 14:02:25 +0200 (CEST)","by mail-oa1-x2a.google.com with SMTP id\n\t586e51a60fabf-101ab23ff3fso10243937fac.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jun 2022 05:02:25 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655726547;\n\tbh=aeWuiCgREpD0dUfPzB4DAuNvzQAnb0sQX9IkGLBxNlc=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=nXcGiEyBgqPU1YSzXhRJ3fcMWg57BpHPfR1RiZoQce6rHXmmUgxoFPRqVJolDwqxb\n\tw7TuBgdnMhBYCfJSZrh80cO/tdRRgjgbjbtSFtrqBkndQx8C6/Ja0pHRZbS+XVgDe/\n\tMbLYkbd6VJ/3DJ2SenpgqLJBRfs+HxtP4L/aRjTrLG9XSuDVRij+ATX8fFwGDxg/dN\n\tlxVzhmWEyBLvIgmTEXq6eTYU8l9uXQIQ+AQN1C/zewyw5H7/knhXFd/7UD49+iWzFj\n\tNab0gFvcf7VJcWpOnGVNF7Pz+EGlmP54DCAdpFsUuJbtu1PS/z492YSeDdwk1YHK3v\n\tQ0bkxNDkwVTBA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=v77fosB9d3AErEun0MdEn4GQD87/U9M6zRXAGMTTzb4=;\n\tb=XW5H5FZezE/uPbPNciiJIjD86Nnph/dXe5uKHp4gvxR4DeV1E3T0YKFx6IXx2PUhMj\n\tWS67hxY/xh1GorXnTZDCoUym3brWsOOfl/1RIu9M7MNxlDT2BmQd12H5FyrGduZUuceU\n\tmbr++Ei04NhJKeCA7+mnkXyDUA/VEkpr4WH7E="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"XW5H5FZe\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=v77fosB9d3AErEun0MdEn4GQD87/U9M6zRXAGMTTzb4=;\n\tb=avr9/1ZBPg204QndVy9cecZEUO2C9zs9aZHoNqQvW5DbLmjZ4wB8FXt+6D8fGaWWuD\n\t5pS+WaL+Xr8s6ZXGC0ok255+roiQGJWSymJwDvokF9T+1juFGugNXCFAlqrtiuQdBnw7\n\truBhFaCiAsR+NeuQzu3n+N2uj2yTNpawdV2YKLOEUez/HSylyP/kxKpn78WQvupKcANF\n\tsQb4F7FAZg/650rmi9+DxJsPLohSJ6bGq+FeFVCECQlRnnQEyoUtmwpk7Ij8dUVO1FJA\n\tH8HIX9frhQiVGw0ARkCpkZ4XSFjZP4vJJ3BY3ry9C6V3noaBftjbz3S43G7b/vLCckuU\n\tSLhg==","X-Gm-Message-State":"AJIora9GbdoIosSu7Wde5deoWCic6cq6xtqHwCVYroMKF7fcCJnK8x9V\n\tNxiBEjHdiKFk4Jb1fdQxT4L8clZbQcUy8grxk4wH2g==","X-Google-Smtp-Source":"AGRyM1u9Zag4c8GlCwjIatzZV6zIQ7PUILOa/BXwtSFPQjfN6prxzunJLmEh7nnKDvWMJh9nrQVbcAD/SAVayI/4u8g=","X-Received":"by 2002:a05:6870:ea8e:b0:fe:251a:3c3d with SMTP id\n\ts14-20020a056870ea8e00b000fe251a3c3dmr18285962oap.62.1655726544232;\n\tMon, 20 Jun 2022 05:02:24 -0700 (PDT)","MIME-Version":"1.0","References":"<20220616142403.20723-1-laurent.pinchart@ideasonboard.com>\n\t<20220616142403.20723-3-laurent.pinchart@ideasonboard.com>\n\t<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>\n\t<YqyLDB6Vq8rqqjuJ@pendragon.ideasonboard.com>\n\t<20220617142058.mbxyjxxamf5kvxvq@uno.localdomain>","In-Reply-To":"<20220617142058.mbxyjxxamf5kvxvq@uno.localdomain>","Date":"Mon, 20 Jun 2022 20:02:13 +0800","Message-ID":"<CAJAuwM=VgRciUh3OcvrroV-6ehXj1OUriGMc9L6mucCL=bTDbg@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","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>","From":"Hanlin Chen via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Hanlin Chen <hanlinchen@chromium.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23490,"web_url":"https://patchwork.libcamera.org/comment/23490/","msgid":"<YrBjktnCNESYdbIE@pendragon.ideasonboard.com>","date":"2022-06-20T12:09:54","subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Han-Lin,\n\nOn Mon, Jun 20, 2022 at 08:02:13PM +0800, Hanlin Chen wrote:\n> Hi Laurent,\n> \n> Reviewed-by: Han-Lin Chen <hanlinchen@chromium.org>\n> \n> Thanks for the extension. Looks great to me.\n> Just to confirm, I suppose the adding an adaptor instead of returning\n> the const version of map and list is to hide the underlying types?\n\nThat's right, it's mostly to hide the unique_ptr (and of course also\nbecause libcamera wasn't making use of\nhttps://en.wikipedia.org/wiki/Curiously_recurring_template_pattern yet\n:-)).\n\n> On Fri, Jun 17, 2022 at 10:21 PM Jacopo Mondi via libcamera-devel wrote:\n> >\n> > Hi Laurent\n> >\n> >   thanks for the clarifications, with the minor issue of the iterator\n> > type changed\n> >\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > On Fri, Jun 17, 2022 at 05:09:16PM +0300, Laurent Pinchart wrote:\n> > > On Fri, Jun 17, 2022 at 03:45:05PM +0200, Jacopo Mondi wrote:\n> > > > On Thu, Jun 16, 2022 at 05:23:58PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > > Allow using range-based for loops over YamlObject instances by\n> > > > > implementing iterators. New YamlObject::DictAdapter and\n> > > > > YamlObject::ListAdapter adapter classes are introduced to provide\n> > > > > different iterators depending on the object type.\n> > > > >\n> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > > > ---\n> > > > >  include/libcamera/internal/yaml_parser.h | 117 ++++++++++++++++++++++-\n> > > > >  src/libcamera/yaml_parser.cpp            |  39 ++++++++\n> > > > >  2 files changed, 154 insertions(+), 2 deletions(-)\n> > > > >\n> > > > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > > > index 29b7b218f0d9..8edd32354184 100644\n> > > > > --- a/include/libcamera/internal/yaml_parser.h\n> > > > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > > > @@ -7,6 +7,7 @@\n> > > > >\n> > > > >  #pragma once\n> > > > >\n> > > > > +#include <iterator>\n> > > > >  #include <map>\n> > > > >  #include <string>\n> > > > >  #include <vector>\n> > > > > @@ -22,7 +23,116 @@ class YamlParserContext;\n> > > > >\n> > > > >  class YamlObject\n> > > > >  {\n> > > > > +private:\n> > > > > + using DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;\n> > > > > + using ListContainer = std::vector<std::unique_ptr<YamlObject>>;\n> > > > > +\n> > > > >  public:\n> > > > > +#ifndef __DOXYGEN__\n> > > > > + template<typename Container, typename Derived>\n> > > > > + class Iterator\n> > > > > + {\n> > > > > + public:\n> > > > > +         using difference_type = std::ptrdiff_t;\n> > > > > +         using iterator_category = std::bidirectional_iterator_tag;\n> > > >\n> > > > Does it make any different the fact you only implement operator++ ?\n> > > > Should this be just a forward_iterator ?\n> > >\n> > > Very good point. I'll go for a forward_iterator as the goal is really to\n> > > support a for range loop. If someone wants to iterator backwards in the\n> > > future, this can then be extended.\n> > >\n> > > > > +\n> > > > > +         Iterator(typename Container::const_iterator it)\n> > > > > +                 : it_(it)\n> > > > > +         {\n> > > > > +         }\n> > > > > +\n> > > > > +         Derived &operator++()\n> > > >\n> > > > Being it_ a ::const_iterator, should the returned type be a const too  ?\n> > >\n> > > No, a const_iterator isn't const itself. The \"const\" is related to the\n> > > fact that the iterator doesn't allow modifying the element it points to,\n> > > but the iterator itself can be modified (incremented in this case).\n> > >\n> > > > > +         {\n> > > > > +                 ++it_;\n> > > > > +                 return *static_cast<Derived *>(this);\n> > > >\n> > > > Weird, I was expecting this to be\n> > > >                     return *static_cast<Derived *>(it);\n> > > >\n> > > > But I've surely missed something as the class clearly works :)\n> > >\n> > > Derived is the type of the class deriving from Iterator (DictIterator or\n> > > ListIterator), while it_ is the iterator on the underlying container.\n> > > The pre-increment operator returns a reference to itself, the only\n> > > reason why we need a cast is that DictIterator::operator++() should\n> > > return a DictIterator & and ListIterator::operator++() a ListIterator &.\n> > > As operator++() is implemented once in the base class only, we pass the\n> > > Derived type as a template argument, so the right type can be returned.\n> > >\n> > > > > +         }\n> > > > > +\n> > > > > +         Derived operator++(int)\n> > > >\n> > > > Is the 'int' tag to allow random access ?\n> > >\n> > > It's used to differentiate the pre-increment (++x) and post-increment\n> > > (x++) operators, as C++ doesn't allow overloads to differ by they return type\n> > > only. It's a C++ convention, operator++() is the pre-increment operator,\n> > > and operator++(int) the post-increment operator.\n> > >\n> > > > > +         {\n> > > > > +                 Derived it = *static_cast<Derived *>(this);\n> > > > > +                 it_++;\n> > > > > +                 return it;\n> > > > > +         }\n> > > > > +\n> > > > > +         friend bool operator==(const Iterator &a, const Iterator &b)\n> > > > > +         {\n> > > > > +                 return a.it_ == b.it_;\n> > > > > +         }\n> > > > > +\n> > > > > +         friend bool operator!=(const Iterator &a, const Iterator &b)\n> > > > > +         {\n> > > > > +                 return a.it_ != b.it_;\n> > > > > +         }\n> > > > > +\n> > > > > + protected:\n> > > > > +         typename Container::const_iterator it_;\n> > > > > + };\n> > > > > +\n> > > > > + template<typename Container, typename Iterator>\n> > > > > + class Adapter\n> > > > > + {\n> > > > > + public:\n> > > > > +         Adapter(const Container &container)\n> > > > > +                 : container_(container)\n> > > > > +         {\n> > > > > +         }\n> > > > > +\n> > > > > +         Iterator begin() const\n> > > > > +         {\n> > > > > +                 return Iterator{ container_.begin() };\n> > > > > +         }\n> > > > > +\n> > > > > +         Iterator end() const\n> > > > > +         {\n> > > > > +                 return Iterator{ container_.end() };\n> > > > > +         }\n> > > > > +\n> > > > > + protected:\n> > > > > +         const Container &container_;\n> > > > > + };\n> > > > > +\n> > > > > + class ListIterator : public Iterator<ListContainer, ListIterator>\n> > > > > + {\n> > > > > + public:\n> > > > > +         using value_type = const YamlObject &;\n> > > > > +         using pointer = const YamlObject *;\n> > > > > +         using reference = value_type;\n> > > > > +\n> > > > > +         value_type operator*() const\n> > > > > +         {\n> > > > > +                 return *it_->get();\n> > > > > +         }\n> > > > > +\n> > > > > +         pointer operator->() const\n> > > > > +         {\n> > > > > +                 return it_->get();\n> > > > > +         }\n> > > > > + };\n> > > > > +\n> > > > > + class DictIterator : public Iterator<DictContainer, DictIterator>\n> > > > > + {\n> > > > > + public:\n> > > > > +         using value_type = std::pair<const std::string &, const YamlObject &>;\n> > > > > +         using pointer = value_type *;\n> > > > > +         using reference = value_type &;\n> > > > > +\n> > > > > +         value_type operator*() const\n> > > > > +         {\n> > > > > +                 return { it_->first, *it_->second.get() };\n> > > > > +         }\n> > > > > + };\n> > > > > +\n> > > > > + class DictAdapter : public Adapter<DictContainer, DictIterator>\n> > > > > + {\n> > > > > + public:\n> > > > > +         using key_type = std::string;\n> > > > > + };\n> > > > > +\n> > > > > + class ListAdapter : public Adapter<ListContainer, ListIterator>\n> > > > > + {\n> > > > > + };\n> > > > > +#endif /* __DOXYGEN__ */\n> > > > > +\n> > > > >   YamlObject();\n> > > > >   ~YamlObject();\n> > > > >\n> > > > > @@ -55,6 +165,9 @@ public:\n> > > > >  #endif\n> > > > >   T get(const T &defaultValue, bool *ok = nullptr) const;\n> > > > >\n> > > > > + DictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n> > > > > + ListAdapter asList() const { return ListAdapter{ list_ }; }\n> > > >\n> > > > When I first thought about making this iterable I was expecting\n> > > > simple wrappers around std::list iterator functions. That would not\n> > > > allow to have it as dict or as list. I wonder if providing both\n> > > > abstraction is necessary (my bad, I never played very much with this\n> > > > class).\n> > >\n> > > I would have liked a way to avoid asDict() and asList(). It's possible,\n> > > but then we would need the values to be stored in a single container\n> > > (done in the RFC v3 of the RPi YamlParser series), and the value_type of\n> > > the iterator to always be a pair, with the first member containing empty\n> > > strings when the YamlObject is a list. That's not very nice from an API\n> > > point of view.\n> > >\n> > > > More questions than comments, so I would say the patch looks good (as\n> > > > far as I understand it :p )\n> > >\n> > > Thank you :-)\n> > >\n> > > > > +\n> > > > >   const YamlObject &operator[](std::size_t index) const;\n> > > > >\n> > > > >   bool contains(const std::string &key) const;\n> > > > > @@ -75,8 +188,8 @@ private:\n> > > > >   Type type_;\n> > > > >\n> > > > >   std::string value_;\n> > > > > - std::vector<std::unique_ptr<YamlObject>> list_;\n> > > > > - std::map<const std::string, std::unique_ptr<YamlObject>> dictionary_;\n> > > > > + ListContainer list_;\n> > > > > + DictContainer dictionary_;\n> > > > >  };\n> > > > >\n> > > > >  class YamlParser final\n> > > > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > > > > index 85f6694f5fde..4df7e5a33d47 100644\n> > > > > --- a/src/libcamera/yaml_parser.cpp\n> > > > > +++ b/src/libcamera/yaml_parser.cpp\n> > > > > @@ -259,6 +259,45 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const\n> > > > >\n> > > > >  #endif /* __DOXYGEN__ */\n> > > > >\n> > > > > +/**\n> > > > > + * \\fn YamlObject::asDict() const\n> > > > > + * \\brief Wrap a dictionary YamlObject in an adapter that exposes iterators\n> > > > > + *\n> > > > > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > > > > + * type depends on whether the object is a Dictionary or List. This function\n> > > > > + * wraps a YamlObject of Dictionary type into an adapter that exposes\n> > > > > + * iterators, as well as begin() and end() functions, allowing usage of\n> > > > > + * range-based for loops with YamlObject. As YAML mappings are not ordered, the\n> > > > > + * iteration order is not specified.\n> > > > > + *\n> > > > > + * The iterator's value_type is a\n> > > > > + * <em>std::pair<const std::string &, const \\ref YamlObject &></em>.\n> > > > > + *\n> > > > > + * If the YamlObject is not of Dictionary type, the returned adapter operates\n> > > > > + * as an empty container.\n> > > > > + *\n> > > > > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > > > > + */\n> > > > > +\n> > > > > +/**\n> > > > > + * \\fn YamlObject::asList() const\n> > > > > + * \\brief Wrap a list YamlObject in an adapter that exposes iterators\n> > > > > + *\n> > > > > + * The YamlObject class doesn't directly implement iterators, as the iterator\n> > > > > + * type depends on whether the object is a Dictionary or List. This function\n> > > > > + * wraps a YamlObject of List type into an adapter that exposes iterators, as\n> > > > > + * well as begin() and end() functions, allowing usage of range-based for loops\n> > > > > + * with YamlObject. As YAML lists are ordered, the iteration order is identical\n> > > > > + * to the list order in the YAML data.\n> > > > > + *\n> > > > > + * The iterator's value_type is a <em>const YamlObject &</em>.\n> > > > > + *\n> > > > > + * If the YamlObject is not of List type, the returned adapter operates as an\n> > > > > + * empty container.\n> > > > > + *\n> > > > > + * \\return An adapter of unspecified type compatible with range-based for loops\n> > > > > + */\n> > > > > +\n> > > > >  /**\n> > > > >   * \\fn YamlObject::operator[](std::size_t index) const\n> > > > >   * \\brief Retrieve the element from list YamlObject by index","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 E7E45BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jun 2022 12:10:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 996D765639;\n\tMon, 20 Jun 2022 14:10:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1FC0B65632\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jun 2022 14:10:10 +0200 (CEST)","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 8CC6725B;\n\tMon, 20 Jun 2022 14:10:09 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655727011;\n\tbh=KIO8BMfDp+FOQcOS3XOFgwbqdT4dQJKuGuls36kFqiI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pkNSAW14otciEHHE4OPjJIztr76AATplOu1QcjpHLaJHYopqyzMfWl6lFLJepKWZv\n\tZtNnaFShhne+octx17dIJVF5I47jybZ0Jk1A5/0n26CUTHPAJ6+KhieeNCIj3eoR2B\n\tZZqg37Zk42uh3jA3QXE9lL15HBJLcUgEZ3apWmrRSCXgzlfMzPjNTrDLScF7nMS6c5\n\tMyuaPD5Tm39HZT7CXPDugmhzU7ectnFpTk+q4CJpM9o9fH6LxLb5gSORZ0vRrWE8hN\n\tjveJr4fp960tzjxMiEj0Jqju9SJbyTfc0fh/YyYNZamQrg3UQSBKTbp8PaEjNRa81g\n\t6xPS3+ZcF9cug==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655727009;\n\tbh=KIO8BMfDp+FOQcOS3XOFgwbqdT4dQJKuGuls36kFqiI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EolhiagyLaBMQHiik5xl8L51A8hVYdT0vkjPNJ/UUgfhJ6DFRAqdKRwRLuXEWd+Dn\n\tsFLWDCQZ3HKjE09sN3i6pgkxTD1+tczowXPxFrFaHXnijEw4VsT+a2sF4wVfnSqUXZ\n\tqKA7NmnuvJRk1HHZQtAvg7rtUKa/EPaDjJLI8ofA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Eolhiagy\"; dkim-atps=neutral","Date":"Mon, 20 Jun 2022 15:09:54 +0300","To":"Hanlin Chen <hanlinchen@chromium.org>","Message-ID":"<YrBjktnCNESYdbIE@pendragon.ideasonboard.com>","References":"<20220616142403.20723-1-laurent.pinchart@ideasonboard.com>\n\t<20220616142403.20723-3-laurent.pinchart@ideasonboard.com>\n\t<20220617134505.3i3dxzpmrlnw46x7@uno.localdomain>\n\t<YqyLDB6Vq8rqqjuJ@pendragon.ideasonboard.com>\n\t<20220617142058.mbxyjxxamf5kvxvq@uno.localdomain>\n\t<CAJAuwM=VgRciUh3OcvrroV-6ehXj1OUriGMc9L6mucCL=bTDbg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAJAuwM=VgRciUh3OcvrroV-6ehXj1OUriGMc9L6mucCL=bTDbg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 2/7] libcamera: yaml_parser: Add\n\titerator API","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]