Opened 16 years ago

Closed 16 years ago

#695 closed Bugs (Fixed)

permutation_iterator does not support conversions

Reported by: daniel_kruegler Owned by: david_abrahams
Milestone: Component: iterator
Version: None Severity:
Keywords: Cc:

Description

The current implementation of
boost::permutation_iterator does not support compatible
conversions. The below attached example demonstrates
the problem. The reason is due to the fact that the
converting c'tor

template<class OtherElementIterator, class
OtherIndexIterator>
permutation_iterator(
permutation_iterator<OtherElementIterator,
OtherIndexIterator> const& r
      , typename
enable_if_convertible<OtherElementIterator,
ElementIterator>::type* = 0
      , typename
enable_if_convertible<OtherIndexIterator,
IndexIterator>::type* = 0
      )
    : super_t(r.base()), m_elt_iter(r.m_elt_iter)
  {}

is provoked in the example and is generally defective:
There exists no public accessor for the private member
m_elt_iter and thus it's access is not allowed at the
point of its copy construction.

There exists two solutions to fix this issue. Since I'm
not sure, which is the best one, I propose both:

Solution (1): Add

template <typename, typename> friend class
permutation_iterator;
  
inside the permutation_iterator class template.

Advantage: Does not change the current interface of
permutation_iterator<>, disadvantage: It provides
friendship to every template
permutation_iterator<typename>, which could
theoretically be hijacked be programmers, which define
their own template specialization of the c'tor (See
http://www.gotw.ca/gotw/076.htm "The Language Lawyer"),
although that risk is low.

Solution (2): Provide a public accessor for member
m_elt_iter, e.g.

ElementIterator element_iterator() const { return
m_elt_iter; }

and call this member function in the conversion c'tor
instead of direct member access.

The advantage is that no friendship is needed (;-)),
the disadvantage is, that it adds a new public member.
I actual problem with that accessor for the internal
element iterator is: What expect users, which position
it corresponds to? Is it its current position or is it
its initial position (which is true, of course). The
last argument could be weakened by enforcing a more
descriptive name, e.g. primary_element_iterator().

I will provide cvs patches in the following minutes

Change History (3)

comment:1 by daniel_kruegler, 16 years ago

Logged In: YES 
user_id=1540640

cvs patch for solution (1)

Index: permutation_iterator.hpp
===================================================================
RCS file:
/cvsroot/boost/boost/boost/iterator/permutation_iterator.hpp,v
retrieving revision 1.4
diff -r1.4 permutation_iterator.hpp
33a34,35
>   template <typename, typename> friend class
permutation_iterator;
>   

comment:2 by daniel_kruegler, 16 years ago

Logged In: YES 
user_id=1540640

cvs patch for solution (2)

Index: permutation_iterator.hpp
===================================================================
RCS file:
/cvsroot/boost/boost/boost/iterator/permutation_iterator.hpp,v
retrieving revision 1.4
diff -r1.4 permutation_iterator.hpp
46c46
<     : super_t(r.base()), m_elt_iter(r.m_elt_iter)
---
>     : super_t(r.base()), m_elt_iter(r.element_iterator())
47a48,49
>   
>   ElementIterator element_iterator() const { return
m_elt_iter; }

comment:3 by david_abrahams, 16 years ago

Status: assignedclosed
Note: See TracTickets for help on using tickets.