Re: [gdal-dev] Use of C++ iterators in API
From: Even Rouault [mailto:even.roua...@spatialys.com] > I just planted the seed. Contributions (preferably as pull requests) > welcome... Ok just did that PR #411. ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
> I would recommend that Layers::iterator is updated to conform to the C++ > iterator concepts (to the very least to the InputIterator concept). It > still requires the following: > - five typedefs (value_type, reference, pointer, iterator_category, > difference_type) - three constructors (default, copy, move) > - two assignment operators (copy, move) > - post-increment operator (operator++(int)) > > See also http://en.cppreference.com/w/cpp/concept/InputIterator. I just planted the seed. Contributions (preferably as pull requests) welcome... > Considering the potential confusion, you can just avoid the issue completely > and write: >for(const OGRLayer* layer : layers) Fair enough. I'll use this. -- Spatialys - Geospatial professional services http://www.spatialys.com ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
> Even: > In fact the nature of the object which is returned depends on the iterator, > and as documented in the API, for the sake of implementation simplicity, > those iterators do not necessarily respect the whole semantics of iterators. I don't think that is necessary, at least in the case of Layers you can conform to the InputIterator concept without much extra work [snip] > So basically you should only use them in range based loops, where you don't > really manipulate the iterator and cannot do stuff like the above. That is also the limitation of InputIterator. > Improvements (if doable and not adding (too much) overhead to the underlying > objects) are welcome of course. I would recommend that Layers::iterator is updated to conform to the C++ iterator concepts (to the very least to the InputIterator concept). It still requires the following: - five typedefs (value_type, reference, pointer, iterator_category, difference_type) - three constructors (default, copy, move) - two assignment operators (copy, move) - post-increment operator (operator++(int)) See also http://en.cppreference.com/w/cpp/concept/InputIterator. * Regarding the for(auto&& layer : layers) discussion: The return type of Layers::Iterator::operator*() is OGRLayer*. Therefore, the following: for(auto& layer : layers) is incorrect and should not compile, because it binds a temporary to a reference. Furthermore, the following: for(const auto& layer: layers) is only correct because an assignment of a temporary to a const reference extends its lifetime, and it is therefore equivalent to for(const auto layer : layers) Considering the potential confusion, you can just avoid the issue completely and write: for(const OGRLayer* layer : layers) Kind regards, Alex ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On 11 April 2018 at 14:40, Even Rouault wrote: > On mercredi 11 avril 2018 05:10:53 CEST Kurt Schwehr wrote: >> The auto&& is really confusing. > > A experimentally working alternative with miminal use of '&' is > > for( auto poLayer: poDS->GetLayers() ) <-- this is a plain OGRLayer* IMHO, it's better to get users familar with these two: auto const& - as preferred default auto& - where one requires, explicitly It should cover 99.9% of uses :) Best regards, -- Mateusz Loskot, http://mateusz.loskot.net ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On Wed, Apr 11, 2018 at 8:40 AM, Even Rouault
wrote:
> On mercredi 11 avril 2018 05:10:53 CEST Kurt Schwehr wrote:
> > The auto&& is really confusing.
>
> A experimentally working alternative with miminal use of '&' is
>
> for( auto poLayer: poDS->GetLayers() ) <-- this is a plain OGRLayer*
> {
> for( auto& poFeature: *poLayer ) <-- this is a
> unique_ptr so needs to be obtained by reference (otherwise
> compiler complains about use of deleted function unique_ptr(const
> unique_ptr&) = delete
> {
> for( auto& oField: *poFeature ) <-- this is a
> OGRFeature::FieldValue object that has no copy constructor (intended, we
> don't want user to be able to instanciate standalone FieldValue)
>
> Does that look better ?
>
> In all those above cases, the auto could also be const'ified, so the
> stricter typing would be
>
> for( const auto poLayer: poDS->GetLayers() )
> {
> for( const auto& poFeature: *poLayer )
> {
> for( const auto& oField: *poFeature )
> Should we put that instead ?
>
I believe the universal reference (&&) has no purpose in this context. It
will decay to a standard reference. So what you post is more clear.
--
Andrew Bell
andrew.bell...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On mercredi 11 avril 2018 05:10:53 CEST Kurt Schwehr wrote:
> The auto&& is really confusing.
A experimentally working alternative with miminal use of '&' is
for( auto poLayer: poDS->GetLayers() ) <-- this is a plain OGRLayer*
{
for( auto& poFeature: *poLayer ) <-- this is a unique_ptr
so needs to be obtained by reference (otherwise compiler complains about use of
deleted function unique_ptr(const unique_ptr&) = delete
{
for( auto& oField: *poFeature ) <-- this is a
OGRFeature::FieldValue object that has no copy constructor (intended, we don't
want user to be able to instanciate standalone FieldValue)
Does that look better ?
In all those above cases, the auto could also be const'ified, so the stricter
typing would be
for( const auto poLayer: poDS->GetLayers() )
{
for( const auto& poFeature: *poLayer )
{
for( const auto& oField: *poFeature )
Should we put that instead ?
--
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
The auto&& is really confusing. -kurt On Tue, Apr 10, 2018 at 2:41 PM, Mateusz Loskot wrote: > On 10 April 2018 at 23:06, Even Rouault > wrote: > > On mardi 10 avril 2018 22:34:56 CEST Mateusz Loskot wrote: > >> On 10 April 2018 at 22:06, Even Rouault > wrote: > >> > > >> > Just wanted to advertize the recent new introduction of C++ iterators > >> > (...) > >> > >> Good one, thanks! > >> > >> > for( auto&& poLayer: poDS->GetLayers() ) > >> > >> Do the iterators return a proxy reference hence the tutorial prefers > >> auto&& over plain auto&? > >> (ie. like std::vector iterator does) > > > > Not completely sure to understand what a proxy reference is (C++11 stuff > is > > still very new to me, and I don't pretend to master C++98, so ...), but > I feel > > that must be close to what I have used in some cases, particularly wen > > iterating over fields of a features, or points of a linestring. > > I didn't look at the iterators impl. yet. > I just looked at your example and wondered if there are proxies > - for GDAL/OGR, those could be necessary, I imagine. > > I mean a proxy as an object that 'looks' like a reference. > It's often necessary to for (proxied) collections of objects that can not > be > accessed directly, via regular pointers/references. > > > I read in some tutorial that using auto&& worked in all situations, > including > > when the returned reference was const, so now I just use that everywhere. > > auto&& is most useful when container::reference returns a proxy object, > which would not bind to auto& (it would bind to auto const& though). > > > In fact the nature of the object which is returned depends on the > iterator, > > and as documented in the API, for the sake of implementation simplicity, > those > > iterators do not necessarily respect the whole semantics of iterators. > > > > That is if you do > > > > auto iter = poLineString->begin(); > > OGRPoint& oPoint = *iter; > > ++iter; > > OGRPoint& oAnotherPoint = *iter; > > In fact &oAnotherPoint == &oPoint. > > At first, it seems like they model the input iterators, single-pass > iterator category > http://en.cppreference.com/w/cpp/concept/InputIterator > > > So basically you should only use them in range based loops, where you > don't > > really manipulate the iterator and cannot do stuff like the above. > > Or, use like any input iterators, seems safe. > > > I guess the "correct" solution would have been to return a OGRPointProxy > that > > would keep a link to the linestring and the point index. But not sure > how you > > would do that for a const iterator where the operator * is supposed to > > returned a const reference... > > Or reference-like proxy like std::vector does. > > Best regards, > -- > Mateusz Loskot, http://mateusz.loskot.net > ___ > gdal-dev mailing list > gdal-dev@lists.osgeo.org > https://lists.osgeo.org/mailman/listinfo/gdal-dev > -- -- http://schwehr.org ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On 10 April 2018 at 23:06, Even Rouault wrote: > On mardi 10 avril 2018 22:34:56 CEST Mateusz Loskot wrote: >> On 10 April 2018 at 22:06, Even Rouault wrote: >> > >> > Just wanted to advertize the recent new introduction of C++ iterators >> > (...) >> >> Good one, thanks! >> >> > for( auto&& poLayer: poDS->GetLayers() ) >> >> Do the iterators return a proxy reference hence the tutorial prefers >> auto&& over plain auto&? >> (ie. like std::vector iterator does) > > Not completely sure to understand what a proxy reference is (C++11 stuff is > still very new to me, and I don't pretend to master C++98, so ...), but I feel > that must be close to what I have used in some cases, particularly wen > iterating over fields of a features, or points of a linestring. I didn't look at the iterators impl. yet. I just looked at your example and wondered if there are proxies - for GDAL/OGR, those could be necessary, I imagine. I mean a proxy as an object that 'looks' like a reference. It's often necessary to for (proxied) collections of objects that can not be accessed directly, via regular pointers/references. > I read in some tutorial that using auto&& worked in all situations, including > when the returned reference was const, so now I just use that everywhere. auto&& is most useful when container::reference returns a proxy object, which would not bind to auto& (it would bind to auto const& though). > In fact the nature of the object which is returned depends on the iterator, > and as documented in the API, for the sake of implementation simplicity, those > iterators do not necessarily respect the whole semantics of iterators. > > That is if you do > > auto iter = poLineString->begin(); > OGRPoint& oPoint = *iter; > ++iter; > OGRPoint& oAnotherPoint = *iter; > In fact &oAnotherPoint == &oPoint. At first, it seems like they model the input iterators, single-pass iterator category http://en.cppreference.com/w/cpp/concept/InputIterator > So basically you should only use them in range based loops, where you don't > really manipulate the iterator and cannot do stuff like the above. Or, use like any input iterators, seems safe. > I guess the "correct" solution would have been to return a OGRPointProxy that > would keep a link to the linestring and the point index. But not sure how you > would do that for a const iterator where the operator * is supposed to > returned a const reference... Or reference-like proxy like std::vector does. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On mardi 10 avril 2018 22:34:56 CEST Mateusz Loskot wrote: > On 10 April 2018 at 22:06, Even Rouault wrote: > > Hi, > > > > Just wanted to advertize the recent new introduction of C++ iterators > > (...) > > Good one, thanks! > > > for( auto&& poLayer: poDS->GetLayers() ) > > Do the iterators return a proxy reference hence the tutorial prefers > auto&& over plain auto&? > (ie. like std::vector iterator does) Not completely sure to understand what a proxy reference is (C++11 stuff is still very new to me, and I don't pretend to master C++98, so ...), but I feel that must be close to what I have used in some cases, particularly wen iterating over fields of a features, or points of a linestring. I read in some tutorial that using auto&& worked in all situations, including when the returned reference was const, so now I just use that everywhere. In fact the nature of the object which is returned depends on the iterator, and as documented in the API, for the sake of implementation simplicity, those iterators do not necessarily respect the whole semantics of iterators. That is if you do auto iter = poLineString->begin(); OGRPoint& oPoint = *iter; ++iter; OGRPoint& oAnotherPoint = *iter; In fact &oAnotherPoint == &oPoint. The reason is that internally a line string is not an array of OGRPoint, but an array of OGRRawPoint + optional array of z + optional array of m, so the iterator object only holds a single OGRPoint and always return the reference to it when you dereference it, and updates its content when you increment it. So basically you should only use them in range based loops, where you don't really manipulate the iterator and cannot do stuff like the above. I guess the "correct" solution would have been to return a OGRPointProxy that would keep a link to the linestring and the point index. But not sure how you would do that for a const iterator where the operator * is supposed to returned a const reference... Except allocating a std::array in the iterator, which seems overkill. Improvements (if doable and not adding (too much) overhead to the underlying objects) are welcome of course. Even -- Spatialys - Geospatial professional services http://www.spatialys.com ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On 10 April 2018 at 22:06, Even Rouault wrote: > Hi, > > Just wanted to advertize the recent new introduction of C++ iterators (...) Good one, thanks! > for( auto&& poLayer: poDS->GetLayers() ) Do the iterators return a proxy reference hence the tutorial prefers auto&& over plain auto&? (ie. like std::vector iterator does) Best regards, -- Mateusz Loskot, http://mateusz.loskot.net ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
