Custom shared_ptr

[Olysold]

I'm trying to write my own version of shared_ptr and I just don't get what this error is about.

Shared_ptr header.

#ifndef SHARED_PTR_H_INCLUDED
#define SHARED_PTR_H_INCLUDED

#include <iostream>
#include "Make_shared.h"

template <typename A> class Make_shared;

template <typename T>
class Share
{
template <typename A> friend class Make_shared;
template <typename A>
friend std::ostream& operator<<(std::ostream&, const Share<A>&);

public:
    //Constructors
    Share();
    Share(T *p);

    //Overloaded operators
    T &operator*();
    void operator()(T *p);
    Share &operator=(Make_shared<T>&);

private:
    T *pointer;
};

//Constructors
template <typename T>
Share<T>::Share(): pointer(nullptr) { }

template <typename T>
Share<T>::Share(T *p)
{
    pointer = new T(*p);
}

//Non-member overloaded operators
template <typename T>
std::ostream& operator<<(std::ostream &os, const Share<T> &obj)
{
    os << obj.pointer;
}

//Member overloaded operators
template <typename T>
T &Share<T>::operator*()
{
    return *pointer;
}

template <typename T>
void Share<T>::operator()(T *p)
{
    pointer = new T(*p);
}

template <typename T>
Share<T> &Share<T>::operator=(Make_shared<T> &rhs)
{
    *pointer = *rhs.new_data;
    pointer = rhs.new_data;
    return this;
}

#endif // SHARED_PTR_H_INCLUDED 

Make_shared header

#ifndef MAKE_SHARED_H_INCLUDED
#define MAKE_SHARED_H_INCLUDED

#include "Shared_ptr.h"

template <typename A> class Share;

template <typename T>
class Make_shared
{
template <typename A> class Share;
public:
    //constructor
    Make_shared() = delete;
    Make_shared(T);

private:
    T *new_data;
};

template <typename T>
Make_shared<T>::Make_shared(T n)
{
    new_data = new T(n);
}

#endif // MAKE_SHARED_H_INCLUDED 

main.cpp

#include<iostream>

#include "Shared_ptr.h"
#include "Make_shared.h"

using namespace std;

int main()
{
    Share<int> p = Make_shared<int>(50);
}

Error: conversion from 'Make_shared<int> to non-scalar type 'Share<int>' requested.

Can anyone shed some light on this please?

[MiiNiPaa]

You should either make Make_shared into a function returning rvalue reference to Share object and create a move constructor to Share

or you can create constructor of Share which takes Make_shared object as argument (less intuitive solution)

Yor problem lies in fact that line 10 in main.cpp does not call an assigment operator. It tries to convert Make_shared to Share.

[Olysold]

#ifndef SHARED_PTR_H_INCLUDED
#define SHARED_PTR_H_INCLUDED

#include <iostream>

template <typename T> class Share;

template <typename T>
Share<T>&& Make_shared(const T n)
{
    return Share<T>(new T(n));
}

template <typename T>
class Share
{
template <typename A>
friend std::ostream& operator<<(std::ostream&, const Share<A>&);

public:
    //Constructors
    Share();
    Share(T*);
    Share(Share &&) noexcept;

    //Copy Constructors
    Share(const Share&);
    Share& operator=(Share &&);

    //Destructor
    ~Share()
    {
        if (--*cnt == 0)
        {
            delete pointer;
            delete cnt;
        }
    }

    //Overloaded operators
    T &operator*();

private:
    T *pointer;
    size_t *cnt;
};

//Constructors
template <typename T>
Share<T>::Share(): pointer(nullptr), cnt(new size_t(1)) { }

template <typename T>
Share<T>::Share(T *p): pointer(nullptr), cnt(new size_t(1))
{
    pointer = new T(*p);
}

template <typename T>
Share<T>::Share(Share<T> &&rhs) noexcept : pointer(nullptr), cnt(new size_t(1))
{
    pointer = new T (*rhs.pointer);
    *cnt = *rhs.cnt;
}

//Copy constructors
template <typename T>
Share<T>::Share(const Share<T> &rhs):
    pointer(rhs.pointer), cnt(rhs.cnt) { ++*cnt; }

template <typename T>
Share<T> &Share<T>::operator=(Share<T> &&rhs)
{
    ++*rhs.cnt;
    pointer = rhs.pointer;
    cnt = rhs.cnt;

    return *this;
}


//Non-member overloaded operators
template <typename T>
std::ostream& operator<<(std::ostream &os, const Share<T> &obj)
{
    os << obj.pointer;
    return os;
}

//Member overloaded operators
template <typename T>
T &Share<T>::operator*()
{
    return *pointer;
}

#endif // SHARED_PTR_H_INCLUDED 

Not sure how to continue from here. I can't steal the data from Make_shared it just gets immediately destroyed.

[MiiNiPaa]

template <typename T>
Share<T>&& Make_shared(const T n)
{
    return std::move(Share<T>(new T(n)));
}

After code review I want to say that your Shared_Ptr is wrong. It would create many copies of the same object but will delete only last one. Also move constructor is inefficient

[Olysold]

Could you please elaborate how it would create many copies but just delete the last one?

[MiiNiPaa]

pointer = new T(*p);This will crete new copy of variable stored in p. And only last Shared_Ptr in group will actually delete his own copy. So there:
a) memory leak
b) Different Shared_Ptr will always point to different objects. You cannot have two shared pointer pointing to same object, which defats its purpose.

[Olysold]

That's for a constructor that takes built-in pointers not other shared_ptr. I just copied what I could observe that the std::shared_ptr could do.

The other new in the move constructor I removed it and replaced with this

template <typename T>
Share<T>::Share(Share<T> &&rhs) noexcept: pointer(nullptr), cnt(new size_t(1))
{
    pointer = rhs.pointer;
    cnt = rhs.cnt;

    rhs.pointer = nullptr;
    *rhs.cnt = 0;
}

Not sure if the last two bits is necessary though.

Aren't shared_ptr suppose to share the object? I mean if they are a copy of each other.

int main()
{
    shared_ptr<int> p1 = make_shared<int>(4);
    shared_ptr<int> p2 = p1;
    shared_ptr<int> p3;

    p3 = p2;

    cout << p1 << endl;
    cout << p2 << endl;
    cout << p3 << endl;
}

They all point to the same address, doesn't that mean they are all pointing to the same object?

[MiiNiPaa]

Ok, I was looking into move constructor. It is better now.
Some other promlems with move semantic:

{
    shared_ptr<int>* p1;
    *p1 =  make_shared<int>(4);
    shared_ptr<int> p2 = std::move(*p1);
}//Object stored in p2 and *p1 is not destroyed here. 

Move assumes that moved-from pointer will lose ownership of object and will not be counted in reference count.

[Olysold]

I couldn't quite figure out how to deal with the move thing and returning by rvalue with the Make_shared. During the return from Make_shared the data get's destroyed and nothing gets passed. What you just said sounds like it isn't quite the right solution. Then again I may be misunderstanding.

Looking around several documentations it says that Make_shared returns a shared_ptr<T> type. So I just settle with this.

template <typename T>
Share<T> Make_shared(const T n = T())
{
    Share<T> ret;
    ret.pointer = new T(n);
    ret.cnt = new size_t(1);

    return ret;
}

int main()
{
    Share<int> p = Make_shared<int>(5);
}

Would there be a memory leak here? I'm thinking that p is pointing to the object ret is pointing to and if p gets deleted, there shouldn't be a problem.

[MiiNiPaa]

Yes, it is fine.
Only problem with

int main()
{
    Share<int> p = Make_shared<int>(5);
    p = Make_shared<int>(10);
}

first in would not be destroyed. You should handle this situation. To test such situations I recommend to make class which will output each construction and destruction.


And provide non-move constructors and assigment operator too!

[Olysold]

Okay I'll add them in. The assignment operator is already redefined to handled that problem, unfortunately I copied it from the book. I also have std::couts all over the place to find out the construction and destruction, but I should get into a habit of utilizing class for that like you said, or functs.

Thanks for the help =).