While reviewing some testing code, I have come across this situation:
Suppose we have these two classes:
class Project
{
public:
Project(std::string path);
~Project;
// [many more functions]
};
class ResourceManager
{
public:
ResourceManager(Project &proj);
virtual ~ResourceManager();
// [many more functions]
// [notably no getProject()]
};
In the test code, there exists a 'fake' version of the ResourceManager:
class FakeResourceManager : public ResourceManager
{
std::unique_ptr<Project> project_;
Project* makeProject()
{
project_= std::make_unique<Project>("/some/path"); // oops, accessing non-initialized member
return project_.get();
};
public:
FakeResourceManager()
: ResourceManager(*makeProject())
{}
virtual ~FakeResourceManager(); // defaulted in cpp
void markResourceAsDirty(); // 'extension' of base functionality for testing
};
This of course doesn't work, since the project_ member is not initialized yet when makeProject is called, which the address sanitizer catches at runtime.
I reworked this initialization code, but I am unsure if it's now free of any UB and does the expected thing:
class FakeResourceManager : public ResourceManager
{
std::unique_ptr<Project> project_;
FakeResourceManager(std::unique_ptr<Project> project)
: ResourceManager(*project.get())
, project_(std::move(project)) //(1)
{}
public:
FakeResourceManager()
: FakeResourceManager(std::make_unique<Project>("/some/path"))
{}
virtual ~FakeResourceManager(); // defaulted in cpp
void markResourceAsDirty(); // 'extension' of base functionality for testing
}
Points I considered:
std::move (1) of the unique_ptr should not invalidate the pointer previously returned by get(). The moved-to member should correctly delete the Project on destruction.ResourceManager destructor uses the passed project reference it would be UB, since that object already got deleted by the derived class dtor. Since it doesn't, it should be ok to have a dangling reference here (or is this still UB?)Does this solution still contain UB (or some other logic error), or is this safe?
*project.get() is unsafe to me (and can just be *project).
To fix your problem, I'd do this:
struct HasProject {
std::unique_ptr<Project> project_;
[[nodiscard]] Project& makeProject(std::string_view path) {
assert(!project_);
project_= std::make_unique<Project>(path);
return *project_;
};
};
class FakeResourceManager : private HasProject, public ResourceManager
{
public:
FakeResourceManager()
: ResourceManager(makeProject("/some/path"))
{}
virtual ~FakeResourceManager(); // defaulted in cpp
void markResourceAsDirty(); // 'extension' of base functionality for testing
};
we move the storage into an earlier than ResourceManager base class, and that base class is fully initialized by the time we construct ResourceManager.
We can handle failures of project construction within makeProject (throwing or whatever), and don't have a method that takes a unique_ptr but fails catastrophically on an nullptr.