ericniebler / range-v3

Range library for C++14/17/20, basis for C++20's std::ranges
Other
4.11k stars 442 forks source link

Make range-v3 views work with Boost #486

Closed tonyelewis closed 7 years ago

tonyelewis commented 7 years ago

I think it would be helpful if range-v3 played nicely with Boost. If I try this:

#include <range/v3/all.hpp>

#include <boost/algorithm/string/join.hpp>

#include <iostream>

int main() {
    const std::vector<std::string> a = { "underground",
                                         "overground" };

    const std::vector<std::string> b = { "wombling",
                                         "free" };

    std::cerr << boost::algorithm::join( ranges::view::concat( a, b ), " " ) << "\n";
}

I get the following compile errors, which AFAICT are due to the relevant range-v3 types not having the typedefs (eg iterator, const_iterator, value_type) that Boost expect ranges to have.

In file included from range-v3_boost_interop.cpp:3:
In file included from /opt/include/boost/algorithm/string/join.hpp:16:
/opt/include/boost/range/value_type.hpp:26:70: error: no type named 'type' in 'boost::range_iterator<ranges::v3::concat_view<ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> >, ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> > >, void>'
    struct range_value : iterator_value< typename range_iterator<T>::type >
                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
/opt/include/boost/algorithm/string/join.hpp:45:25: note: in instantiation of template class 'boost::range_value<ranges::v3::concat_view<ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> >, ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> > > >' requested here
        inline typename range_value<SequenceSequenceT>::type 
                        ^
range-v3_boost_interop.cpp:14:15: note: while substituting deduced template arguments into function template 'join' [with SequenceSequenceT = ranges::v3::concat_view<ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> >, ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> > >, Range1T = char [2]]
        std::cerr << boost::algorithm::join( ranges::view::concat( a, b ), " " ) << "\n";
                     ^
range-v3_boost_interop.cpp:14:15: error: no matching function for call to 'join'
        std::cerr << boost::algorithm::join( ranges::view::concat( a, b ), " " ) << "\n";
                     ^~~~~~~~~~~~~~~~~~~~~~
/opt/include/boost/algorithm/string/join.hpp:46:9: note: candidate template ignored: substitution failure [with SequenceSequenceT = ranges::v3::concat_view<ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> >, ranges::v3::iterator_range<std::__1::__wrap_iter<const std::__1::basic_string<char> *>, std::__1::__wrap_iter<const std::__1::basic_string<char> *> > >, Range1T = char [2]]
        join(
        ^
2 errors generated.

Is it possible to add these typedefs in the types to make everything play nicely?

Thanks very much for all your hard work on this excellent library.


Compile commands:

g++     -std=c++11                -isystem range-v3/include -isystem /opt/include range-v3_boost_interop.cpp -o range-v3_boost_interop.gcc_bin   && ./range-v3_boost_interop.gcc_bin
clang++ -std=c++11 -stdlib=libc++ -isystem range-v3/include -isystem /opt/include range-v3_boost_interop.cpp -o range-v3_boost_interop.clang_bin && ./range-v3_boost_interop.clang_bin
ericniebler commented 7 years ago

I think it would be helpful if range-v3 played nicely with Boost.

Depends on what you mean with "played nicely". I would be pretty wary of mixing the Boost iterator adaptors with range-v3's, like you show here. Boost.Range makes some assumptions about iterators being valid after their source range has been destroyed, and it knows nothing about move semantics so adaptors compose by reference, which is a recipe for disaster. But there is other sensible code that should work by all accounts.

Is it possible to add these typedefs in the types to make everything play nicely?

I think that would be the wrong solution. Code that assumes the presence of these typedefs is a problem because it is not generic. We can steer folks in the right direction by disabusing them of the notion that iterator should have these typedefs (pointers don't), or that ranges should (arrays don't).

A better solution would be to non-intrusively hook boost::range_itertor, boost::iterator_value, and friends. I would be happy to accept a patch.

tonyelewis commented 7 years ago

That all sounds good to me. I'm not sure how far I'll get with implementing the solution you propose but I'll happily try. Can you easily think of any suitable tests you'd like to see supporting the changes?

tonyelewis commented 7 years ago

Some updates...

the above example works if I add this:

namespace boost {
    template<typename T, typename U> struct range_mutable_iterator;
    template<typename T, typename U> struct range_const_iterator;
    template<typename T            > struct range_value;

    template <typename... Ts>
    struct range_mutable_iterator< ranges::v3::concat_view< Ts... >, void> {
        using type = ranges::range_iterator_t<       ranges::v3::concat_view< Ts... > >;
    };
    template <typename... Ts>
    struct range_const_iterator  < ranges::v3::concat_view< Ts... >, void> {
        using type = ranges::range_iterator_t< const ranges::v3::concat_view< Ts... > >;
    };
    template <typename... Ts>
    struct range_value           < ranges::v3::concat_view< Ts... >      > {
        using type = ranges::range_value_t   <       ranges::v3::concat_view< Ts... > >;
    };

} // namespace 'boost'

Is that the sort of approach you're thinking of?

I've added the un-specialised templates too so that the specialisations will compile if the relevant Boost range header has not-yet-been or will-not-be included. Presumably, this means things will break if Boost Range ever changes their form. Is there a better way I should be handling this?

tonyelewis commented 7 years ago

Also, do you have preferences about how this should be organised? One option is a single, optional header, included by no other range-v3 header, along the lines of:

#ifndef RANGES_V3_BOOST_RANGE_COMPAT_HPP
#define RANGES_V3_BOOST_RANGE_COMPAT_HPP

#include <boost/range.hpp>

#include <range/v3/range_fwd.hpp>

namespace boost {

    // replace_view                  is covered by iter_transform_view
    // replace_if_view               is covered by iter_transform_view
    // set_difference_view           is covered by detail::set_algorithm_view
    // set_intersection_view         is covered by detail::set_algorithm_view
    // set_symmetric_difference_view is covered by detail::set_algorithm_view
    // set_union_view                is covered by detail::set_algorithm_view
    // unique_view                   is covered by adjacent_filter_view
    // values_view                   is covered by transform_view
    template <typename... Ts> struct range_mutable_iterator< ranges::v3::adjacent_filter_view          < Ts... >, void> { using type = ranges::range_iterator_t<       ranges::v3::adjacent_filter_view          < Ts... > >; };
    template <typename... Ts> struct range_const_iterator  < ranges::v3::adjacent_filter_view          < Ts... >, void> { using type = ranges::range_iterator_t< const ranges::v3::adjacent_filter_view          < Ts... > >; };
    template <typename... Ts> struct range_value           < ranges::v3::adjacent_filter_view          < Ts... >      > { using type = ranges::range_value_t   <       ranges::v3::adjacent_filter_view          < Ts... > >; };
    template <typename... Ts> struct range_mutable_iterator< ranges::v3::adjacent_remove_if_view       < Ts... >, void> { using type = ranges::range_iterator_t<       ranges::v3::adjacent_remove_if_view       < Ts... > >; };
    template <typename... Ts> struct range_const_iterator  < ranges::v3::adjacent_remove_if_view       < Ts... >, void> { using type = ranges::range_iterator_t< const ranges::v3::adjacent_remove_if_view       < Ts... > >; };
    template <typename... Ts> struct range_value           < ranges::v3::adjacent_remove_if_view       < Ts... >      > { using type = ranges::range_value_t   <       ranges::v3::adjacent_remove_if_view       < Ts... > >; };
    template <typename    T > struct range_mutable_iterator< ranges::v3::any_bidirectional_view        < T     >, void> { using type = ranges::range_iterator_t<       ranges::v3::any_bidirectional_view        < T     > >; };
    template <typename    T > struct range_const_iterator  < ranges::v3::any_bidirectional_view        < T     >, void> { using type = ranges::range_iterator_t< const ranges::v3::any_bidirectional_view        < T     > >; };
    template <typename    T > struct range_value           < ranges::v3::any_bidirectional_view        < T     >      > { using type = ranges::range_value_t   <       ranges::v3::any_bidirectional_view        < T     > >; };
    template <typename    T > struct range_mutable_iterator< ranges::v3::any_forward_view              < T     >, void> { using type = ranges::range_iterator_t<       ranges::v3::any_forward_view              < T     > >; };
    template <typename    T > struct range_const_iterator  < ranges::v3::any_forward_view              < T     >, void> { using type = ranges::range_iterator_t< const ranges::v3::any_forward_view              < T     > >; };
    template <typename    T > struct range_value           < ranges::v3::any_forward_view              < T     >      > { using type = ranges::range_value_t   <       ranges::v3::any_forward_view              < T     > >; };
    template <typename    T > struct range_mutable_iterator< ranges::v3::any_input_view                < T     >, void> { using type = ranges::range_iterator_t<       ranges::v3::any_input_view                < T     > >; };
    template <typename    T > struct range_const_iterator  < ranges::v3::any_input_view                < T     >, void> { using type = ranges::range_iterator_t< const ranges::v3::any_input_view                < T     > >; };
    template <typename    T > struct range_value           < ranges::v3::any_input_view                < T     >      > { using type = ranges::range_value_t   <       ranges::v3::any_input_view                < T     > >; };
    template <typename    T > struct range_mutable_iterator< ranges::v3::any_random_access_view        < T     >, void> { using type = ranges::range_iterator_t<       ranges::v3::any_random_access_view        < T     > >; };
[...]

[...]
    template <typename... Ts> struct range_value           < ranges::v3::transform_view                < Ts... >      > { using type = ranges::range_value_t   <       ranges::v3::transform_view                < Ts... > >; };
    template <typename... Ts> struct range_mutable_iterator< ranges::v3::unbounded_view                < Ts... >, void> { using type = ranges::range_iterator_t<       ranges::v3::unbounded_view                < Ts... > >; };
    template <typename... Ts> struct range_const_iterator  < ranges::v3::unbounded_view                < Ts... >, void> { using type = ranges::range_iterator_t< const ranges::v3::unbounded_view                < Ts... > >; };
    template <typename... Ts> struct range_value           < ranges::v3::unbounded_view                < Ts... >      > { using type = ranges::range_value_t   <       ranges::v3::unbounded_view                < Ts... > >; };
    template <typename... Ts> struct range_mutable_iterator< ranges::v3::zip_view                      < Ts... >, void> { using type = ranges::range_iterator_t<       ranges::v3::zip_view                      < Ts... > >; };
    template <typename... Ts> struct range_const_iterator  < ranges::v3::zip_view                      < Ts... >, void> { using type = ranges::range_iterator_t< const ranges::v3::zip_view                      < Ts... > >; };
    template <typename... Ts> struct range_value           < ranges::v3::zip_view                      < Ts... >      > { using type = ranges::range_value_t   <       ranges::v3::zip_view                      < Ts... > >; };
    template <typename... Ts> struct range_mutable_iterator< ranges::v3::zip_with_view                 < Ts... >, void> { using type = ranges::range_iterator_t<       ranges::v3::zip_with_view                 < Ts... > >; };
    template <typename... Ts> struct range_const_iterator  < ranges::v3::zip_with_view                 < Ts... >, void> { using type = ranges::range_iterator_t< const ranges::v3::zip_with_view                 < Ts... > >; };
    template <typename... Ts> struct range_value           < ranges::v3::zip_with_view                 < Ts... >      > { using type = ranges::range_value_t   <       ranges::v3::zip_with_view                 < Ts... > >; };

} // namespace 'boost'

#endif

This has the advantage of explicitly including boost/range.hpp without affecting anyone that's uninterested in Boost Range compatability.

If you approve of this approach, do you prefer this large, one-line-per-specialisation table format that's ugly but probably simpler to maintain?

ericniebler commented 7 years ago

Is that the sort of approach you're thinking of?

Yes, but from http://www.boost.org/doc/libs/1_62_0/libs/range/doc/html/range/reference/concept_implementation/semantics/metafunctions.html it looks like boost::range_iterator is the customization point you're looking for.

I've added the un-specialised templates too so that the specialisations will compile if the relevant Boost range header has not-yet-been or will-not-be included. Presumably, this means things will break if Boost Range ever changes their form. Is there a better way I should be handling this?

This is exactly why these templates exist, so Boost is unlikely to change them.

Also, do you have preferences about how this should be organised?

My preference is that each view header would hook these metafunctions for the view(s) defined in that header. You could even (gasp) write a macro to make this easier.

Thanks for taking this on.

tonyelewis commented 7 years ago

Thanks for your message and apologies for my slow response.

Yes, but from http://www.boost.org/doc/libs/1_62_0/libs/range/doc/html/range/reference/concept_implementation/semantics/metafunctions.html it looks like boost::range_iterator is the customization point you're looking for.

I ended up with boost::range_mutable_iterator and boost::range_const_iterator because:

This is exactly why these templates exist, so Boost is unlikely to change them.

My preference is that each view header would hook these metafunctions for the view(s) defined in that header. You could even (gasp) write a macro to make this easier.

Roger.

Thanks for taking this on.

:)

I'll try to get a pull-request together with what I've got. Please don't hesitate to reject if you: remain concerned about the specialisation point; can think of tests you'd like to see; see anything else you dislike.

Thanks very much.

ericniebler commented 7 years ago

Fixed in b73b4ba