Why this code does not work as expected?

Question

The following code does not work in Microsoft Visual Studio 2015:

#include <vector>
#include <list>
#include <iostream>

class ListWithIterator
{
public:
    ListWithIterator() : m_iterator(m_list.end()) {}

    bool check() const { return m_iterator == m_list.end(); }

private:
    typedef std::list<int> list_t;

    list_t m_list;
    list_t::const_iterator m_iterator;
};

int main(int, char**)
{
    std::vector<ListWithIterator> v;
    v.resize(1);
    if (v[0].check())
    {
        std::cerr << "Yes" << std::endl;
    }
    else
    {
        std::cerr << "No" << std::endl;
    }
    return 0;
}

MSVC says:

"Debug Assertion Failed", "Expression: list iterators incompatible" inside check() function.

g++ compiles it without any warnings but it works wrong:

$ g++ sample.cpp -g -Og -Wall
$ ./a.out
No
$

I expected output "Yes", because iterator was initialized by m_list.end() but bool check() const { return m_iterator == m_list.end(); } returns false.

Update:

Correct solution:

$ cat sample.cpp
#include <vector>
#include <list>
#include <iostream>

class ListWithIterator
{
public:
    ListWithIterator() : m_iterator(m_list.end()) {}
    ListWithIterator(const ListWithIterator& from): m_list(from.m_list), m_iterator(m_list.end())
    {
    }

    bool check() const
    {
        std::cerr << m_list.size() << std::endl;
        return m_iterator == m_list.end();
    }

private:
    typedef std::list<int> list_t;

    list_t m_list;
    list_t::iterator m_iterator;
};

int main(int, char**)
{
    std::vector<ListWithIterator> v;
    v.resize(1);
    if (v[0].check())
    {
        std::cerr << "Yes" << std::endl;
    }
    else
    {
        std::cerr << "No" << std::endl;
    }
    return 0;
}
$ g++ sample.cpp -g -Og -Wall
$ ./a.out
0
Yes
$

I think the problem is you are comparing a `const_iterator` with `iterator` in `check()`. — drescherjm, Jan 23 '17 at 21:44
Yes, it is. Put log into pastebin: http://pastebin.com/FDziGfMm — Anton Gorev, Jan 23 '17 at 21:48
No, it does not work even if I change `const_iterator` to `iterator`. — Anton Gorev, Jan 23 '17 at 21:50
With the new solution you lose the position of the iterator during a copy if that was important. What I mean is if the m_iterator was not pointing at the end() this is not preserved in the copy constructor. — drescherjm, Jan 23 '17 at 22:06

score 4 · Accepted Answer · edited May 23 '17 at 11:45

You are compiling without -std=c++11 flag, and there's only one pre-C++11 overload:

void resize( size_type count, T value = T() );

If the current size is less than count, additional elements are appended and initialized with copies of value.

Now, the defaulted copy constructor copy-constructs both member data and so you end up with an iterator to the old (destructed) list, which you later compare with an end iterator of a different list instance, leading to undefined behavior.

Compiling under C++11 would make the behaviour defined (an overload which uses the default constructor would be chosen), but it is in your own interest to fix the broken copy semantics.

As to Visual Studio, I am unsure of what could had (or have?) been the problem. VS2015 should have that one-parameter overload. Does it appear to work, too? — LogicStuff, Jan 23 '17 at 22:05

score 3 · Answer 2 · answered Jan 23 '17 at 21:50

If you're triggering the implicit copy constructor of ListWithIterator, then you end up with a copy of the list and a copy of the iterator. However, the copied iterator still refers to the original list. Comparisons of iterators for different contains is undefined.

I don't see where your ListWithIterator would get copied, but the message from MSVC suggests that it is being copied nonetheless.

score 3 · Answer 3 · answered Jan 23 '17 at 21:54

You haven't defined a copy constructor, and list::resize inserts a copy of a default-constructed object.
This copy contains an iterator that refers to the list in the original object, not to the list in the copy.

You need to be as careful with iterators as you would be with pointers.

Why this code does not work as expected?

3 Answers3