Memory error due to std::map iterator in a class derived from std::map

Question

I am deriving a class from std::map as I wish to create my own methods for this data structure. I am having issues with "mySelect", which should return nullptr if the element is not there and unique_ptr otherwise.

I have tried specifying the typename keyword before declaration of the iterator to no avail.

template <class KeyType, class ValueType>
class Container : public std::map<KeyType, ValueType> {

    public:
        std::unique_ptr<ValueType> mySelect(KeyType key) {
        typename map<KeyType, ValueType>::iterator value;
            if ((value = this->find(key)) == this->end())
            return nullptr;
        return std::make_unique<ValueType>(value);
        }
}

I'm getting this error:

Error   C2664   'std::vector<std::shared_ptr<Transaction>,std::allocator<_Ty>>::vector(const std::vector<_Ty,std::allocator<_Ty>> &)': cannot convert argument 1 from 'std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<_Ty>>>' to 'const _Alloc &'

Not your problem, but worth reading: [Is there any real risk to deriving from the C++ STL containers?](https://stackoverflow.com/questions/922248/is-there-any-real-risk-to-deriving-from-the-c-stl-containers) — Yksisarvinen, Jun 07 '19 at 08:54
It is supposed to return a unique_ptr pointing to the value of type Value found by the iterator. — Ninrich, Jun 07 '19 at 08:54
Then you are completely on the wrong path. 1 - `std::make_unique` creates a new instance. 2 - `std::unique_ptr` provides ownership. 3 - you try to create a new instance and initialize it from an iterator — Slava, Jun 07 '19 at 08:56
The code breaks before it could be used. I believe this class should compile on its own. — Ninrich, Jun 07 '19 at 08:57
You're trying to call constructor of `ValueType`, giving as argument a single object of type `std::map::iterator`. I suppose you meant `std::make_unique(*value);`, but it's still very unclear what are you trying to do with the ownership of the objects here (why not return simply `ValueType` for example?) — Yksisarvinen, Jun 07 '19 at 09:01
standard containers arent really made to be publicly inherited from. for starters they have no virtual destructor — 463035818_is_not_an_ai, Jun 07 '19 at 09:01
I am returning a unique_ptr because it was my task to. How is it possible to make the caller of mySelect() to become the owner of the returned object? — Ninrich, Jun 07 '19 at 09:04

Slava · Accepted Answer · 2019-06-07T09:05:09.447

First of all this code:

 return std::make_unique<ValueType>(value);

is logically equal to this:

std::unique_ptr<ValueType> tmp = new Value(value);
return tmp;

(it is not the same though so you should not replace one with another, just to make you understand). So you are trying to create a new instance of class Value and initialize it from the iterator. This would not work unless Value would provide such constructor. If you want to make a copy and return it with transferring ownership, then change your code to:

 return std::make_unique<ValueType>(value->second);

but I am not sure this is what you want to do. If you want to return a pointer to existing object, you cannot use std::unique_ptr here as it provides unique ownership (hense the name), you either need to store std::shared_ptr in your map instead of object by value and return a copy of it, or simply return a raw pointer.

How is it possible to make the caller of mySelect() to become the owner of the returned object?

Either as I said you store object by std::shared_ptr and share ownership with the caller of this method, or you store object originally as std::unique_ptr but then you have to move it out, as your std::map would not be able to own that object anymore.

Memory error due to std::map iterator in a class derived from std::map

1 Answers1