Re: [boost] [gil] Is bit_aligned_pixel_iterator a standard conformant random access iterator?

Subject: Re: [boost] [gil] Is bit_aligned_pixel_iterator a standard conformant random access iterator?
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-05-31 19:53:30

Hi Adam,

On Mon, May 31, 2010 at 1:25 PM, Adam Merz <adammerz_at_[hidden]> wrote:
> Hi Christian, this is ildjarn from the MSDN forums. :-]
>

Interesting disguise. ;-)

> To be clear, it's not that MSVC10 doesn't understand that
> bit_aligned_pixel_iterator is a random access iterator, it's that
> bit_aligned_pixel_iterator *isn't* a random access iterator. I.e.,
> std::iterator_traits< bit_aligned_pixel_iterator >::iterator_category is
> std::input_iterator_tag on every compiler/platform. It's just that MSVC10
> refuses to instantiate std::copy given a destination iterator categorized as an
> input iterator (which is a good thing). If GIL had a regression test with a
> static assertion that boost::is_same< std::iterator_traits<
> bit_aligned_pixel_iterator<...> >::iterator_category,
> std::random_access_iterator_tag >::value == true, this would have been caught
> long ago. (hint, hint)
>
> One perspective is that the bug here is that bit_aligned_pixel_iterator is
> trying to pass itself off as a random access iterator in the first place,
> despite the fact that it returns a proxy object rather than an actual
> reference, violating the standard's requirements for forward iterators.
> However, since it appears to work properly on all tested toolsets, that could
> be deemed an acceptable flaw, in which case the proper fix is to pass
> iterator_facade a category of std::random_access_iterator_tag rather than
> boost::random_access_traversal_tag. Easy enough. :-]

Easy enough, correct. I did this change to my local boost for a couple
of gil's iterators. I have less compiler errors now. Do older MSVC
versions and other compiler define std::random_access_iterator_tag? If
though, I could add this fix to boost, as is.

>
> Another perspective is that the bug here is caused by iterator_facade silently
> decaying into an input iterator despite being told to function as a random
> access iterator. This is documented behavior, so it's not a bug in and of
> itself, but the fact that it does so *silently* is a questionable design issue
> given that it's negatively affected at least two released libraries so far. I'm
> not sure what mechanism could be used to make the decay 'noisy' rather than
> silent, but this is clearly a gotcha for users of iterator_facade. Also, the
> fact that it decays into an input iterator rather than an input/output iterator
> seems arbitrary and less than ideal. Why doesn't it decay into
> boost::detail::input_output_iterator_tag? At least then it could be used as a
> destination for std::copy.

I really have a hard understanding where and when the metafunction
boost::detail::iterator_facade_default_category is actually reducing
bit_aligned_pixel_iterator to an input iterator?

When I do this:

using namespace boost;
using namespace gil;

typedef bit_aligned_image1_type< 1, gray_layout_t >::type image_t;
typedef image_t::view_t    view_t;
typedef view_t::reference  pixel_t;

typedef bit_aligned_pixel_iterator< pixel_t > it_t;
typedef iterator_traits< it_t >::iterator_category it_cat_t;

it_cat_t is still
boost::detail::iterator_category_with_traversal<std::input_iterator_tag,boost::random_access_traversal_tag>.
Though, where does the "decaying" happen?

Regards,
Christian