[BOOST_FOREACH] Problem to iterate on a non-copyable container...

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[BOOST_FOREACH] Problem to iterate on a non-copyable container...

nico
I'm not sure of the title, because I'm not sure the issue comes from the
"copyablility" of my container.
I tryied quite everything but I can't get rid of this error.

Here is a simplified version of my code (please do not challenge the class
design, I really would like to keep the end-used syntax in the
BOOST_FOREACH):

    template <typename T>
    class MyContainer
    {
    public:
        typedef typename std::vector<T>::iterator iterator;
        typedef typename std::vector<T>::const_iterator const_iterator;

        MyContainer(std::vector<T>& vec, boost::mutex& mutex) :
            m_vector(vec),
            m_lock(mutex)
        {
        }

        iterator begin() { return m_vector.begin(); }
        const_iterator begin() const { return m_vector.begin(); }
        iterator end() { return m_vector.end(); }
        const_iterator end() const { return m_vector.end(); }


    private:
        std::vector<T>& m_vector;
        boost::lock_guard<boost::mutex> m_lock;
    };

    template <typename T>
    struct GetContainer
    {
        GetContainer(std::vector<T>& vec, boost::mutex& mutex) :
            m_vector(vec),
            m_mutex(mutex)
        {
        }

        MyContainer<T> Get()
        {
            return MyContainer<T>(m_vector, m_mutex);
        }

        std::vector<T>& m_vector;
        boost::mutex& m_mutex;
    };



    int main()
    {
        std::vector<int> v;
        v.push_back(1);
        v.push_back(2);
        boost::mutex m;

        GetContainer<int> getter(v, m);

        BOOST_FOREACH(int i, getter.Get())
        {
            std::cout << i << std::endl;
        }

        return 0;
    }


The compiler complains about not having a copy constructor
for MyContainer<int>::MyContainer(const MyContainer<int>&)
I also have :
error: no matching function for call to
‘MyContainer<int>::MyContainer(boost::foreach_detail_::rvalue_probe<const
MyContainer<int> >::value_type)’

I follow the extensibility tips:
http://www.boost.org/doc/libs/1_58_0/doc/html/foreach/extensibility.html#foreach.extensibility.making__literal_boost_foreach__literal__work_with_non_copyable_sequence_types

But, making MyContainer<T> : private boost::noncopyable
doesn't solve the issue.
Nor defining the function boost_foreach_is_noncopyable or specializing the
template struct is_noncopyable for MyContainer<int> (in fact, how would I
specialize this template for a template type ?)

Last "tip":
If I remove the mutex and the lock from everywhere (I just pass the vector
to GetContainer and to MyContainer<T>, it works.
But it doesn't work if I make MyContainer<T> : private boost::noncopyable
(I expected it should, so I'm not sure my problem is with BOOST_FOREACH,
but maybe because I return a copy of MyContainer with my getter ?)

I thank you if you read me until here, and thanks in advance for help.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [BOOST_FOREACH] Problem to iterate on a non-copyable container...

Paul Fultz II
> The compiler complains about not having a copy constructor

That is because you are passing an rvalue to BOOST_FOREACH. All non-copyable
containers must be passed as an lvalue.

Paul
Reply | Threaded
Open this post in threaded view
|

Re: [BOOST_FOREACH] Problem to iterate on a non-copyable container...

Chris Glover
In reply to this post by nico
On Tue, 19 May 2015 at 15:16 nico <[hidden email]> wrote:

> I'm not sure of the title, because I'm not sure the issue comes from the
> "copyablility" of my container.
> I tryied quite everything but I can't get rid of this error.
>
>
Hi Nico,

You can't do what you are trying to do in C++03.  To accomplish it, you
need C++11 move semantics to be able to move the MyContainer out of the Get
function. Even without using BOOST_FOREACH, the following code fails;

GetContainer<int> getter(v, m);
MyContainer<int> c = getter.Get(); // <-- Error.

Here's an example with the necessary changes; I changed the scoped_lock to
a unique_lock and added a move constructor.

template <typename T>
class MyContainer
{
public:
    typedef typename std::vector<T>::iterator iterator;
    typedef typename std::vector<T>::const_iterator const_iterator;

    MyContainer(std::vector<T>& vec, boost::mutex& mutex)
        : m_vector(&vec)
        , m_lock(mutex)
    {}

    MyContainer(MyContainer&& other)
        : m_vector(other.m_vector)
    {
        m_lock = std::move(other.m_lock);
        other.m_vector = nullptr;
    }

    iterator begin() { return m_vector->begin(); }
    const_iterator begin() const { return m_vector->begin(); }
    iterator end() { return m_vector->end(); }
    const_iterator end() const { return m_vector->end(); }

private:

    std::vector<T>* m_vector;
    boost::unique_lock<boost::mutex> m_lock;
};

template <typename T>
struct GetContainer
{
    GetContainer(std::vector<T>& vec, boost::mutex& mutex) :
        m_vector(vec),
        m_mutex(mutex)
    {}

    MyContainer<T> Get()
    {
        return MyContainer<T>(m_vector, m_mutex);
    }

    std::vector<T>& m_vector;
    boost::mutex& m_mutex;
};

Once you have move semantics though, you should have access to
range-based-for, which is also C++11

    for(auto&& i : getter.Get())
    {
        std::cout << i << std::endl;
    }

The above code works, so do you still need BOOST_FOREACH?

-- chris

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [BOOST_FOREACH] Problem to iterate on a non-copyable container...

nico
Hello Chris, and thank you for your reply.

I'm feel like an old guy: I don't even ear about move semantic. I wish I'll be quickly able to work on a modern os and compiler...

For my case, with the old C++, I finally made almost like you: change the lock to an unqiue lock, and provide a copy constructor:

    MyContainer(const MyContainer& other)
        : m_vector(other.m_vector),
          m_lock(*other.m_lock.mutex())
    {
    }

It's not a move semantic, but it did the job, and is usable with BOOST_FOREACH.
But I have to change the mutex to a recursive_mutex...
Reply | Threaded
Open this post in threaded view
|

Re: [BOOST_FOREACH] Problem to iterate on a non-copyable container...

Paul Fultz II
> I finally made almost like you: change the lock to an unqiue lock, and provide
> a copy constructor

Actually, I think it would be better to default construct the lock on copy:

    MyContainer(const MyContainer& other)
        : m_vector(other.m_vector),
          m_lock()
    {
    }

Because on copy you want to severe the relationship, so there is one lock
associated with each copy of the document(or data).

Paul