| Message ID | 20260120144431.264758-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On Tue, Jan 20, 2026 at 03:44:31PM +0100, Barnabás Pőcze wrote: > For example, `std::span` does not have a `const_iterator` typedef before > C++23, so compilation fails. Simply use `auto`, the `const` qualifier on > the `items` variable should already ensure that, if one exists, a "const" > iterator will be used. What will be used with C++20, std::span::iterator ? I'm surprised that C++20 doesn't have a const_iterator for std::span, but as std::span<const T>::iterator is not mutable, I see no issue with this patch. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > include/libcamera/base/utils.h | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h > index 0b7407f77..2cb8e0a88 100644 > --- a/include/libcamera/base/utils.h > +++ b/include/libcamera/base/utils.h > @@ -110,8 +110,7 @@ std::string join(const Container &items, const std::string &sep, UnaryOp op) > std::ostringstream ss; > bool first = true; > > - for (typename Container::const_iterator it = std::begin(items); > - it != std::end(items); ++it) { > + for (auto it = std::begin(items); it != std::end(items); ++it) { > if (!first) > ss << sep; > else > @@ -129,8 +128,7 @@ std::string join(const Container &items, const std::string &sep) > std::ostringstream ss; > bool first = true; > > - for (typename Container::const_iterator it = std::begin(items); > - it != std::end(items); ++it) { > + for (auto it = std::begin(items); it != std::end(items); ++it) { > if (!first) > ss << sep; > else
Quoting Laurent Pinchart (2026-01-20 18:03:45) > On Tue, Jan 20, 2026 at 03:44:31PM +0100, Barnabás Pőcze wrote: > > For example, `std::span` does not have a `const_iterator` typedef before > > C++23, so compilation fails. Simply use `auto`, the `const` qualifier on > > the `items` variable should already ensure that, if one exists, a "const" > > iterator will be used. > > What will be used with C++20, std::span::iterator ? > > I'm surprised that C++20 doesn't have a const_iterator for std::span, > but as std::span<const T>::iterator is not mutable, I see no issue with > this patch. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > --- > > include/libcamera/base/utils.h | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h > > index 0b7407f77..2cb8e0a88 100644 > > --- a/include/libcamera/base/utils.h > > +++ b/include/libcamera/base/utils.h > > @@ -110,8 +110,7 @@ std::string join(const Container &items, const std::string &sep, UnaryOp op) > > std::ostringstream ss; > > bool first = true; > > > > - for (typename Container::const_iterator it = std::begin(items); > > - it != std::end(items); ++it) { > > + for (auto it = std::begin(items); it != std::end(items); ++it) { > > if (!first) > > ss << sep; > > else > > @@ -129,8 +128,7 @@ std::string join(const Container &items, const std::string &sep) > > std::ostringstream ss; > > bool first = true; > > > > - for (typename Container::const_iterator it = std::begin(items); > > - it != std::end(items); ++it) { > > + for (auto it = std::begin(items); it != std::end(items); ++it) { > > if (!first) > > ss << sep; > > else > > -- > Regards, > > Laurent Pinchart
2026. 01. 20. 19:03 keltezéssel, Laurent Pinchart írta: > On Tue, Jan 20, 2026 at 03:44:31PM +0100, Barnabás Pőcze wrote: >> For example, `std::span` does not have a `const_iterator` typedef before >> C++23, so compilation fails. Simply use `auto`, the `const` qualifier on >> the `items` variable should already ensure that, if one exists, a "const" >> iterator will be used. > > What will be used with C++20, std::span::iterator ? Yes. > > I'm surprised that C++20 doesn't have a const_iterator for std::span, > but as std::span<const T>::iterator is not mutable, I see no issue with > this patch. I imagine the motivation might have been that dereferencing a `T * const` gives `T&`. So `*((const std::span<T>)::begin())` should be the same. And one can trivially go from a const span/pointer to a non-cost one, it is not really possible to enforce any const-ness if `T` is not `const`. But in C++23 they added `std::basic_const_iterator` that wraps an iterator and does not allow mutation. So they could easily add a const iterator to std::span now. `std::ranges::cbegin()` (but not `std::cbegin()` apparently) has also been changed to try to return such immutable iterators. So things appear to be moving towards ensuring that an iterator from `c{,r}{begin,end}()` does not allow mutation. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> include/libcamera/base/utils.h | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h >> index 0b7407f77..2cb8e0a88 100644 >> --- a/include/libcamera/base/utils.h >> +++ b/include/libcamera/base/utils.h >> @@ -110,8 +110,7 @@ std::string join(const Container &items, const std::string &sep, UnaryOp op) >> std::ostringstream ss; >> bool first = true; >> >> - for (typename Container::const_iterator it = std::begin(items); >> - it != std::end(items); ++it) { >> + for (auto it = std::begin(items); it != std::end(items); ++it) { >> if (!first) >> ss << sep; >> else >> @@ -129,8 +128,7 @@ std::string join(const Container &items, const std::string &sep) >> std::ostringstream ss; >> bool first = true; >> >> - for (typename Container::const_iterator it = std::begin(items); >> - it != std::end(items); ++it) { >> + for (auto it = std::begin(items); it != std::end(items); ++it) { >> if (!first) >> ss << sep; >> else > > -- > Regards, > > Laurent Pinchart
2026. 01. 23. 11:28 keltezéssel, Barnabás Pőcze írta: > 2026. 01. 20. 19:03 keltezéssel, Laurent Pinchart írta: >> On Tue, Jan 20, 2026 at 03:44:31PM +0100, Barnabás Pőcze wrote: >>> For example, `std::span` does not have a `const_iterator` typedef before >>> C++23, so compilation fails. Simply use `auto`, the `const` qualifier on >>> the `items` variable should already ensure that, if one exists, a "const" >>> iterator will be used. >> >> What will be used with C++20, std::span::iterator ? > > Yes. Just to clarify: std::span<...>::begin() always returns `std::span<...>::iterator`, even if the span itself is const qualified. Since C++23 there is `c{,r}begin()` that always returns an "immutable" iterator (`std::span<...>::const_iterator`), even if the span itself is not const qualified. > > >> >> I'm surprised that C++20 doesn't have a const_iterator for std::span, >> but as std::span<const T>::iterator is not mutable, I see no issue with >> this patch. > > I imagine the motivation might have been that dereferencing a `T * const` > gives `T&`. So `*((const std::span<T>)::begin())` should be the same. And > one can trivially go from a const span/pointer to a non-cost one, it is > not really possible to enforce any const-ness if `T` is not `const`. > > But in C++23 they added `std::basic_const_iterator` that wraps an iterator > and does not allow mutation. So they could easily add a const iterator to > std::span now. `std::ranges::cbegin()` (but not `std::cbegin()` apparently) > has also been changed to try to return such immutable iterators. So things > appear to be moving towards ensuring that an iterator from `c{,r}{begin,end}()` > does not allow mutation. > > >> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> include/libcamera/base/utils.h | 6 ++---- >>> 1 file changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h >>> index 0b7407f77..2cb8e0a88 100644 >>> --- a/include/libcamera/base/utils.h >>> +++ b/include/libcamera/base/utils.h >>> @@ -110,8 +110,7 @@ std::string join(const Container &items, const std::string &sep, UnaryOp op) >>> std::ostringstream ss; >>> bool first = true; >>> >>> - for (typename Container::const_iterator it = std::begin(items); >>> - it != std::end(items); ++it) { >>> + for (auto it = std::begin(items); it != std::end(items); ++it) { >>> if (!first) >>> ss << sep; >>> else >>> @@ -129,8 +128,7 @@ std::string join(const Container &items, const std::string &sep) >>> std::ostringstream ss; >>> bool first = true; >>> >>> - for (typename Container::const_iterator it = std::begin(items); >>> - it != std::end(items); ++it) { >>> + for (auto it = std::begin(items); it != std::end(items); ++it) { >>> if (!first) >>> ss << sep; >>> else >> >> -- >> Regards, >> >> Laurent Pinchart >
diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h index 0b7407f77..2cb8e0a88 100644 --- a/include/libcamera/base/utils.h +++ b/include/libcamera/base/utils.h @@ -110,8 +110,7 @@ std::string join(const Container &items, const std::string &sep, UnaryOp op) std::ostringstream ss; bool first = true; - for (typename Container::const_iterator it = std::begin(items); - it != std::end(items); ++it) { + for (auto it = std::begin(items); it != std::end(items); ++it) { if (!first) ss << sep; else @@ -129,8 +128,7 @@ std::string join(const Container &items, const std::string &sep) std::ostringstream ss; bool first = true; - for (typename Container::const_iterator it = std::begin(items); - it != std::end(items); ++it) { + for (auto it = std::begin(items); it != std::end(items); ++it) { if (!first) ss << sep; else
For example, `std::span` does not have a `const_iterator` typedef before C++23, so compilation fails. Simply use `auto`, the `const` qualifier on the `items` variable should already ensure that, if one exists, a "const" iterator will be used. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/base/utils.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)