[spirit][classic] returning reference to temporary when using position_iterator with istreambuf_iterator<char>

[Julien Charbon <jcharbon@…>]

In boost spirit classic, when using position_iterator with istreambuf_iterator<char>, the underlying iterator_adaptor::deference() might return a reference to temporary:

For example with clang34/libc++:

$ clang++34 -O2 -Wall test-boost.cpp -o test-boost
source/boost/boost/iterator/iterator_adaptor.hpp:310:18: warning: returning reference to local temporary object [-Wreturn-stack-address]
        { return *m_iterator; }
                 ^~~~~~~~~~~

or with gcc47/libstdc++ in c++11 mode:

$ g++47 --std=c++11 -O2 -Wall test-boost.cpp -o test-boost
source/boost/boost/iterator/iterator_adaptor.hpp:310:19: warning: returning reference to temporary [enabled by default]

but not in default gcc47 default code (gnu++98):

$ g++47 -O2 -Wall test-boost.cpp -o test-boost

We tracked down this issue to istreambuf_iterator<char>::reference and istreambuf_iterator<char>::operator*() definition:

• On clang3.4/libc++:

istreambuf_iterator<char>::reference type is 'char' and istreambuf_iterator<char>::operator*() also returns a 'char'.

• On gcc47/libstdc++ in c++11 mode:

istreambuf_iterator<char>::reference type is 'char' and istreambuf_iterator<char>::operator*() also returns a 'char'.

See: istreambuf_iterator::reference changes from _CharT& to _CharT: ​http://gcc.gnu.org/wiki/Cxx11AbiCompatibility

• However on gcc47/libstdc++ in default mode:

istreambuf_iterator<char>::reference type is 'char &' where istreambuf_iterator<char>::operator*() returns a 'char'.

And position_iterator being built from iterator_adaptor with:

    typedef boost::iterator_adaptor<
        main_iter_t,
        ForwardIterT,
        const_value_type,
        boost::forward_traversal_tag
    > type;

there is no 5th template type which defines iterator_adaptor 'Reference':

  // Iterator Adaptor
  // ...
  // 
  //   Reference - the reference type of the resulting iterator, and in
  //      particular, the result type of operator*(). If not supplied but
  //      Value is supplied, Value& is used. Otherwise
  //      iterator_traits<Base>::reference is used.

Thus by default, iterator_adaptor::deference() from a position_iterator built of istreambuf_iterator<char> returns a &char, which is fine only when istreambuf_iterator<char>::reference type is also 'char &'.

[Julien Charbon <jcharbon@…>]

Simple test case example

[Julien Charbon <jcharbon@…>]

Side-effects of this returning reference to temporary are enlighten with attached test case:

• clang34/libc++ (fail):

$ clang++34 -O2 -Wall test-boost.cpp -o test-boost
source/boost/boost/iterator/iterator_adaptor.hpp:310:18: warning: returning reference to local temporary object [-Wreturn-stack-address]
        { return *m_iterator; }
                 ^~~~~~~~~~~
...
$ ./test-boost 

File:   internal string
Line:   1
Col:    1
Expecting a number, but found something else

Parsing failed

• gcc47/libstdc++ in c++11 mode (fail):

$ g++47 -O2 -Wall test-boost.cpp -o test-boost
source/boost/boost/iterator/iterator_adaptor.hpp:310:19: warning: returning reference to temporary [enabled by default]
...
$ ./test-boost 

File:   internal string
Line:   1
Col:    1
Expecting a number, but found something else

Parsing failed

• gcc47/libstdc++ default mode (success):

$ g++47 -O2 -Wall test-boost.cpp -o test-boost
$ ./test-boost 
Parsing:
0, 1, 2
Parses OK: 
0: 0
1: 1
2: 2

For the record the complete warnings clang34/libc++:

$ clang++34 -O2 -Wall test-boost.cpp -o test-boost
In file included from test-boost.cpp:5:
In file included from source/boost/boost/spirit/include/classic_position_iterator.hpp:11:
In file included from source/boost/boost/spirit/home/classic/iterator/position_iterator.hpp:98:
In file included from source/boost/boost/spirit/home/classic/iterator/impl/position_iterator.ipp:15:
In file included from source/boost/boost/iterator_adaptors.hpp:11:
source/boost/boost/iterator/iterator_adaptor.hpp:310:18: warning: returning reference to local temporary object [-Wreturn-stack-address]
        { return *m_iterator; }
                 ^~~~~~~~~~~
source/boost/boost/iterator/iterator_facade.hpp:514:20: note: in instantiation of member function 'boost::iterator_adaptor<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, const char, boost::forward_traversal_tag, boost::use_default, boost::use_default>::dereference' requested here
          return f.dereference();
                   ^
source/boost/boost/iterator/iterator_facade.hpp:639:40: note: in instantiation of function template specialization 'boost::iterator_core_access::dereference<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t> >' requested here
          return iterator_core_access::dereference(this->derived());
                                       ^
source/boost/boost/spirit/home/classic/core/scanner/scanner.hpp:54:20: note: in instantiation of member function 'boost::iterator_facade<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, const char, boost::forward_traversal_tag, const char &, long long>::operator*' requested here
            return *scan.first;
                   ^
source/boost/boost/spirit/home/classic/core/scanner/skipper.hpp:61:66: note: in instantiation of function template specialization 'boost::spirit::classic::iteration_policy::get<boost::spirit::classic::scanner<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>, boost::spirit::classic::match_policy, boost::spirit::classic::action_policy> > >' requested here
            while (!BaseT::at_end(scan) && impl::isspace_(BaseT::get(scan)))
                                                                 ^
source/boost/boost/spirit/home/classic/core/scanner/skipper.hpp:53:18: note: in instantiation of function template specialization 'boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>::skip<boost::spirit::classic::scanner<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>, boost::spirit::classic::match_policy, boost::spirit::classic::action_policy> > >' requested here
            scan.skip(scan);
                 ^
source/boost/boost/spirit/home/classic/core/scanner/scanner.hpp:252:43: note: in instantiation of function template specialization 'boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>::at_end<boost::spirit::classic::scanner<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>, boost::spirit::classic::match_policy, boost::spirit::classic::action_policy> > >' requested here
            return iteration_policy_type::at_end(*this);
                                          ^
source/boost/boost/spirit/home/classic/core/scanner/scanner.hpp:235:13: note: in instantiation of member function 'boost::spirit::classic::scanner<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>, boost::spirit::classic::match_policy, boost::spirit::classic::action_policy> >::at_end' requested here
            at_end();
            ^
source/boost/boost/spirit/home/classic/core/scanner/impl/skipper.ipp:132:27: note: in instantiation of member function 'boost::spirit::classic::scanner<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<boost::spirit::classic::iteration_policy>, boost::spirit::classic::match_policy, boost::spirit::classic::action_policy> >::scanner' requested here
                scanner_t scan(first, last);
                          ^
source/boost/boost/spirit/home/classic/core/scanner/impl/skipper.ipp:155:13: note: in instantiation of function template specialization 'boost::spirit::classic::impl::phrase_parser<boost::spirit::classic::space_parser>::parse<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::sequence<boost::spirit::classic::alternative<boost::spirit::classic::action<boost::spirit::classic::real_parser<double, boost::spirit::classic::real_parser_policies<double> >, boost::spirit::classic::ref_value_actor<std::__1::vector<double, std::__1::allocator<double> >, boost::spirit::classic::push_back_action> >, boost::spirit::classic::functor_parser<error_report_parser> >, boost::spirit::classic::kleene_star<boost::spirit::classic::sequence<boost::spirit::classic::alternative<boost::spirit::classic::chlit<char>, boost::spirit::classic::functor_parser<error_report_parser> >, boost::spirit::classic::alternative<boost::spirit::classic::action<boost::spirit::classic::real_parser<double, boost::spirit::classic::real_parser_policies<double> >, boost::spirit::classic::ref_value_actor<std::__1::vector<double, std::__1::allocator<double> >, boost::spirit::classic::push_back_action> >, boost::spirit::classic::functor_parser<error_report_parser> > > > > >' requested here
            parse(first, last, p.derived(), skip.derived());
            ^
test-boost.cpp:92:12: note: in instantiation of function template specialization 'boost::spirit::classic::parse<boost::spirit::classic::position_iterator<boost::spirit::classic::multi_pass<std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, boost::spirit::classic::multi_pass_policies::input_iterator, boost::spirit::classic::multi_pass_policies::ref_counted, boost::spirit::classic::multi_pass_policies::buf_id_check, boost::spirit::classic::multi_pass_policies::std_deque>, boost::spirit::classic::file_position_base<std::__1::basic_string<char> >, boost::spirit::classic::nil_t>, boost::spirit::classic::sequence<boost::spirit::classic::alternative<boost::spirit::classic::action<boost::spirit::classic::real_parser<double, boost::spirit::classic::real_parser_policies<double> >, boost::spirit::classic::ref_value_actor<std::__1::vector<double, std::__1::allocator<double> >, boost::spirit::classic::push_back_action> >, boost::spirit::classic::functor_parser<error_report_parser> >, boost::spirit::classic::kleene_star<boost::spirit::classic::sequence<boost::spirit::classic::alternative<boost::spirit::classic::chlit<char>, boost::spirit::classic::functor_parser<error_report_parser> >, boost::spirit::classic::alternative<boost::spirit::classic::action<boost::spirit::classic::real_parser<double, boost::spirit::classic::real_parser_policies<double> >, boost::spirit::classic::ref_value_actor<std::__1::vector<double, std::__1::allocator<double> >, boost::spirit::classic::push_back_action> >, boost::spirit::classic::functor_parser<error_report_parser> > > > >, boost::spirit::classic::space_parser>' requested here
    return parse(pos_begin, pos_end,
           ^
1 warning generated.

[Julien Charbon <jcharbon@…>]

Patch that fix this specific issue (open to discussion)

[Julien Charbon <jcharbon@…>]

The joined position_iterator.patch fixes this specific issue, however it is difficult to promote this patch as _the_ solution as position_iterator usages are quite diverse. It can be used to start discussion on this issue.

[Joel de Guzman]

fixed in ​https://github.com/boostorg/spirit/pull/422