multi_pass cannot deal with all input iterators

[Sebastian Redl]

multi_pass has trouble with various input iterators.

First, Microsoft's istreambuf_iterator, because it doesn't implement DR445, reports its reference as Char&, but its operator* returns Char. This causes compilation errors all over the place.

Second, a standards-conformant istreambuf_iterator post-DR445 (i.e. C++0x) has both operator* return type and reference as Char. This compiles but is unsafe because InputPolicy::get_value is specified to return a const Value&, and this reference binds to the temporary returned by operator*. InputPolicy::get_value should return MultiPass::reference. This would also make the Value template parameter to this function superfluous.

Doing this change allows Spirit to compile with my own istreambuf_iterator replacement. I have no idea what to do about Microsoft's istreambuf_iterator, though. A workaround for that problem would be nice, since using istreambuf_iterator is far preferable over using istream_iterator, since the latter is subject to the skipws flag and thus easily leads to hard-to-find bugs. (By the way, the documentation should prominently mention the danger of skipws.)

[Hartmut Kaiser]

(In [60496]) Spirit: fixed #3999:

[anonymous]

The commit mention above fixes the return value issue. Thanks for pointing this out!

The problem with Microsofts istreambuf_iterator has already been discussed on Spirit's mailing list. So I recently added a new input policy: buffering_input_iterator which is buffering the last character it got from the underlying iterator, which allows to return a refernce from multi_pass' operator*() in this case as well. This new policy is now the default input policy when using make_multi_pass(). It's a bit less efficient than the input_iterator policy, but helps to overcome the problem you described.

Regards Hartmut

[Dave Abrahams]

I'm not sure this bug should be considered fixed. multi_pass claims to make any valid underlying input iterator into a forward iterator, does it not? Shouldn't it select the right policy automatically? I have the same problem with clang's istreambuf_iterator in C++11 mode.

[Dave Abrahams]

In fact, under Clang with C++11, this program fails to compile:

#include <utility>
#include <boost/spirit/home/support/multi_pass.hpp>
#include <iterator>
#include <boost/mpl/assert.hpp>
#include <boost/type_traits/is_reference.hpp>
#include <boost/type_traits/is_same.hpp>

typedef boost::spirit::multi_pass<
    std::istreambuf_iterator<char>, 
    boost::spirit::iterator_policies::default_policy<
        boost::spirit::iterator_policies::first_owner
      , boost::spirit::iterator_policies::ref_counted
      , boost::spirit::iterator_policies::buffering_input_iterator 
    >
> iterator;

BOOST_MPL_ASSERT((
    boost::is_same<
        std::iterator_traits<iterator>::iterator_category
      , std::forward_iterator_tag>
));

BOOST_MPL_ASSERT((
  boost::is_reference<std::iterator_traits<iterator>::reference>
));

int main() {}

[Dave Abrahams]

…and I think this simple patch fixes it. Hopefully someone can check my reasoning

buffering_input_iterator_policy.hpp

@@ -49 +49 @@
                 typename boost::detail::iterator_traits<T>::pointer
             pointer;
             typedef
-                typename boost::detail::iterator_traits<T>::reference
+                result_type&
             reference;
             typedef result_type value_type;
 

[Hartmut Kaiser]

Replying to dave:

…and I think this simple patch fixes it. Hopefully someone can check my reasoning

buffering_input_iterator_policy.hpp

@@ -49 +49 @@
                 typename boost::detail::iterator_traits<T>::pointer
             pointer;
             typedef
-                typename boost::detail::iterator_traits<T>::reference
+                result_type&
             reference;
             typedef result_type value_type;
 

Does this mean that in your case result_type& is something different from typename boost::detail::iterator_traits<T>::reference? That does not look right to me.

[Dave Abrahams]

See the C++11 specification for istreambuf_iterator<char>. Note that result_type is just an implementation detail for this particular policy.

[Dave Abrahams]

I know everyone's busy, but it disturbs me that my my claim here is given so little credence, and when I offer a standard reference to prove my point, it isn't followed up on. I am, after all, an authority on the iterator requirements and an author of the Boost Iterator Library.

[Hartmut Kaiser]

Replying to dave:

I know everyone's busy, but it disturbs me that my my claim here is given so little credence, and when I offer a standard reference to prove my point, it isn't followed up on. I am, after all, an authority on the iterator requirements and an author of the Boost Iterator Library.

Even if you're the authority, you'll have to cut me some slack while I'm travelling and on holidays. There are more important things than iterators after all.

[Dave Abrahams]

Fair enough; enjoy your break, Hartmut. BTW your BoostCon talk is inspiring :-)

[Hartmut Kaiser]

(In [85178]) Spirit: fixed #3999: multi_pass cannot deal with all input iterators

[Dave Abrahams]

Thanks!