From 3ecf600434844e246cc5c6bac6f66690a873795d Mon Sep 17 00:00:00 2001 From: Neil Groves Date: Mon, 16 Jun 2014 14:49:25 +0100 Subject: [PATCH] Fixed range adaptors that were creating underlying iterators that were not default constructible. --- include/boost/range/adaptor/filtered.hpp | 26 ++-- include/boost/range/adaptor/replaced.hpp | 26 +++- include/boost/range/adaptor/replaced_if.hpp | 24 +++- include/boost/range/adaptor/transformed.hpp | 46 ++++--- .../detail/default_constructible_unary_fn.hpp | 64 ++++++++++ test/Jamfile.v2 | 1 + test/adaptor_test/chained.cpp | 117 ++++++++++++++++++ 7 files changed, 272 insertions(+), 32 deletions(-) create mode 100644 include/boost/range/detail/default_constructible_unary_fn.hpp create mode 100644 test/adaptor_test/chained.cpp diff --git a/include/boost/range/adaptor/filtered.hpp b/include/boost/range/adaptor/filtered.hpp index 9e050a6..b6d3ab1 100644 --- a/include/boost/range/adaptor/filtered.hpp +++ b/include/boost/range/adaptor/filtered.hpp @@ -12,6 +12,7 @@ #define BOOST_RANGE_ADAPTOR_FILTERED_HPP #include +#include #include #include #include @@ -23,21 +24,28 @@ namespace boost template< class P, class R > struct filtered_range : boost::iterator_range< - boost::filter_iterator< P, - BOOST_DEDUCED_TYPENAME range_iterator::type + boost::filter_iterator< + typename default_constructible_unary_fn_gen::type, + typename range_iterator::type > > { private: typedef boost::iterator_range< - boost::filter_iterator< P, - BOOST_DEDUCED_TYPENAME range_iterator::type - > - > base; + boost::filter_iterator< + typename default_constructible_unary_fn_gen::type, + typename range_iterator::type + > + > base; public: - filtered_range( P p, R& r ) - : base( make_filter_iterator( p, boost::begin(r), boost::end(r) ), - make_filter_iterator( p, boost::end(r), boost::end(r) ) ) + typedef typename default_constructible_unary_fn_gen::type + pred_t; + + filtered_range(P p, R& r) + : base(make_filter_iterator(pred_t(p), + boost::begin(r), boost::end(r)), + make_filter_iterator(pred_t(p), + boost::end(r), boost::end(r))) { } }; diff --git a/include/boost/range/adaptor/replaced.hpp b/include/boost/range/adaptor/replaced.hpp index 2a6ab56..1950b82 100644 --- a/include/boost/range/adaptor/replaced.hpp +++ b/include/boost/range/adaptor/replaced.hpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace boost { @@ -32,19 +33,36 @@ namespace boost typedef const Value& result_type; typedef const Value& first_argument_type; + // Rationale: + // The default constructor is required to allow the transform + // iterator to properly model the iterator concept. + replace_value() + { + } + replace_value(const Value& from, const Value& to) - : m_from(from), m_to(to) + : m_impl(data(from, to)) { } const Value& operator()(const Value& x) const { - return (x == m_from) ? m_to : x; + return (x == m_impl->m_from) ? m_impl->m_to : x; } private: - Value m_from; - Value m_to; + struct data + { + data(const Value& from, const Value& to) + : m_from(from) + , m_to(to) + { + } + + Value m_from; + Value m_to; + }; + boost::optional m_impl; }; template< class R > diff --git a/include/boost/range/adaptor/replaced_if.hpp b/include/boost/range/adaptor/replaced_if.hpp index b9f73ae..e425e7d 100644 --- a/include/boost/range/adaptor/replaced_if.hpp +++ b/include/boost/range/adaptor/replaced_if.hpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace boost { @@ -32,19 +33,34 @@ namespace boost typedef const Value& result_type; typedef const Value& first_argument_type; + // Rationale: + // required to allow the iterator to be default constructible. + replace_value_if() + { + } + replace_value_if(const Pred& pred, const Value& to) - : m_pred(pred), m_to(to) + : m_impl(data(pred, to)) { } const Value& operator()(const Value& x) const { - return m_pred(x) ? m_to : x; + return m_impl->m_pred(x) ? m_impl->m_to : x; } private: - Pred m_pred; - Value m_to; + struct data + { + data(const Pred& p, const Value& t) + : m_pred(p), m_to(t) + { + } + + Pred m_pred; + Value m_to; + }; + boost::optional m_impl; }; template< class Pred, class R > diff --git a/include/boost/range/adaptor/transformed.hpp b/include/boost/range/adaptor/transformed.hpp index 9edd27c..428ff4b 100644 --- a/include/boost/range/adaptor/transformed.hpp +++ b/include/boost/range/adaptor/transformed.hpp @@ -12,6 +12,7 @@ #define BOOST_RANGE_ADAPTOR_TRANSFORMED_HPP #include +#include #include #include #include @@ -21,31 +22,46 @@ namespace boost { namespace range_detail { + // A type generator to produce the transform_iterator type conditionally + // including a wrapped predicate as appropriate. + template + struct transform_iterator_gen + { + typedef transform_iterator< + typename default_constructible_unary_fn_gen< + P, + typename transform_iterator::reference + >::type, + It + > type; + }; template< class F, class R > struct transformed_range : public boost::iterator_range< - boost::transform_iterator< F, - BOOST_DEDUCED_TYPENAME range_iterator::type - > - > + typename transform_iterator_gen< + F, typename range_iterator::type>::type> { private: - typedef boost::iterator_range< - boost::transform_iterator< F, - BOOST_DEDUCED_TYPENAME range_iterator::type - > - > - base; + typedef typename transform_iterator_gen< + F, typename range_iterator::type>::type transform_iter_t; + + typedef boost::iterator_range base; public: - typedef F transform_fn_type; + typedef typename default_constructible_unary_fn_gen< + F, + typename transform_iterator< + F, + typename range_iterator::type + >::reference + >::type transform_fn_type; + typedef R source_range_type; - transformed_range( F f, R& r ) - : base( boost::make_transform_iterator( boost::begin(r), f ), - boost::make_transform_iterator( boost::end(r), f ) ) - + transformed_range(transform_fn_type f, R& r) + : base(transform_iter_t(boost::begin(r), f), + transform_iter_t(boost::end(r), f)) { } }; diff --git a/include/boost/range/detail/default_constructible_unary_fn.hpp b/include/boost/range/detail/default_constructible_unary_fn.hpp new file mode 100644 index 0000000..374ddda --- /dev/null +++ b/include/boost/range/detail/default_constructible_unary_fn.hpp @@ -0,0 +1,64 @@ +// Boost.Range library +// +// Copyright Neil Groves 2014. Use, modification and +// distribution is subject to the Boost Software License, Version +// 1.0. (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) +// +// For more information, see http://www.boost.org/libs/range/ +// +#ifndef BOOST_RANGE_DETAIL_DEFAULT_CONSTRUCTIBLE_UNARY_FN_HPP_INCLUDED +#define BOOST_RANGE_DETAIL_DEFAULT_CONSTRUCTIBLE_UNARY_FN_HPP_INCLUDED + +#include +#include +#include + +namespace boost +{ + namespace range_detail + { + +template +class default_constructible_unary_fn_wrapper +{ +public: + typedef R result_type; + + default_constructible_unary_fn_wrapper() + { + } + default_constructible_unary_fn_wrapper(const F& source) + : m_impl(source) + { + } + template + R operator()(const Arg& arg) const + { + BOOST_ASSERT(m_impl); + return (*m_impl)(arg); + } + template + R operator()(Arg& arg) const + { + BOOST_ASSERT(m_impl); + return (*m_impl)(arg); + } +private: + boost::optional m_impl; +}; + +template +struct default_constructible_unary_fn_gen +{ + typedef typename boost::mpl::if_< + boost::has_trivial_default_constructor, + F, + default_constructible_unary_fn_wrapper + >::type type; +}; + + } // namespace range_detail +} // namespace boost + +#endif // include guard diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 5f84421..2c6a2a0 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -57,6 +57,7 @@ test-suite range : [ compile-fail compile_fail/adaptor/uniqued_concept3.cpp ] [ compile-fail compile_fail/adaptor/uniqued_concept4.cpp ] [ range-test adaptor_test/adjacent_filtered ] + [ range-test adaptor_test/chained ] [ range-test adaptor_test/copied ] [ range-test adaptor_test/filtered ] [ range-test adaptor_test/indexed ] diff --git a/test/adaptor_test/chained.cpp b/test/adaptor_test/chained.cpp new file mode 100644 index 0000000..1b756c9 --- /dev/null +++ b/test/adaptor_test/chained.cpp @@ -0,0 +1,117 @@ +// Boost.Range library +// +// Copyright Neil Groves 2014. Use, modification and +// distribution is subject to the Boost Software License, Version +// 1.0. (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) +// +// +// For more information, see http://www.boost.org/libs/range/ +// +// Credits: +// Jurgen Hunold provided a test case that demonstrated that the range adaptors +// were producing iterators that were not default constructible. This became +// symptomatic after enabling concept checking assertions. This test is a +// lightly modified version of his supplied code to ensure that his use case +// never breaks again. (hopefully!) +// + +#include +#include +#include +#include + +#include +#include + +#include +#include + +namespace boost_range_test +{ + namespace + { + +class foo +{ +public: + static foo from_string(const std::string& source) + { + foo f; + f.m_valid = true; + f.m_value = 0u; + for (std::string::const_iterator it = source.begin(); + it != source.end(); ++it) + { + f.m_value += *it; + if ((*it < 'a') || (*it > 'z')) + f.m_valid = false; + } + return f; + } + bool is_valid() const + { + return m_valid; + } + bool operator<(const foo& other) const + { + return m_value < other.m_value; + } + bool operator==(const foo& other) const + { + return m_value == other.m_value && m_valid == other.m_valid; + } + bool operator!=(const foo& other) const + { + return !operator==(other); + } + + friend inline std::ostream& operator<<(std::ostream& out, const foo& obj) + { + out << "{value=" << obj.m_value + << ", valid=" << std::boolalpha << obj.m_valid << "}\n"; + return out; + } + +private: + boost::uint64_t m_value; + bool m_valid; +}; + +void chained_adaptors_test() +{ + std::vector sep; + + sep.push_back("AB"); + sep.push_back("ab"); + sep.push_back("aghj"); + + std::set foos; + + boost::copy(sep + | boost::adaptors::transformed(boost::bind(&foo::from_string, _1)) + | boost::adaptors::filtered(boost::bind(&foo::is_valid, _1)), + std::inserter(foos, foos.end())); + + std::vector reference; + reference.push_back(foo::from_string("ab")); + reference.push_back(foo::from_string("aghj")); + + BOOST_CHECK_EQUAL_COLLECTIONS( + reference.begin(), reference.end(), + foos.begin(), foos.end()); +} + + } // anonymous namespace +} // namespace boost_range_test + +boost::unit_test::test_suite* +init_unit_test_suite(int argc, char* argv[]) +{ + boost::unit_test::test_suite* test + = BOOST_TEST_SUITE( "RangeTestSuite.adaptor.chained adaptors" ); + + test->add(BOOST_TEST_CASE( boost_range_test::chained_adaptors_test)); + + return test; +}