5

I have a class called classA, something like this:

class classA {
    private:        
        char* data; 
    public:
        classA(const classA&) = delete;
        ~classA();      
    };

~classA()
    {
        delete[] data;
    }

In another class, let's call it classB, I have as a member a shared pointer to classA:

class classB
    {
    private:
        std::shared_ptr<classA> ptrA;
    public: 
        classB(std::shared_ptr<classA>);
    };

classB(std::shared_ptr<classA> sp) : ptrA(sp)
    {}

This is how I instantiate my classB:

classA ca;
classB cb(std::make_shared<classA>(ca));

This gives me the following error:

attempting to reference a deleted function

Obviously, I am trying to reference the copy constructor, which I defined as deleted (there is a reason for this, objects of this class shouldn't be copied). But I am confused as to why the copy constructor is called since I am passing a shared pointer, and how to avoid this.

Justin
  • 24,288
  • 12
  • 92
  • 142
Eutherpy
  • 4,471
  • 7
  • 40
  • 64
  • how do you suspect the shared pointer is constructed? – kmdreko Apr 12 '18 at 23:33
  • Note that if you did `std::make_shared(std::move(ca))`, you'd still have a problem, see [Default move constructor/assignment and deleted copy constructor/assignment](https://stackoverflow.com/questions/37276413/default-move-constructor-assignment-and-deleted-copy-constructor-assignment) – Justin Apr 12 '18 at 23:35
  • 1
    You should show actual code, your code cannot have that issue. – Slava Apr 12 '18 at 23:36
  • 2
    `make_shared` is trying to construct a `classA` from another `classA` (`ca`). What are you trying to do? Are you hoping for the `shared_ptr` to take ownership of `ca`? – Tas Apr 12 '18 at 23:37
  • @Tas you are wrong, `ca` is not an instance of `classA` – Slava Apr 12 '18 at 23:37
  • @Slava While it's true that `classA ca();` forward declares a function called `ca` that returns a `classA`, I think that we all know what the OP really did mean. Sure, they didn't post actual code, but it's still clear enough – Justin Apr 12 '18 at 23:38
  • Very good point though Slava, even after all these years most vexing parse still gets me evey. single. time. That said Justin is right also, I was just _assuming_ OP was making a proper `classA` otherwise I'm not sure how they'd come across this error (likely they'd have had a different one) – Tas Apr 12 '18 at 23:49

2 Answers2

6

You're calling the copy constructor trying to make the shared pointer.

std::make_shared<classA>(ca)
                         ^^ constructs the classA using these parameters

You can call make_shared<classA>() to create a shared pointer to a default constructed classA. Or chose another constructor.

kmdreko
  • 42,554
  • 6
  • 57
  • 106
  • If I had a constructor with parameters, let's say `classA(int n)`, how would I "choose" this constructor over the default one when creating the shared pointer? – Eutherpy Apr 12 '18 at 23:57
  • Simple pass the desired constructor parameters directly to `std::make_shared()`, eg: `std::make_shared(12345)`. Just like you did when passing `ca` to std::make_shared()` to call the copy constructor. – Remy Lebeau Apr 13 '18 at 00:02
  • you would pass it to `make_shared` like `std::make_shared(someInt)` – kmdreko Apr 13 '18 at 00:02
  • I tried `auto x = make_shared(some_integer_value)`, and then `classB cb(x);`, but that gives me another error: classA (classA &&)': cannot convert argument 1 from 'std::shared_ptr' to 'const classA &' – Eutherpy Apr 13 '18 at 00:03
  • The error "*cannot convert argument 1 from 'std::shared_ptr' to 'const classA &'*" means that `classB`'s constructor is expecting a `const classA &` as input, not a `std::shared_ptr`. Change `classB` to take a `std::shared_ptr` (so the `classA` can't be destroyed if `x` goes out of scope while `classB` is still using the `classA`), or else call `std::shared_ptr::operator*`, eg: `classB cb(*x);` and make sure `x` doesn't go out of scope before `cb` – Remy Lebeau Apr 13 '18 at 00:04
  • consider making another question because that doesn't seem to match what you've posted above – kmdreko Apr 13 '18 at 00:05
4

This example can be simplified quite a bit.

#include <memory>

class classA {
 public:
  classA(const classA&) = delete;
  classA() = default;
};


int main() {
  classA ca; // note, no ()
  auto sp = std::make_shared<classA>(ca); // problem.
  auto sp2 = std::make_shared<classA>();  // this does what you want
}

You are passing ca as an argument to std::make_shared, which constructs a classA by calling classA::classA with whatever arguments that you passed to make_shared.

This might be more obvious if you consider how a make_shared might be implemented.

template <typename Cls, typename... Args>
std::shared_ptr<Cls> MakeShared(Args&&... args) {
                                     //this is where the copying happens
  return std::shared_ptr<Cls>{new Cls{std::forward<Args>(args)...}};
}

int main() {
  classA ca;
  auto sp = MakeShared<classA>(ca);
  auto sp2 = MakeShared<classA>();
}

You pass ca to MakeShared, which then calls new Cls(...) where the ... is whatever you passed to MakeShared, in this case, another classA object.

If the above is too dense (maybe you aren't used to forward or variadic templates), then consider this simplified version of MakeShared which does the same thing for your problem case

template <typename Cls, typename Arg>
std::shared_ptr<Cls> MakeShared(const Arg& arg) {
                      // copy here ----+
                      //               v
  return std::shared_ptr<Cls>{new Cls{arg}};
}

int main() {
  classA ca;
  auto sp = MakeShared<classA>(ca);
}
Ryan Haining
  • 35,360
  • 15
  • 114
  • 174