boostorg / multi_index

Boost.org multi_index module
http://boost.org/libs/multi_index
45 stars 59 forks source link

Question: default iterator are different? #25

Closed filcuc closed 5 years ago

filcuc commented 5 years ago
#include <iostream>
#include <cstdlib>
#include <boost/multi_index_container.hpp>
#include <boost/multi_index/ordered_index.hpp>
#include <boost/multi_index/identity.hpp>
#include <boost/multi_index/member.hpp>
#include <boost/multi_index/tag.hpp>
#include <boost/multi_index/hashed_index.hpp>

namespace bmi = boost::multi_index;

namespace Node {
    using Id = std::int64_t;
    struct Tag {};
}

namespace Edge {
    using Id = std::int64_t;
    struct Tag {};
}

struct IndexById {};
struct IndexBySource {};
struct IndexByTarget {};

    struct EdgeData {
        Edge::Id id;
        bool deleted = false;
        Node::Id source;
        Node::Id target;
    };

    using EdgeDataContainer = boost::multi_index_container<
    EdgeData,
    bmi::indexed_by<
    bmi::ordered_unique<bmi::tag<IndexById>, bmi::member<EdgeData, Edge::Id, &EdgeData::id>>,
    bmi::ordered_non_unique<bmi::tag<IndexBySource>, bmi::member<EdgeData, Node::Id, &EdgeData::source>>,
    bmi::ordered_non_unique<bmi::tag<IndexByTarget>, bmi::member<EdgeData, Node::Id, &EdgeData::target>>
    >
    >;

        using EdgeDataContainerByIdIndex = typename bmi::index<EdgeDataContainer, IndexById>::type;
    using EdgeDataContainerBySourceIndex = typename bmi::index<EdgeDataContainer, IndexBySource>::type;
    using EdgeDataContainerByTargetIndex = typename bmi::index<EdgeDataContainer, IndexByTarget>::type;

int main()
{
    EdgeDataContainerByIdIndex::const_iterator it_1, it_2;

    std::cout << (it_1 == it_2 ? "true" : "false") << std::endl;
}

Is there a reason why this print false? i would expected to print true since the iterator are both default constructed

joaquintides commented 5 years ago

Hi Filippo,

A default-initialized iterator is in a so-called singular state, where the only thing you can validly do with it is, basically, assign it a non-singular value (i.e. the value of some other iterator that actually points to a sequence). If you like standardese, details can be found here. So your program is engendering undefined behavior because you're comparing two singular values (it_1 and it_2): in principle, the program can output true, false, or wipe up your hard disk, as the old joke about UB goes.

I've run your program in Wandbox with Boost.MultiIndex safe mode on and the result is:

a.out: /usr/local/include/boost/multi_index/detail/safe_mode.hpp:468:
  bool boost::multi_index::safe_mode::safe_iterator<Iterator, Container>::equal(
     const boost::multi_index::safe_mode::safe_iterator<Iterator, Container>&) const [with ...]:
  Assertion `safe_mode::check_valid_iterator(*this)' failed.
  ...

This issue is not exclusive to Boost.MultiIndex, and messing with singular iterators raises UB with any standard container. For example:

#include <list>
#include <iostream>

int main()
{
  std::list<int>::iterator it1,it2;
  std::cout<<(it1==it2)<<"\n";
}

is detected as invalid when run in Wandbox in _GLIBCXX_DEBUG mode:

/opt/wandbox/gcc-9.1.0/include/c++/9.1.0/debug/safe_iterator.h:454:
In function:
    bool __gnu_debug::operator==(const _Self&, const _Self&)

Error: attempt to compare a singular iterator to a singular iterator.
...
filcuc commented 5 years ago

@joaquintides thank you for the insightful answer!! Ok than I will use an optional for handling not initialized iterators

joaquintides commented 5 years ago

Glad to be helpful. You may also want to ask yourself why your program needs to handle uninitialized entities --maybe you can refactor your design so as to get rid of this requirement.