57

In a piece of code I reviewed lately, which compiled fine with g++-4.6, I encountered a strange try to create a std::shared_ptr from std::unique_ptr:

std::unique_ptr<Foo> foo...
std::make_shared<Foo>(std::move(foo));

This seems rather odd to me. This should be std::shared_ptr<Foo>(std::move(foo)); afaik, though I'm not perfectly familiar with moves (and I know std::move is only a cast, nothing get's moved).

Checking with different compilers on this SSC(NUC*)E

#include <memory>

int main()
{
   std::unique_ptr<int> foo(new int);
   std::make_shared<int>(std::move(foo));
}

Results of compilation:

  • g++-4.4.7 gives compilation error
  • g++-4.6.4 compiles without any error
  • g++-4.7.3 gives internal compiler error
  • g++-4.8.1 gives compilation error
  • clang++-3.2.1 compiles without any error

So the question is: which compiler is right in terms of the standard? Does the standard require this to be an invalid statement, a valid statement or is this simply undefined?

Addition

We've agreed on that some of these compilers, such as clang++ and g++-4.6.4, permit the conversion while they shouldn't. However with g++-4.7.3 (which produces an internal compiler error on std::make_shared<Foo>(std::move(foo));), correctly rejects int bar(std::move(foo));

Because of this huge difference in behavior, I'm leaving the question as it is, although part of it would be answerable with the reduction to int bar(std::move(foo));.


*) NUC: Not universally compilable

stefan
  • 10,215
  • 4
  • 49
  • 90
  • 14
    Unless `Foo` has a constructor that takes a `std::unique_ptr` it shouldn't compile. – Simple Sep 19 '13 at 08:37
  • 1
    it's confusing saying "compiles with correct error" when you mean "doesn't compile". And the question title is confusing, you're not creating a shared_ptr from a unique_ptr, you're creating an `int` from a unique_ptr. – Jonathan Wakely Sep 19 '13 at 17:34
  • @JonathanWakely I adjusted the formulation. However I will leave the title as it is. Why? The intent of the developer was to create a `std::shared_ptr` that way. It is not the point of this question to create an `int` from a `unique_ptr`, but why different compilers allow this statement or fail miserably. – stefan Sep 19 '13 at 20:05
  • just for a reference I checked whether `unique_ptr` is implicitly convertible to a normal pointer (but it isn't) – Alexander Oh Sep 19 '13 at 20:13
  • @Alex and it shouldn't be. This would break the concept of `*unique*_ptr` – stefan Sep 19 '13 at 20:15
  • Why don't you simply use release: `std::unique_ptr foo(new int); std::shared_ptr shptr(foo.release());`? (It is not compiler bug.) – Naszta Sep 19 '13 at 20:28
  • @Naszta Yes, that was my first reading of the question. The point is, the line `std::make_shared(std::move(foo));` should not compile. That is the real question whether the standard allows it. In my opinion, it is a compiler bug. – Ali Sep 19 '13 at 20:52
  • @Naszta As I've said, it wasn't _my_ code and the intent was to make a pointer shared from unique. The correct way is via the shared_ptr ctor which takes a unique_ptr. The incorrect way shown here produces a variety of different behaviour in compilers, some of which are wrong. – stefan Sep 20 '13 at 07:13
  • 1
    @Naszta Do not write `std::shared_ptr shptr(foo.release());`! This is dangerous code that can cause a memory leak. If `shared_ptr` fails to allocate its reference block and throws an exception you now have leaked memory. – Simple Sep 20 '13 at 07:19
  • @Simple this will not be a memory leak. the cpp reference says that in case of bad_alloc exception the delete ptr will be called – user179156 Sep 09 '15 at 21:24
  • Does this answer your question? [Does C++11 unique\_ptr and shared\_ptr able to convert to each other's type?](https://stackoverflow.com/questions/37884728/does-c11-unique-ptr-and-shared-ptr-able-to-convert-to-each-others-type) – Ida Sep 17 '20 at 09:40
  • I got my answer seven years ago, thanks ;-) – stefan Sep 17 '20 at 15:02

4 Answers4

34

UPDATE 2: This bug has been fixed in Clang in r191150. GCC rejects the code with a proper error message.


UPDATE: I have submitted a bug report. The following code on my machine with clang++ 3.4 (trunk 191037)

#include <iostream>
#include <memory>

int main()
{
   std::unique_ptr<int> u_ptr(new int(42));

   std::cout << " u_ptr.get() = " <<  u_ptr.get() << std::endl;
   std::cout << "*u_ptr       = " << *u_ptr       << std::endl;

   auto s_ptr = std::make_shared<int>(std::move(u_ptr));

   std::cout << "After move" << std::endl;

   std::cout << " u_ptr.get() = " <<  u_ptr.get() << std::endl;
   std::cout << "*u_ptr       = " << *u_ptr       << std::endl;
   std::cout << " s_ptr.get() = " <<  s_ptr.get() << std::endl;
   std::cout << "*s_ptr       = " << *s_ptr       << std::endl;
}

prints this:

 u_ptr.get() = 0x16fa010
*u_ptr       = 42
After move
 u_ptr.get() = 0x16fa010
*u_ptr       = 42
 s_ptr.get() = 0x16fa048
*s_ptr       = 1

As you can see, the unique_ptr hasn't been moved from. The standard guarantees that it should be null after it has been moved from. The shared_ptr points to a wrong value.

The weird thing is that it compiles without a warning and valgrind doesn't report any issues, no leak, no heap corruption. Weird.

The proper behavior is shown if I create s_ptr with the shared_ptr ctor taking an rvalue ref to a unique_ptr instead of make_shared:

#include <iostream>
#include <memory>

int main()
{
   std::unique_ptr<int> u_ptr(new int(42));

   std::cout << " u_ptr.get() = " <<  u_ptr.get() << std::endl;
   std::cout << "*u_ptr       = " << *u_ptr       << std::endl;

   std::shared_ptr<int> s_ptr{std::move(u_ptr)};

   std::cout << "After move" << std::endl;

   std::cout << " u_ptr.get() = " <<  u_ptr.get() << std::endl;
   //std::cout << "*u_ptr       = " << *u_ptr       << std::endl; // <-- would give a segfault
   std::cout << " s_ptr.get() = " <<  s_ptr.get() << std::endl;
   std::cout << "*s_ptr       = " << *s_ptr       << std::endl;
}

It prints:

 u_ptr.get() = 0x5a06040
*u_ptr       = 42
After move
 u_ptr.get() = 0
 s_ptr.get() = 0x5a06040
*s_ptr       = 42

As you see, u_ptr is null after the move as required by the standard and s_ptr points to the correct value. This is the correct behavior.


(The original answer.)

As Simple has pointed out: "Unless Foo has a constructor that takes a std::unique_ptr it shouldn't compile."

To expand on it a little bit: make_shared forwards its arguments to T's constructor. If T doesn't have any ctor that could accept that unique_ptr<T>&& it is a compile error.

However, it is easy to fix this code and get what you want (online demo):

#include <memory>
using namespace std;

class widget { };

int main() {

    unique_ptr<widget> uptr{new widget};

    shared_ptr<widget> sptr(std::move(uptr));
}

The point is: make_shared is the wrong thing to use in this situation. shared_ptr has a ctor that accepts an unique_ptr<Y,Deleter>&&, see (13) at the ctors of shared_ptr.

Community
  • 1
  • 1
Ali
  • 56,466
  • 29
  • 168
  • 265
  • @stefan Sorry, I misunderstood your question. It took me 5 more minutes to understand the real problem here. Please check my updated answer. I will get back to you later today with the results obtained with the latest clang. – Ali Sep 19 '13 at 17:29
  • It's a very good answer now exploring the misbehaviour of clang. Thank you for that! – stefan Sep 19 '13 at 20:06
  • 1
    It turns out that `g++-4.6.4` permits the intermediate use of `operator bool()` to first convert from `unique_ptr` to `bool` to `int`. Probably, clang++ does the same. – stefan Sep 19 '13 at 20:26
  • @stefan Thanks for the info. Please give some time, I will check the latest clang. In my opinion, the line `std::make_shared(std::move(foo));` should not compile. If it does, it is a bug. By the way, where did you find this info? – Ali Sep 19 '13 at 20:54
  • @stefan By the way, +1 for the question, a very weird compiler bug. – Ali Sep 19 '13 at 22:30
  • I've just inserted a debug statement into the `operator bool()`, compiled and executed. The debug statement appeared, hence it was used. It's as simple as that. ;-) – stefan Sep 20 '13 at 07:10
  • Oh, I see. :) If you check my bug report, it has been confirmed by Richard Smith that it is a bug. Also, Jonathan Wakely is another prominent expert and compiler developer in the field. So, it *is* a bug. – Ali Sep 20 '13 at 07:51
  • I'm experiencing this with MSVC 12 (Visual Studio 2013) and the most suprising thing is that the code works! I mistakenly wrote code like this, but the content of the shared_ptr was fine. I don't understand it at all. – AzP Jan 29 '16 at 11:53
  • @AzP Yeah, early compiler versions had weird bugs in this situation. It is not worth wasting your time tracking down these bugs; most likely a newer version of the compiler does not suffer from this. – Ali Jan 29 '16 at 12:15
  • @Ali Yeah, for me it was actually an error, so I fixed my code instead. – AzP Jan 29 '16 at 12:51
  • I am trying to do something similar: auto const size = 5;auto up1array1 = std::make_unique(size); std::shared_ptr sparray(std::move( up1array1 ));...but it doesn't work. Wonder why!! – nurabha Mar 02 '16 at 09:57
12

This shouldn't compile. If we disregard the uniqueness and sharedness of the pointers for a moment, it's basically trying to do this:

int *u = new int;
int *s = new int(std::move(u));

It means it's dynamically creating an int and initialising it with an rvalue reference to std::unique_ptr<int>. For ints, that simply shouldn't compile.

For a general class Foo, it depends on the class. If it has a constructor taking a std::unique_ptr<Foo> by value, const ref or rvalue ref, it will work (but maybe not do what the author intended). In other cases, it shouldn't compile.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • Right, good answer. Interestingly there is still a difference between the smart pointer version and yours, as all compilers mentioned correctly reject your sample code. – stefan Sep 19 '13 at 13:40
5

Here's a reduced example which clang incorrectly compiles:

struct ptr
{
  int* p;

  explicit operator bool() const { return p != nullptr; }
};

int main()
{
  ptr u{};
  int* p = new int(u);
}

Clang uses the explicit bool conversion operator to initialize the int (and the Intel compiler does too.)

Clang 3.4 does not allow:

int i = int(u);

but it does allow:

int* p = new int(u);

I think both should be rejected. (Clang 3.3 and ICC allow both.)

I've added this example to the bug report.

Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
0

Although counterintuitive it is perfectly allowed:

std::shared_ptr<Type> shp = std::make_unique<Type>( arg );

KeyC0de
  • 4,728
  • 8
  • 44
  • 68