Copy-Assignment Operator of class uses (copy and swap) and ordinary implement give different result

Question

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <iterator>
#include <memory>

using std::cout;
using std::vector;


class Has_ptr {
public:
    friend void swap(Has_ptr &lhs, Has_ptr &rhs);
    friend bool operator<(const Has_ptr &a, const Has_ptr &b);

    
    // constructor
    Has_ptr(const std::string &s, int num) : ptr(new std::string(s)), i(num) {}

    // copy constructor
    Has_ptr(const Has_ptr &a) : ptr(new std::string(*a.ptr)), i(a.i) {}


    // copy-assign operator
    /*
    Has_ptr& operator=(Has_ptr tmp) 
    {
        this->swap(tmp);
        return *this;
    }
    */

    /*
    Has_ptr& operator=(Has_ptr &tmp) 
    {
        std::string *x = new std::string(*(tmp.ptr));
        delete ptr;
        ptr = x;
        i = tmp.i;
        return *this;
    }
    */

    

    // destructor
    ~Has_ptr() {
        delete ptr;
    }
    
    void swap(Has_ptr &rhs) {
        using std::swap;
        swap(ptr, rhs.ptr);
        swap(i, rhs.i);
    }
    
    void print() {
        cout << *ptr << i;
    }
private:
    std::string *ptr;
    int i;
};


// add a swap function
void swap(Has_ptr &lhs, Has_ptr &rhs) {
    lhs.swap(rhs);
}


bool operator<(const Has_ptr &a, const Has_ptr &b) {
    return a.i < b.i;
}

In C++ Primer 5th,13.3 Swap, exercise 13.31. The question require give the class Has_ptr a < operator and create a vector with few element then sort the vector. The implementation of class is above.

Has_ptr that have 2 data member

string pointer ptr, point to dynamic memory
int i

Has_ptr class have following copy-control member

constructor takes a string and an int.
copy_constructor, implemented as the class act like value.
destructor that delete the dynamic memory allocated by ptr.

I have attempt two implementation for the copy-assign operator. One I first create temp string pointer variable x to save the RHS pointer, then delete LHS pointer and finally assign the RHS to LHS. However, this implementation makes the code unable to compiler.

Has_ptr& operator=(Has_ptr &tmp) 
{
    std::string *x = new std::string(*(tmp.ptr));
    delete ptr;
    ptr = x;
    i = tmp.i;
    return *this;
}

The second implementation used copy and swap. This makes code compiler and run fragmentally.

// copy and swap
Has_ptr& operator=(Has_ptr tmp) 
{
    this->swap(tmp);
    return *this;
}

My main function is the following

int main() {
    Has_ptr a("a", 0), b("b", 1), c("c", 2);
    a.print();
    vector<Has_ptr> coll{a, b, c};

    std::sort(coll.begin(), coll.end());

    for (auto h : coll) 
        h.print();
}

The second implementation produce no error/warning message while compiler and produce following output

a0a0

The first implementation produce following error message

In file included from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\algorithm:62:0,             

    from 13_31.c++:9:
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h: In instantiation of 'void std::__insertion_sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]':
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1882:25:   required from 'void std::__final_insertion_sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1968:31:   required from 'void std::__sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:4707:18:   required from 'void std::sort(_RAIter, _RAIter) [with _RAIter = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >]'
13_31.c++:88:39:   required from here
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1849:17: error: invalid initialization of non-const reference of type 'Has_ptr&' from an rvalue of type 'std::remove_reference<Has_ptr&>::type {aka Has_ptr}'
        *__first = _GLIBCXX_MOVE(__val);
                 ^
13_31.c++:42:14: note:   initializing argument 1 of 'Has_ptr& Has_ptr::operator=(Has_ptr&)'
     Has_ptr& operator=(Has_ptr &tmp)
              ^~~~~~~~
In file included from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:61:0,
                 from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\algorithm:62,
                 from 13_31.c++:9:
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_heap.h: In instantiation of 'void std::__pop_heap(_RandomAccessIterator, _RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]':
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1672:19:   required from 'void std::__heap_select(_RandomAccessIterator, _RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1930:25:   required from 'void std::__partial_sort(_RandomAccessIterator, _RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1945:27:   required from 'void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Size = int; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1965:25:   required from 'void std::__sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:4707:18:   required from 'void std::sort(_RAIter, _RAIter) [with _RAIter = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >]'
13_31.c++:88:39:   required from here
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_heap.h:246:17: error: invalid initialization of non-const reference of type 'Has_ptr&' from an rvalue of type 'std::remove_reference<Has_ptr&>::type {aka Has_ptr}'
       *__result = _GLIBCXX_MOVE(*__first);
                 ^
13_31.c++:42:14: note:   initializing argument 1 of 'Has_ptr& Has_ptr::operator=(Has_ptr&)'
     Has_ptr& operator=(Has_ptr &tmp)
              ^~~~~~~~
In file included from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:61:0,
                 from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\algorithm:62,
                 from 13_31.c++:9:
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_heap.h: In instantiation of 'void std::__adjust_heap(_RandomAccessIterator, _Distance, _Distance, _Tp, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Distance = int; _Tp = Has_ptr; _Compare = __gnu_cxx::__ops::_Iter_less_iter]':
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_heap.h:335:22:   required from 'void std::__make_heap(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1669:23:   required from 'void std::__heap_select(_RandomAccessIterator, _RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1930:25:   required from 'void std::__partial_sort(_RandomAccessIterator, _RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1945:27:   required from 'void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Size = int; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1965:25:   required from 'void std::__sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:4707:18:   required from 'void std::sort(_RAIter, _RAIter) [with _RAIter = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >]'
13_31.c++:88:39:   required from here
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_heap.h:220:29: error: invalid initialization of non-const reference of type 'Has_ptr&' from an rvalue of type 'std::remove_reference<Has_ptr&>::type {aka Has_ptr}'  
    *(__first + __holeIndex) = _GLIBCXX_MOVE(*(__first + __secondChild));
                             ^
13_31.c++:42:14: note:   initializing argument 1 of 'Has_ptr& Has_ptr::operator=(Has_ptr&)'
     Has_ptr& operator=(Has_ptr &tmp)
              ^~~~~~~~
In file included from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:61:0,
                 from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\algorithm:62,
                 from 13_31.c++:9:
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_heap.h:226:29: error: invalid initialization of non-const reference of type 'Has_ptr&' from an rvalue of type 'std::remove_reference<Has_ptr&>::type {aka Has_ptr}'
    *(__first + __holeIndex) = _GLIBCXX_MOVE(*(__first
                             ^
13_31.c++:42:14: note:   initializing argument 1 of 'Has_ptr& Has_ptr::operator=(Has_ptr&)'
     Has_ptr& operator=(Has_ptr &tmp)
              ^~~~~~~~
In file included from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\char_traits.h:39:0,
                 from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\ios:40,
                 from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\ostream:38,
                 from c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\iostream:39,
                 from 13_31.c++:6:
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algobase.h: In instantiation of 'static _BI2 std::__copy_move_backward<true, false, std::random_access_iterator_tag>::__copy_move_b(_BI1, _BI1, _BI2) [with _BI1 = Has_ptr*; _BI2 = Has_ptr*]':
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algobase.h:588:58:   required from '_BI2 std::__copy_move_backward_a(_BI1, _BI1, _BI2) [with bool _IsMove = true; _BI1 = Has_ptr*; _BI2 = Has_ptr*]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algobase.h:598:5:   required from '_BI2 std::__copy_move_backward_a2(_BI1, _BI1, _BI2) [with bool _IsMove = true; _BI1 = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _BI2 = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algobase.h:668:48:   required from '_BI2 std::move_backward(_BI1, _BI1, _BI2) [with _BI1 = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _BI2 = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1848:8:   required from 'void std::__insertion_sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1882:25:   required from 'void std::__final_insertion_sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:1968:31:   required from 'void std::__sort(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >; _Compare = __gnu_cxx::__ops::_Iter_less_iter]'
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algo.h:4707:18:   required from 'void std::sort(_RAIter, _RAIter) [with _RAIter = __gnu_cxx::__normal_iterator<Has_ptr*, std::vector<Has_ptr> >]'
13_31.c++:88:39:   required from here
c:\mingw\lib\gcc\mingw32\6.3.0\include\c++\bits\stl_algobase.h:548:18: error: invalid initialization of non-const reference of type 'Has_ptr&' from an rvalue of type 'std::remove_reference<Has_ptr&>::type {aka Has_ptr}'
      *--__result = std::move(*--__last);
      ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
13_31.c++:42:14: note:   initializing argument 1 of 'Has_ptr& Has_ptr::operator=(Has_ptr&)'
     Has_ptr& operator=(Has_ptr &tmp)
              ^~~~~~~~

The non-swapping copy-assignment operator should take a const reference parameter. — molbdnilo, Jan 10 '22 at 07:18
Consider using std::uniqur_ptr, it is as fast and small to store as normal pointer, has smaller source code and is easier to use. E.g. it deletes automatically at the destructor or when you overwrite the pointer in the assignment operator. — Sebastian, Jan 10 '22 at 09:24
Thanks for the suggestion Sebastion. I should use smart_pointer in real scenario. This is an exercise from the book. — David Zhou, Jan 10 '22 at 23:51

Copy-Assignment Operator of class uses (copy and swap) and ordinary implement give different result

0 Answers0