1

I'm trying to pop_back my shared_pointer from my vector and convert to a unique_ptr. Unfortunately, it's giving a strange compilation message.

IFCCB.cpp:

std::unique_ptr<IFC> IFCCCB::getElementVectorIFC()
{
    return (std::unique_ptr<IFC>(make_unique<IFC>(m_shpVectorIFC.pop_back())));
}

IFCCB.h:

public:
unique_ptr<IFC> getElementVectorIFC();

compile error:

error C2784: 'enable_if::value,std::unique_ptr<_Ty,std::default_delete<_Ty>>>::type std::make_unique(_Types &&...)' : could not deduce template argument for '_Types &&' from 'void'

As far as I can tell, I'm doing what I see elsewhere.

I looked at make_unique info but it doesn't give a very good example, and unique_ptr use. Any ideas?

Michele
  • 3,617
  • 12
  • 47
  • 81

4 Answers4

5

You can't transfer ownership from a shared pointer to a unique pointer. Once ownership is shared, it remains shared.

Either store unique pointers if you don't need to share ownership with the container, or continue to use shared pointers after removing from the container if you do.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
4

There are many things wrong here:

return (std::unique_ptr<IFC>(make_unique<IFC>(m_shpVectorIFC.pop_back())));

Let us break it down into multiple lines, C++11 style. This isn't quite equivalent, but close:

auto&& a = m_shpVectorIFC.pop_back();
auto&& b = make_unique<IFC>(std::move(a));
auto&& c = std::unique_ptr<IFC>(std::move(b));
return std::move(c);

which isn't a bad technique to start with.

Now you'll get an error that will be more informative.

The first problem is that pop_back() returns void. Fix that with:

auto a = std::move(m_shpVectorIFC.back()); // note a value, as we are about to mutate the vector
m_shpVectorIFC.pop_back();
auto&& b = make_unique<IFC>(std::move(a));
auto&& c = std::unique_ptr<IFC>(std::move(b));
return std::move(c);

we still run into problems. IFC cannot be constructed from a std::shared_ptr<IFC>&&. It cannot. We can make a copy:

auto a = std::move(m_shpVectorIFC.back());
m_shpVectorIFC.pop_back();
auto&& b = make_unique<IFC>(*a);
auto&& c = std::unique_ptr<IFC>(std::move(b));
return std::move(c);

Next, auto&&c is pointless.

auto a = std::move(m_shpVectorIFC.back());
m_shpVectorIFC.pop_back();
auto&& b = make_unique<IFC>(*a);
return std::move(b);

and we might as well store b as a value:

auto a = std::move(m_shpVectorIFC.back());
m_shpVectorIFC.pop_back();
auto b = make_unique<IFC>(*a);
return b;

then some error detecting:

auto a = std::move(m_shpVectorIFC.back());
m_shpVectorIFC.pop_back();
if (!a)
  return {};
auto b = make_unique<IFC>(*a);
return b;

and micro-optimize:

auto a = std::move(m_shpVectorIFC.back());
m_shpVectorIFC.pop_back();
if (!a)
  return {};
auto b = a.unique()?make_unique<IFC>(std::move(*a)):make_unique<IFC>(*a);
return b;

Now, this doesn't return the shared_ptr data, but rather a copy of it. You cannot move the guts of a C++11 shared_ptr into a unique_ptr. You can do it the other way.

Odds are you should be storing a unique_ptr in your vector anyhow.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • That's a lot of great ideas. I'm thinking of changing it to adding a shared_ptr to the vector of shared_ptrs now. It makes sense. I've just had conflicting requests from the team. Making a copy would get around the problem, but putting the shared_ptr in in the first place would be good too. – Michele Jan 07 '15 at 18:40
  • I'm not sure it's a good idea to return a type of auto, since auto is only locally scoped. – Michele Jan 07 '15 at 19:06
  • So if I made it a vector of unique_ptrs, do you think this is how I would return a unique_ptr? std::unique_ptr IFCCB::getElementVectorIFC() { auto temp; if (m_upVectorIFC.size()) //if there's something in there { temp = std::move(m_upVectorIFC.back()); //access last item m_upVectorIFC.pop_back(); //remove item from vector } if (!temp) return{}; return(make_unique(std::move(*temp)):make_unique(*temp)); //copy item to it's own unique_ptr } – Michele Jan 07 '15 at 19:20
  • I'm leaning toward something like this now. I'm a little confused about when I can/should be making a copy of the unique_ptr. What do you think? std::unique_ptr IFCCB::getElementVectorIFC() { std::unique_ptr temp = std::move(m_upVectorIFC.back()); //access last item m_upVectorIFC.pop_back(); //remove item from vector return(std::unique_ptr(new IFC(temp.get()))); //move local item to it's own unique_ptr for return } – Michele Jan 07 '15 at 19:43
  • 1
    @michele I only made a copy of the guts of the `shared_ptr` because you cannot convert a `shared_ptr` to a `unique_ptr`. If you are storing a `unique_ptr`, do a `if (vec.empty()) return {}; auto retval = std::move( vec.back() ); vec.pop_back(); return retval;`, which returns an empty `unique_ptr` if the vector is empty, or if the last element is an empty `unique_ptr`. That lines up with your second implementation. Btw, backticks let you quote code in comments or in paragraphcs. – Yakk - Adam Nevraumont Jan 07 '15 at 20:21
3

std::vector::pop_back returns void. That's the reason you are getting that specific error, however once you fix that then you will encounter the problem the others mention.

sjdowling
  • 2,994
  • 2
  • 21
  • 31
1

i should think something like this would be preferable (assuming m_shpVectorIFC contains unique_ptr objects):

std::unique_ptr<IFC> IFCCCB::getElementVectorIFC()
{
  auto result = std::move(m_shpVectorIFC.back());
  m_shpVectorIFC.pop_back();
  return result;
}

EDIT: ah, just seen the word shared_ptr in there... nope, you have to return a shared_ptr or store a unique_ptr in the container.

Richard Hodges
  • 68,278
  • 7
  • 90
  • 142