When I run this code:
#include <memory>
#include <iostream>
class A {
public:
virtual std::unique_ptr<A> clone() = 0;
};
class B : public A {
private:
int b0;
public:
B(const B& b) { b0 = b.get_b0(); }
B(const int& b0) : b0(b0) {}
std:: unique_ptr<A> clone() { return std::make_unique<B>(*this); }
const int& get_b0() const { return b0; }
int& get_b0() { return b0; }
};
class box {
private:
std::unique_ptr<A> o;
public:
box(const box& sp) { o = sp.o->clone(); }
box(std::unique_ptr<A> o) : o(std::move(o)) {}
std::unique_ptr<A> get_o() { return o->clone(); }
};
void foo(std::unique_ptr<box> sp) {
std::unique_ptr<A> o = sp->get_o();
if(const B *ptr1 = dynamic_cast<const B*>(o.get()))
std::cout << ptr1->get_b0() << std::endl;
if(const B *ptr = dynamic_cast<const B*>(sp->get_o().get()))
std::cout << ptr->get_b0() << std::endl;
}
int main() {
B b(100);
box sp(std::make_unique<B>(b));
foo(std::make_unique<box>(sp));
return 0;
}
The output is:
100
3579964222 (some random integer!!!)
If I ditch std::unique_ptr
, and replace it with std::shared_ptr
, the code starts working. But in my use case, shared_ptr
may lead to problems, as changing one object might affect others.
I like to know how I can fix this code? And if the way I am using clone()
is good code practice or not?
It's undefined behavior, ptr
is a dangling pointer.
The expression sp->get_o()
produces a temporary object of std::unique_ptr<A>
.
The C++ standard says in class.temporary#4 that:
Temporary objects are destroyed as the last step in evaluating the full-expression that (lexically) contains the point where they were created.
And "full-expression" is defined in intro.execution#12.3 as:
A full-expression is
- ...
- an init-declarator or a mem-initializer, including the constituent expressions of the initializer,
- ...
Hence, the full-expression here is dynamic_cast<const B*>(sp->get_o().get()))
, after which is evaluated, the temporary object std::unique_ptr<A>
returned by sp->get_o()
will be destroyed, and the object it points will be deleted in its destructor. So ptr
becomes a dangling pointer.
To fix it, just extend the lifetime of the object returned by sp->get_o()
:
std::unique_ptr<A> o1 = sp->get_o();
if (const B* ptr = dynamic_cast<const B*>(o1.get())) {
std::cout << ptr->get_b0() << std::endl;
}