Move assignment operator in a QList<T>-derived class



  • Hello everyone,
    I tried to set up a move assignment operator in a custom QList<T>-derived class (T being a custom basic class with no parent class), but it crashes for no obvious reason (at least to me). Here is a sample of the code :

    MyList.hpp :

    class MyList : public QList<T>
    {
    public:
        MyList(); // default constructor
        MyList (const MyList & list); // copy constructor
        MyList (MyList && list); // move constructor
        MyList & operator=(const MyList & list); // copy assignment
        MyList & operator=(MyList && list); // move assignment
    
    private:
        int _x;
    };
    

    MyList.cpp notably contains those two implementations :

    MyList::MyList () // default constructor
        : QList<T>(),
    	  _x (0)
    {}
    

    and

    MyList & MyList::operator=(MyList && list) // move assignment
    {
        if (this != & list)
        {
            this->_x = list._x;
            list._x = 0;
            QList<T>::operator=(std::move(list)); // is that even correct ?
        }
        return *this;
    }
    

    Now, if I execute this from another class :

    void A::search ()
    {
        MyList temp_list;		
        for (int i = 1; i <= 15; i++)
        {
    		temp_list = this->deepSearch (i, ... other params ...); // calls the move assignment
    		...
        }
    }
    
    MyList && A::deepSearch (int, ...)
    {
    	MyList list;
    	...
    	return std::move (list);
    }
    

    it crashes right after that line in the MyList move assignment :

    QList<T>::operator=(std::move(list));
    

    At that point, the list is empty. When hitting F10 in the debugger while on that line, it triggers a SIGILL (illegal instruction) signal, and the debugger cursor moves to the default MyList constructor :

    MyList::MyList () // default constructor
        : QList<T>(),
    	  _x (0) // <--- debugger cursor move here after SIGILL
    {}
    

    Questions are :

    • is QList<T>::operator=(std::move(list)); correct inside the move assignment ?
    • why would the cursor move to the MyList default constructor at that point ?

    That program (well actually it's a custom library) has been working fine for years using the copy assignment. I'd like to check how faster it can go by using move assignments. Of course I could probably make it faster by using MyLIst & references in many places, but the move approach seems to be the cleanest one.

    I tried to set a very basic Qt project in order to reproduce the bug, but of course that project doesn't crash ...

    Thanks for any ideas !



  • It works for me, you can try it with simpler example, maybe the bug come from another place

    MyList<int> list;
    list.push_back(1);
    MyList<int> list2;
    list2.push_back(2);
    
    list = std::move(list2);
    for(auto const Data : list){
        std::cout<<Data<<std::endl;
    }
    std::cout<<std::endl;
    for(auto const Data : list2){
        std::cout<<Data<<std::endl;
    }
    
    
    template<typename T>
    class MyList : public QList<T>
    {
    public:
        MyList(); // default constructor
        MyList (const MyList & list) = default;
        MyList (MyList && list) = default;
        MyList & operator=(const MyList & list) = default;
        MyList & operator=(MyList && list)
        {
            std::cout<<"move"<<std::endl;
            if (this != &list)
            {
                this->_x = list._x;
                list._x = 0;
                QList<T>::operator=(std::move(list));            
            }
            return *this;
        }
    
    private:
        int _x;
    };
    
    template<typename T>
    MyList<T>::MyList () // default constructor
        : QList<T>(),
          _x (0)
    {}
    

    One more thing, some compiler like to use "__" as their variable prefix, maybe this should be considered when you try to name your own variable.



  • Well I've been investigating and I notice that the application crashes (SIGILL) with this :

    MyList && A::deepSearch (int, ...) { ... return std::move (list);}
    

    but works fine with this :

    MyList A::deepSearch (int, ...) { ... return list;}
    

    although in both cases, the line

    temp_list = this->deepSearch (i, ... other params ...);
    

    uses the move assignment, not the copy one.

    It looks like the compiler (MinGW 4.9.1) is - rightfully - using the move approach in the second case, even if not explicitly asked for.

    Also I did a few benchmarks, and the move approach appears to be slightly slower (not faster) than the copy approach (for this, I removed the MyList move assignment implementation).

    According to that other post : c++ 11, Moving slower than copying?!?, it looks like I should use MyList* allocated pointers to make the move assignment much faster.

    I also compared the debug trace leading to the original crash, and the debug trace when the compiler automatically uses the move assignment without crashing. The only difference is that after the QList<T> move is done, and before
    returning *this, the trace of the original code switches to the default MyList constructor for no obvious reason. Still a mystery to me, but at least there's a working alternative now.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.