c++initializationc++20undefined-behaviorbase-class

Initializing Base class and member using the same pointer without UB


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:

Does this solution still contain UB (or some other logic error), or is this safe?


Solution

  • *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.