| Message ID | 20260113000808.15395-27-laurent.pinchart@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > ValueNode provides adapter classes to expose the object as an iteratable > list or dictionary. The Iterator and Adapter classes hardcode the > assumption that the ValueNode is const. To prepare for mutable views, > move the const specifier to the top-level DictAdapter and ListAdapter > class templates. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/value_node.h | 54 ++++++++++++++++--------- > 1 file changed, 35 insertions(+), 19 deletions(-) > > diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h > index eb509d855810..dd45859f3501 100644 > --- a/include/libcamera/internal/value_node.h > +++ b/include/libcamera/internal/value_node.h > @@ -38,14 +38,14 @@ private: > > public: > #ifndef __DOXYGEN__ > - template<typename Derived> > + template<typename Derived, typename ContainerIterator> > class Iterator > { > public: > using difference_type = std::ptrdiff_t; > using iterator_category = std::forward_iterator_tag; > > - Iterator(typename ValueContainer::const_iterator it) > + Iterator(ContainerIterator it) > : it_(it) > { > } > @@ -74,14 +74,14 @@ public: > } > > protected: > - ValueContainer::const_iterator it_; > + ContainerIterator it_; > }; > > - template<typename Iterator> > + template<typename Iterator, typename Container> > class Adapter > { > public: > - Adapter(const ValueContainer &container) > + Adapter(Container &container) > : container_(container) > { > } > @@ -97,47 +97,63 @@ public: > } > > protected: > - const ValueContainer &container_; > + Container &container_; > }; > > - class ListIterator : public Iterator<ListIterator> > + template<typename Value, typename ContainerIterator> > + class ListIterator : public Iterator<ListIterator<Value, ContainerIterator>, > + ContainerIterator> > { > - public: > - using value_type = const ValueNode &; > - using pointer = const ValueNode *; > - using reference = value_type; > + private: > + using Base = Iterator<ListIterator<Value, ContainerIterator>, > + ContainerIterator>; > > - value_type operator*() const > + public: > + using value_type = Value; > + using pointer = value_type *; > + using reference = value_type &; > + > + reference operator*() const > { > - return *it_->value.get(); > + return *Base::it_->value.get(); `Base` can be omitted here and removed altogether if one writes `this->it_`. Same applies everywhere else. > } > > pointer operator->() const > { > - return it_->value.get(); > + return Base::it_->value.get(); > } > }; > > - class DictIterator : public Iterator<DictIterator> > + template<typename Value, typename ContainerIterator> > + class DictIterator : public Iterator<DictIterator<Value, ContainerIterator>, > + ContainerIterator> > { > + private: > + using Base = Iterator<DictIterator<Value, ContainerIterator>, > + ContainerIterator>; > + > public: > - using value_type = std::pair<const std::string &, const ValueNode &>; > + using value_type = std::pair<const std::string &, Value &>; > using pointer = value_type *; > using reference = value_type &; > > value_type operator*() const I'm a bit concerned here because std::iterator_traits<...>::reference r = *it; will/does not work with dictionary iterators. But given that it is an internal type, as long as we don't run into issues, it is probably fine. Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > { > - return { it_->key, *it_->value.get() }; > + return { Base::it_->key, *Base::it_->value.get() }; > } > }; > > - class DictAdapter : public Adapter<DictIterator> > + class DictAdapter : public Adapter<DictIterator<const ValueNode, > + ValueContainer::const_iterator>, > + const ValueContainer> > { > public: > using key_type = std::string; > }; > > - class ListAdapter : public Adapter<ListIterator> > + class ListAdapter : public Adapter<ListIterator<const ValueNode, > + ValueContainer::const_iterator>, > + const ValueContainer> > { > }; > #endif /* __DOXYGEN__ */ > -- > Regards, > > Laurent Pinchart >
On Wed, Jan 14, 2026 at 10:56:38AM +0000, Barnabás Pőcze wrote: > 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > > ValueNode provides adapter classes to expose the object as an iteratable > > list or dictionary. The Iterator and Adapter classes hardcode the > > assumption that the ValueNode is const. To prepare for mutable views, > > move the const specifier to the top-level DictAdapter and ListAdapter > > class templates. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/value_node.h | 54 ++++++++++++++++--------- > > 1 file changed, 35 insertions(+), 19 deletions(-) > > > > diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h > > index eb509d855810..dd45859f3501 100644 > > --- a/include/libcamera/internal/value_node.h > > +++ b/include/libcamera/internal/value_node.h > > @@ -38,14 +38,14 @@ private: > > > > public: > > #ifndef __DOXYGEN__ > > - template<typename Derived> > > + template<typename Derived, typename ContainerIterator> > > class Iterator > > { > > public: > > using difference_type = std::ptrdiff_t; > > using iterator_category = std::forward_iterator_tag; > > > > - Iterator(typename ValueContainer::const_iterator it) > > + Iterator(ContainerIterator it) > > : it_(it) > > { > > } > > @@ -74,14 +74,14 @@ public: > > } > > > > protected: > > - ValueContainer::const_iterator it_; > > + ContainerIterator it_; > > }; > > > > - template<typename Iterator> > > + template<typename Iterator, typename Container> > > class Adapter > > { > > public: > > - Adapter(const ValueContainer &container) > > + Adapter(Container &container) > > : container_(container) > > { > > } > > @@ -97,47 +97,63 @@ public: > > } > > > > protected: > > - const ValueContainer &container_; > > + Container &container_; > > }; > > > > - class ListIterator : public Iterator<ListIterator> > > + template<typename Value, typename ContainerIterator> > > + class ListIterator : public Iterator<ListIterator<Value, ContainerIterator>, > > + ContainerIterator> > > { > > - public: > > - using value_type = const ValueNode &; > > - using pointer = const ValueNode *; > > - using reference = value_type; > > + private: > > + using Base = Iterator<ListIterator<Value, ContainerIterator>, > > + ContainerIterator>; > > > > - value_type operator*() const > > + public: > > + using value_type = Value; > > + using pointer = value_type *; > > + using reference = value_type &; > > + > > + reference operator*() const > > { > > - return *it_->value.get(); > > + return *Base::it_->value.get(); > > `Base` can be omitted here and removed altogether if one writes `this->it_`. > Same applies everywhere else. Is there a reason to prefer one over the other ? > > } > > > > pointer operator->() const > > { > > - return it_->value.get(); > > + return Base::it_->value.get(); > > } > > }; > > > > - class DictIterator : public Iterator<DictIterator> > > + template<typename Value, typename ContainerIterator> > > + class DictIterator : public Iterator<DictIterator<Value, ContainerIterator>, > > + ContainerIterator> > > { > > + private: > > + using Base = Iterator<DictIterator<Value, ContainerIterator>, > > + ContainerIterator>; > > + > > public: > > - using value_type = std::pair<const std::string &, const ValueNode &>; > > + using value_type = std::pair<const std::string &, Value &>; > > using pointer = value_type *; > > using reference = value_type &; > > > > value_type operator*() const > > I'm a bit concerned here because > > std::iterator_traits<...>::reference r = *it; > > will/does not work with dictionary iterators. But given that it is an internal type, > as long as we don't run into issues, it is probably fine. More effort is needed to make ValueNode a public class, if we ever decide to do so. In that case I would be fixing this too. > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > { > > - return { it_->key, *it_->value.get() }; > > + return { Base::it_->key, *Base::it_->value.get() }; > > } > > }; > > > > - class DictAdapter : public Adapter<DictIterator> > > + class DictAdapter : public Adapter<DictIterator<const ValueNode, > > + ValueContainer::const_iterator>, > > + const ValueContainer> > > { > > public: > > using key_type = std::string; > > }; > > > > - class ListAdapter : public Adapter<ListIterator> > > + class ListAdapter : public Adapter<ListIterator<const ValueNode, > > + ValueContainer::const_iterator>, > > + const ValueContainer> > > { > > }; > > #endif /* __DOXYGEN__ */
diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h index eb509d855810..dd45859f3501 100644 --- a/include/libcamera/internal/value_node.h +++ b/include/libcamera/internal/value_node.h @@ -38,14 +38,14 @@ private: public: #ifndef __DOXYGEN__ - template<typename Derived> + template<typename Derived, typename ContainerIterator> class Iterator { public: using difference_type = std::ptrdiff_t; using iterator_category = std::forward_iterator_tag; - Iterator(typename ValueContainer::const_iterator it) + Iterator(ContainerIterator it) : it_(it) { } @@ -74,14 +74,14 @@ public: } protected: - ValueContainer::const_iterator it_; + ContainerIterator it_; }; - template<typename Iterator> + template<typename Iterator, typename Container> class Adapter { public: - Adapter(const ValueContainer &container) + Adapter(Container &container) : container_(container) { } @@ -97,47 +97,63 @@ public: } protected: - const ValueContainer &container_; + Container &container_; }; - class ListIterator : public Iterator<ListIterator> + template<typename Value, typename ContainerIterator> + class ListIterator : public Iterator<ListIterator<Value, ContainerIterator>, + ContainerIterator> { - public: - using value_type = const ValueNode &; - using pointer = const ValueNode *; - using reference = value_type; + private: + using Base = Iterator<ListIterator<Value, ContainerIterator>, + ContainerIterator>; - value_type operator*() const + public: + using value_type = Value; + using pointer = value_type *; + using reference = value_type &; + + reference operator*() const { - return *it_->value.get(); + return *Base::it_->value.get(); } pointer operator->() const { - return it_->value.get(); + return Base::it_->value.get(); } }; - class DictIterator : public Iterator<DictIterator> + template<typename Value, typename ContainerIterator> + class DictIterator : public Iterator<DictIterator<Value, ContainerIterator>, + ContainerIterator> { + private: + using Base = Iterator<DictIterator<Value, ContainerIterator>, + ContainerIterator>; + public: - using value_type = std::pair<const std::string &, const ValueNode &>; + using value_type = std::pair<const std::string &, Value &>; using pointer = value_type *; using reference = value_type &; value_type operator*() const { - return { it_->key, *it_->value.get() }; + return { Base::it_->key, *Base::it_->value.get() }; } }; - class DictAdapter : public Adapter<DictIterator> + class DictAdapter : public Adapter<DictIterator<const ValueNode, + ValueContainer::const_iterator>, + const ValueContainer> { public: using key_type = std::string; }; - class ListAdapter : public Adapter<ListIterator> + class ListAdapter : public Adapter<ListIterator<const ValueNode, + ValueContainer::const_iterator>, + const ValueContainer> { }; #endif /* __DOXYGEN__ */
ValueNode provides adapter classes to expose the object as an iteratable list or dictionary. The Iterator and Adapter classes hardcode the assumption that the ValueNode is const. To prepare for mutable views, move the const specifier to the top-level DictAdapter and ListAdapter class templates. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/value_node.h | 54 ++++++++++++++++--------- 1 file changed, 35 insertions(+), 19 deletions(-)