0

I am writing code that passes a std::unique_ptr through a few layers that look bad, but I don't have a choice but pass it all along for now.

The problem is that I am getting an error when I try to pass the std::unique_ptr to a constructor of the Provider class. At the point where Child::function1() is called, Child has been initialized/constructed with parameters of Impl& and std::unique_ptr<Retriever> from somewhere else.

Could someone please tell me why I'm getting this error, and how to get this work? I wonder if it's because of the inheritance happening, but I can't figure it out what else to do besides moving to get this work... (apart from something like not to pass this ptr through all these classes that seem to look similar with the same constructors... lol)

// child.cpp
Child::Child(Impl& child_impl, std::unique_ptr<Retriever> child_retriever_up)
: Parent(child_impl, std::move(child_retriever_up))
{}

T::Y Child::function1() const
{
    **Provider provider(d_impl, d_retriever_up);** 
    // these d_impl and d_retriever are in Parent class.
    ...
    return Y;
}

// child.h
class Child : public Parent {
public:
    // creators
    explicit Child(Impl& child_impl, std::unique_ptr<Retriever> child_retriever_up);
    ~Child() override = default;
    // accessors
    T::Y Child::function1() const;
private:
    Child(const Child&) = delete;
    Child& operator=(const Child&) = delete;
// parent.cpp
Parent::Parent(Impl& impl, std::unique_ptr<Retriever> retriever_up)
: d_impl(impl), d_retriever_up(std::move(retriever_up))
{}

// parent.h
class Parent {
public:
    // creators
    explicit Parent(Impl& impl, std::unique_ptr<Retriever> retriever_up);
    virtual ~Parent() = default;
    // copy constructor
    Parent(const Parent& parent) 
    : d_context(parent.d_impl)
    , d_retriever_up(std::move(*parent.d_retriever_up))
    {
    }
    // data
    Impl& d_impl;
    std::unique_ptr<Retriever> d_retriever_up;
private:
    //Parent(const Parent&) = delete;
    Parent& operator=(const Parent&) = delete;
// provider.cpp
Provider::Provider(Impl& impl, std::unique_ptr<Retriever> retriever_up)
: d_impl(impl)
, d_retriever(std::move(retriever_up))
{}

// provider.h
class Provider {
public:
    Provider(Impl& context, std::unique_ptr<Retriever> retriever_up);
    //copy contstructor
    Provider(const Provider& provider) 
    : d_impl(provider.d_impl)
    , d_retriever(std::move(*provider.d_retriever))
    {
    }

private:
    Impl& d_impl;
    std::unique_ptr<Retriever> d_retriever;
}
  • 1
    "these d_impl and d_retriever are in Parent class" -- that does not appear to be true. The (partially) shown Parent class does not have any member called `d_retriever`. And even if it were, that's your problem right there. Because this is an lvaluate, and the constructor parameter is copied by value, this requires a copy constructor. Which, in the case of `unique_ptr` is deleted. However, because the shown code fails to meet the requirements of a [mre] it is not possible to authoritatively state and explain this, as an answer. – Sam Varshavchik Jun 25 '22 at 02:28
  • whoops my bad on the d_retriever. I am editing now @SamVarshavchik – user2465084 Jun 25 '22 at 02:33
  • 2
    So, this was not real code, but made-up code? How do we know that whatever your edited code is, will be real code and not still some made-up code, so nobody wastes any time answering a question about made-up code, that would probably not work for the real code? Are you familiar with Stackoverflow's requirements for a [mre]? – Sam Varshavchik Jun 25 '22 at 02:37
  • 1
    *"I am getting an error"* -- I do not see the error message copied into the body of your question, nor do I see an indication of which line triggers the error. (Sometimes the former will handle the latter, because some compilers show you the problematic line -- and an arrow pointing to a specific character in the line -- as part of the error message.) – JaMiT Jun 25 '22 at 03:49
  • You also have an issue with the copy ctor of Provider... it doesn't follow const correctness. – ALX23z Jun 25 '22 at 10:11
  • 1
    The error message said more than "error: use of deleted function", and the rest of the information is the critical part. Don't paraphrase error messages. – Pete Becker Jun 25 '22 at 12:56

1 Answers1

1

In Child::function1(), you are passing d_retriever_up by value to the Provider constructor. That requires making a copy of d_retriever_up, which is impossible as it is a unique_ptr that has delete'd its copy constructor, hence the error.

You need to use std::move() to move d_retriever_up into Provider. You claim to be doing that, but you are not, at least not everywhere it is needed. Moving the caller's unique_ptr into the Provider constructor's retriever_up parameter is a separate operation than moving the retriever_up parameter into the Provider's d_retriever class member.

However, after you fix that, you will run into another problem. Child::function1() is marked asconst, which makes access to d_retriever_up be const in that context, so function1() can't modify d_retriever_up, including moves (since moving a unique_ptr sets its held pointer to nullptr).

So, to make the code work, you need to drop the const and add std::move():

T::Y Child::function1()
{
    Provider provider(d_impl, std::move(d_retriever_up));
    ...
    return Y;
}

That being said, moving d_retriever_up is questionable in your design. You might need to consider using std::shared_ptr instead.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770