DGtal-team / DGtal

Digital Geometry Tools and Algorithm Library
https://dgtal.org
GNU Lesser General Public License v3.0
370 stars 115 forks source link

ConstIteratorAdapter does not satisfy the _is_forward concept of the STL #1437

Open dcoeurjo opened 5 years ago

dcoeurjo commented 5 years ago

On the last apple clang compiler, the following code raises a build error

https://github.com/DGtal-team/DGtal/blob/c0b08384ff210fb33fb0f1067994457b3837eb9d/examples/geometry/volumes/distance/toricdomainvolumetric.cpp#L119

Here you have the error:


/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2494:5: error: static_assert failed due to requirement
      '__is_forward_iterator<DGtal::ConstIteratorAdapter<DGtal::HyperRectDomain_Iterator<DGtal::PointVector<2, int, std::__1::array<int, 2> > >, DGtal::DistanceTransformation<DGtal::SpaceND<2, int>,
      DGtal::functors::SimpleThresholdForegroundPredicate<DGtal::ImageContainerBySTLVector<DGtal::HyperRectDomain<DGtal::SpaceND<2, int> >, unsigned int> >, DGtal::ExactPredicateLpSeparableMetric<DGtal::SpaceND<2, int>, 2, long long>,
      DGtal::ImageContainerBySTLVector<DGtal::HyperRectDomain<DGtal::SpaceND<2, int> >, DGtal::PointVector<2, int, std::__1::array<int, 2> > > >, double> >::value' "std::max_element requires a ForwardIterator"
    static_assert(__is_forward_iterator<_ForwardIterator>::value,
    ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2512:19: note: in instantiation of function template specialization
      'std::__1::max_element<DGtal::ConstIteratorAdapter<DGtal::HyperRectDomain_Iterator<DGtal::PointVector<2, int, std::__1::array<int, 2> > >, DGtal::DistanceTransformation<DGtal::SpaceND<2, int>,
      DGtal::functors::SimpleThresholdForegroundPredicate<DGtal::ImageContainerBySTLVector<DGtal::HyperRectDomain<DGtal::SpaceND<2, int> >, unsigned int> >, DGtal::ExactPredicateLpSeparableMetric<DGtal::SpaceND<2, int>, 2, long long>,
      DGtal::ImageContainerBySTLVector<DGtal::HyperRectDomain<DGtal::SpaceND<2, int> >, DGtal::PointVector<2, int, std::__1::array<int, 2> > > >, double>, std::__1::__less<double, double> >' requested here
    return _VSTD::max_element(__first, __last,
                  ^
/Users/davidcoeurjolly/Sources/DGtal/examples/geometry/volumes/distance/toricdomainvolumetric.cpp:119:49: note: in instantiation of function template specialization 'std::__1::max_element<DGtal::ConstIteratorAdapter<DGtal::HyperRectDomain_Iterator<DGtal::PointVector<2,
      int, std::__1::array<int, 2> > >, DGtal::DistanceTransformation<DGtal::SpaceND<2, int>, DGtal::functors::SimpleThresholdForegroundPredicate<DGtal::ImageContainerBySTLVector<DGtal::HyperRectDomain<DGtal::SpaceND<2, int> >, unsigned int> >,
      DGtal::ExactPredicateLpSeparableMetric<DGtal::SpaceND<2, int>, 2, long long>, DGtal::ImageContainerBySTLVector<DGtal::HyperRectDomain<DGtal::SpaceND<2, int> >, DGtal::PointVector<2, int, std::__1::array<int, 2> > > >, double> >' requested here
  const DTL2::Value       maxv2       = * (std::max_element(dtL2.constRange().begin(), dtL2.constRange().end()));
                                                ^
1 error generated.
``` 
kerautret commented 4 years ago

I confirm ;(

rolanddenis commented 4 years ago

Can one of you test to replace std::max_element by boost::first_max_element that is available in boost/algorithm/minmax_element.hpp? It is a proposed fix from https://github.com/boostorg/graph/issues/175 even if it will not solve the fact that DGtal::ConstIteratorAdapter is not strictly a forward iterator.

kerautret commented 4 years ago

thanks @rolanddenis I look it

kerautret commented 4 years ago

@rolanddenis excellent that fix it 🎉 Did I put in PR the change ?

kerautret commented 4 years ago

it is here: https://github.com/kerautret/DGtal/tree/FixConstIteratorIssue1437

rolanddenis commented 4 years ago

Thx, agree for the PR !

BTW I was more thinking in replacing by the boost version only where needed (with smart iterators like ConstIteratorAdaptor), so probably only in:

We may also add a note in the ConstIteratorAdapterdocumentation (I don't think there is a way to make it a true forward iterator...)

kerautret commented 4 years ago

Yes agree I do it thanks