I'm trying to use a std::unordered_map to store Resource objects with a std::string key. Resource implements type erasure such that the constructor Resource(objectOfAnyType) creates a Resource object which wraps the type of the object passed to the constructor.
It is important that Resources are not copied, since they may be wrapping objects which do not allow copying or only support shallow copying and which deallocate memory when they're destroyed. So I have been trying to use the std::pair piecewise constructor, e.g.
std::unordered_map<std::string, MyStruct> umap;
umap.emplace(std::piecewise_construct, std::forward_as_tuple("tag"), std::forward_as_tuple(constructorArgs));
This works fine. The problem comes when I try to do it with Resource which implements type erasure. Below is a full (but simplified) example which reproduces the problem I'm having. I realise there are several unrelated issues with the code (not least of all the dubious use of the heap in MyStruct) but it isn't production code and has been heavily edited to reproduce the problem with a simpler example.
#include <iostream>
#include <unordered_map>
#include <typeinfo>
#include <memory>
//Resource class providing a handle to generic game resources. Implements type erasure to support indefinite numbers of resource types.
class Resource
{
private:
//Allows mixed storage in STL containers.
class ResourceConcept
{
public:
virtual ~ResourceConcept() { }
};
//Templated model of ResourceConcept. Allows type erasure and storing of any type.
template <class ResourceType>
class ResourceModel : public ResourceConcept
{
private:
ResourceType modelledResource;
public:
ResourceModel(const ResourceType &_resource) : modelledResource(_resource) { }
virtual ~ResourceModel() { }
};
//Unique pointer to the resource itself. Points to an object of type ResourceConcept allowing any specific instantiation of the templated ResourceModel to be stored.
std::unique_ptr<ResourceConcept> resource;
//Uncommenting the two lines below causes an error, because std::pair is trying to copy Resource using operator=.
//Resource(const Resource* _other) = delete;
//Resource& operator= (const Resource&) = delete;
public:
//Constructor which initialises a resource with whichever ResourceModel is required.
template <class ResourceType>
Resource(const ResourceType& _resource) : resource(new ResourceModel<ResourceType>(_resource)) { std::cout << "Resource constructed with type " << typeid(_resource).name() << std::endl; }
};
//Example structure which we want to store/model as a Resource.
struct MyStruct
{
std::string* path;
MyStruct(std::string _path) : path(new std::string(_path)) { std::cout << "MyStruct constructor called..." << std::endl; }
~MyStruct() { std::cout << "MyStruct destructor called!" << std::endl; delete path; } //In a real example, this would deallocate memory, making shallow-copying the object a bad idea.
private:
MyStruct(const MyStruct* _other) = delete;
MyStruct& operator= (const MyStruct&) = delete;
};
int main()
{
std::unordered_map<std::string, Resource> umap;
std::string constructorArgs = "Constructor argument.";
//Store a MyStruct in the map using Resource(MyStruct) constructor...
umap.emplace(std::piecewise_construct, std::forward_as_tuple("tag1"), std::forward_as_tuple(constructorArgs)); //Calls Resource(std::string), which isn't what I want.
umap.emplace(std::make_pair("tag2", Resource(MyStruct(constructorArgs)))); //Calls a Resource(MyStruct), results in the MyStruct destructor being called twice! Example output below.
std::cout << "tag1: " << typeid(umap.at("tag1")).name() << "\ttag2: " << typeid(umap.at("tag2")).name() << std::endl;
std::cout << "End test." << std::endl;
/*
Example output:
Resource constructed with type Ss
MyStruct constructor called...
Resource constructed with type 8MyStruct
MyStruct destructor called! <--- I need to prevent this call to the destructor.
tag1: 8Resource tag2: 8Resource
End test.
MyStruct destructor called!
Segmentation fault
*/
return 0;
}
The main issue is that MyStruct seems to be copied, and one copy is destroyed. In real code this would cause a pointer to be deallocated leaving the other copy with a dangling pointer (hence the seg fault).
Is it possible to construct in place / piecewise pairs while using type erasure? Is it something std::move might be able to help with? One solution might be to use std::shared_ptrs, but if I am needlessly copying Resources anyway, I'd rather sort that out directly.
Thanks for any help.
If you do not want Resources to be copied anyway, you should prohibit that on your resource. Which you tried, but incorrectly: your deleted copy constructors (on MyStruct and on Resource) take a const pointer but it should be const references like
MyStruct(const MyStruct& _other) = delete;
Resource(const Resource& _other) = delete;
Once you use that you don't risk the segfault anymore since MyStruct can effectively not be copied anymore but you'll get a bunch of compiler errors because the rest of the code wants to copy. Enter move constructors, rvalue references and std::move as you already mentioned: I suggest to read about it on the web or SO until you fully understand it.
Applied to your particular code, it needs these changes: move constructor for MyStruct (eventually for Resource as well, but it is not used in the code you show) and the Resource constructor has to take an rvalue reference to the resource so it can move from it:
MyStruct MyStruct&& rh) :
path(rh.path)
{
rh.path = nullptr;
}
template <class ResourceType>
Resource(ResourceType&& _resource) :
resource(new ResourceModel<ResourceType>(std::forward<ResourceType>(_resource)))
{
std::cout << "Resource constructed with type " << typeid(resource).name() << std::endl;
}
Resource(Resource&& rh) :
resource(std::move(rh.resource))
{
}
Then you just use
umap.emplace("tag2", Resource(MyStruct(constructorArgs)))