4

I wish to treat boost::optional as a container that can have zero or one elements in it. Logically I should be able to create an iterator to the container and use boost::for_each on it as well. My attempt is below but it fails to compile. I have tried it on GodBolt.

https://godbolt.org/g/z5MFfe

Code below for reference.

#include <boost/optional.hpp>
#include <boost/range.hpp>
#include <boost/range/algorithm/for_each.hpp>
#include <iostream>

namespace boost {

    template <typename OT>
        class optional_iterator
        : public boost::iterator_facade<optional_iterator<OT>,
        typename OT::value_type,
        boost::forward_traversal_tag> {
            private:
                OT m_o;

            public:
                optional_iterator() : m_o(boost::none) {}
                explicit optional_iterator(OT& o) : m_o(o) {}

            private:
                friend class boost::iterator_core_access;

                void increment() { m_o = boost::none; }

                bool equal(OT& other) const { return this->m_o == other.m_o; }

                typename OT::value_type& dereference() { return *m_o; }
        };

    template <typename T>
        struct optional_iterators {
            typedef optional_iterator<boost::optional<T>> iterator;
            typedef optional_iterator<boost::optional<T> const> const_iterator;
        };

    template <class T>
        struct range_mutable_iterator<boost::optional<T>> {
            typedef typename optional_iterators<T>::iterator type;
        };

    template <class T>
        struct range_const_iterator<boost::optional<T> const> {
            typedef typename optional_iterators<T const>::const_iterator type;
        };

    template <class T>
        inline typename optional_iterators<T>::iterator range_begin(
                boost::optional<T>& x) {
            return typename optional_iterators<T>::iterator(x);
        }

    template <class T>
        inline typename optional_iterators<T>::const_iterator range_begin(
                boost::optional<T> const& x) {
            return typename optional_iterators<T>::const_iterator(x);
        }

    template <class T>
        inline typename optional_iterators<T>::iterator range_end(boost::optional<T>&x) {
            return typename optional_iterators<T>::iterator();
        }

    template <class T>
        inline typename optional_iterators<T>::const_iterator range_end(boost::optional<T> const & x) {
            return typename optional_iterators<T>::const_iterator();
        }
}  // namespace boost

int main() {
    auto a = boost::optional<int>(10);
    auto rb = range_begin(a);
    auto re = range_end(a);

   boost::for_each(a, [](int const& i) { std::cout << i; });
}

I'm fairly sure I've implemented the range and iterator concepts correctly but I get the following compiler errors with gcc 8.1 ( as per godbolt )

#1 with x86-64 gcc 8.1
In file included from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/size_type.hpp:20,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/size.hpp:21,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/functions.hpp:20,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range.hpp:18,

                 from <source>:3:

/opt/compiler-explorer/libs/boost_1_64_0/boost/range/concepts.hpp: In instantiation of 'struct boost::SinglePassRangeConcept<boost::optional<int> >':

/opt/compiler-explorer/libs/boost_1_64_0/boost/concept/detail/has_constraints.hpp:32:62:   required by substitution of 'template<class Model> boost::concepts::detail::yes boost::concepts::detail::has_constraints_(Model*, boost::concepts::detail::wrap_constraints<Model, (& Model::constraints)>*) [with Model = boost::SinglePassRangeConcept<boost::optional<int> >]'

/opt/compiler-explorer/libs/boost_1_64_0/boost/concept/detail/has_constraints.hpp:42:5:   required from 'const bool boost::concepts::not_satisfied<boost::SinglePassRangeConcept<boost::optional<int> > >::value'

/opt/compiler-explorer/libs/boost_1_64_0/boost/concept/detail/has_constraints.hpp:45:31:   required from 'struct boost::concepts::not_satisfied<boost::SinglePassRangeConcept<boost::optional<int> > >'

/opt/compiler-explorer/libs/boost_1_64_0/boost/mpl/if.hpp:63:11:   required from 'struct boost::mpl::if_<boost::concepts::not_satisfied<boost::SinglePassRangeConcept<boost::optional<int> > >, boost::concepts::constraint<boost::SinglePassRangeConcept<boost::optional<int> > >, boost::concepts::requirement<boost::concepts::failed************ boost::SinglePassRangeConcept<boost::optional<int> >::************> >'

/opt/compiler-explorer/libs/boost_1_64_0/boost/concept/detail/general.hpp:51:8:   required from 'struct boost::concepts::requirement_<void (*)(boost::SinglePassRangeConcept<boost::optional<int> >)>'

/opt/compiler-explorer/libs/boost_1_64_0/boost/range/size.hpp:62:9:   required from 'UnaryFunction boost::range::for_each(SinglePassRange&, UnaryFunction) [with SinglePassRange = boost::optional<int>; UnaryFunction = main()::<lambda(const int&)>]'

<source>:75:59:   required from here

/opt/compiler-explorer/libs/boost_1_64_0/boost/range/concepts.hpp:272:17: error: no type named 'type' in 'struct boost::range_iterator<const boost::optional<int>, void>'

         >::type const_iterator;

                 ^~~~~~~~~~~~~~

In file included from /opt/compiler-explorer/libs/boost_1_64_0/boost/concept/assert.hpp:35,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/concept_check.hpp:20,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/concepts.hpp:19,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/size_type.hpp:20,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/size.hpp:21,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range/functions.hpp:20,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/range.hpp:18,

                 from <source>:3:

/opt/compiler-explorer/libs/boost_1_64_0/boost/range/concepts.hpp:279:9: error: no type named 'type' in 'struct boost::range_iterator<const boost::optional<int>, void>'

         BOOST_RANGE_CONCEPT_ASSERT((

         ^~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from /opt/compiler-explorer/gcc-8.1.0/include/c++/8.1.0/algorithm:62,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/core/swap.hpp:25,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/optional/optional.hpp:27,

                 from /opt/compiler-explorer/libs/boost_1_64_0/boost/optional.hpp:15,

                 from <source>:2:

/opt/compiler-explorer/gcc-8.1.0/include/c++/8.1.0/bits/stl_algo.h:3876:5: error: '_Funct std::for_each(_IIter, _IIter, _Funct) [with _IIter = boost::optional_iterator<boost::optional<int> >; _Funct = main()::<lambda(const int&)>]', declared using local type 'main()::<lambda(const int&)>', is used but never defined [-fpermissive]

     for_each(_InputIterator __first, _InputIterator __last, _Function __f)

     ^~~~~~~~

/opt/compiler-explorer/gcc-8.1.0/include/c++/8.1.0/bits/stl_algo.h:3876:5: warning: '_Funct std::for_each(_IIter, _IIter, _Funct) [with _IIter = boost::optional_iterator<boost::optional<int> >; _Funct = main()::<lambda(const int&)>]' used but never defined

Compiler returned: 1
bradgonesurfing
  • 30,949
  • 17
  • 114
  • 217
  • 1
    Defining an iterator is not enough to make an optional model a [forward sequence](https://www.boost.org/doc/libs/1_60_0/libs/fusion/doc/html/fusion/sequence/concepts/forward_sequence.html). Nothing in your code makes the valid expression list hold. Try wrapping the optional in a lightweight proxy that models the required concept. – StoryTeller - Unslander Monica Jun 26 '18 at 07:24
  • According to https://www.boost.org/doc/libs/1_67_0/libs/range/doc/html/range/reference/extending/method_2.html I have provided the free functions required. But the link you showed me seems to define a different way to do it. Or perhaps I need both? – bradgonesurfing Jun 26 '18 at 07:26
  • You overlooked the part about [argument-dependent lookup](https://en.cppreference.com/w/cpp/language/adl). It doesn't consider *all* free functions. Only ones that are added in a *specific* fashion. The namespace where you add it matters. – StoryTeller - Unslander Monica Jun 26 '18 at 07:30
  • I put the free functions in the boost namespace because ``boost::optional`` is in the boost namespace. Is this wrong? If so what namespace do you suggest? – bradgonesurfing Jun 26 '18 at 07:37
  • 1
    Apologies. I missed the reopended boost namespace. That isn't the problem. I looked closer at your linked article, and your error message. The "Related concept" column is the issue. The error message says as much in a very verbose manner: `boost::concepts::requirement – StoryTeller - Unslander Monica Jun 26 '18 at 07:45
  • Though ostensibly, you did everything the article asked. The problem may be with `boost` already doing something unknown for its internal types. – StoryTeller - Unslander Monica Jun 26 '18 at 07:47
  • I hadn't defined the range_end functions correctly. I fixed that but the compiler error doesn't change. I'll hack on it a bit more. :( – bradgonesurfing Jun 26 '18 at 07:53
  • There was another error. I'd defined the equals method on the iterator incorrectly. There now seems to be one last error with regards to const correctness. – bradgonesurfing Jun 26 '18 at 08:05
  • https://godbolt.org/g/m8qNe7 – bradgonesurfing Jun 26 '18 at 08:05
  • Simplify: use pointers for your iterators. They already work as iterators, one less pile of bugs to worry about, and all 0 and 1 size containers are random access and contiguous. ;) – Yakk - Adam Nevraumont Jun 26 '18 at 08:43
  • https://godbolt.org/g/236BCc. You can achieve the const correctness by storing a _pointer_ to the `optional` instead of the `optional` itself as a member. – mindriot Jun 26 '18 at 08:48
  • Thanks @mindriot. I'd just figured that myself a few minutes ago. I'll post a fixed version shortly. – bradgonesurfing Jun 26 '18 at 08:50
  • https://godbolt.org/g/UUzdpx is the fixed version – bradgonesurfing Jun 26 '18 at 08:55
  • Actually not quite. I had to fake the range using std::pair. Still can't resolve SinglePassRangeConcept if I pass the optional directly to foreach. – bradgonesurfing Jun 26 '18 at 08:56

1 Answers1

1

I'd keep it simple:

template <typename T> auto make_range(optional<T>& opt) {
    T* addr = opt? std::addressof(*opt) : nullptr;
    return make_iterator_range(addr, opt? addr+1 : addr);
}

I haven't tested this, but it seems simple enough to maybe just work.

sehe
  • 374,641
  • 47
  • 450
  • 633
  • Actually I might be able to combine the two operations. I don't like using make_range. ``optional`` should just work as a range. However my implementation at https://godbolt.org/g/UUzdpx might be able to be easily changed to use a pointer like your answer instead of the custom iterator. – bradgonesurfing Jun 26 '18 at 14:27
  • Yup. My sketch was the idea, you can have a tiny wrapper type that just does the required things in `begin()` and `end()` and have everything still inline away. I don't see a big difference from using `boost::iterator_range` except for the need to `#include `. So that's an implementation detail – sehe Jun 26 '18 at 14:29