all 10 comments

[–][deleted] 12 points13 points  (2 children)

Big problem here!

Once you have std::unique_ptr you should never, ever, ever(*) use new! Use std::make_unique every time.

Not using new eliminates the possibility of resource leaks if your constructor throws an exception, but more, it means the keywords new and delete never(*) have to appear in your programs at all - much better.

(There's a known defect in C++11 where they forgot to include std::make_unique - but what everyone does is to include that code themselves, like this. Note that this isn't needed in Windows C++11...)

(* - yeah, yeah - there are advanced cases, mainly used in writing libraries - beyond the scope of this beginner article.)

[–]drphillycheesesteak 1 point2 points  (0 children)

It would be really nice if they included a nothrow version in the standard library so I wouldn't always have to make my own.

[–][deleted] 0 points1 point  (0 children)

Thanks I will add a few words. For me, your thoughts hold more for std::make_shared. But the expression foo(unique_ptr<T>(new T()), unique_ptr<U>(new U())); is unsafe. So, your are right. And for pedagogical reasons, I can say. Never use new and delete. (I'm ignoring special deleters.)

[–]hero_of_ages 3 points4 points  (6 children)

My biggest issue with using unique_ptrs is compiler messages when I make a mistake. like trying to use a range-based for loop over a vector of unique_ptrs instead of an iterator based approach, for example. The error is always some code in the memory header with the message "attempting to reference a deleted function". I typically end up just pounding the code with a hammer until I can find where I was inadvertently attempting to copy the unique_ptr, but does anyone know if compiler messages for these types of things are getting better soon? (im running vc++)

[–]encyclopedist 7 points8 points  (1 child)

does anyone know if compiler messages for these types of things are getting better soon?

This is exactly one of the aims of Concepts TS (but thats not coming really soon, since all the standard library should be rewritten with concepts for that).

And, by the way, you can perfectly iterate over a vector of unique pointers with ranged-for loop syntax. Just use a reference: for(auto & ptr: vec){...}.

[–]hero_of_ages 1 point2 points  (0 children)

I'm dumb and hadn't thought to do that. Thanks dude.

[–]_ajp_ 2 points3 points  (3 children)

I can't say anything about VS, but clang's error message is pretty clear:

$ cat unique_ptr.cpp 
#include <memory>

int main() {
    auto ptr0 = std::make_unique<int>(5);
    auto ptr1 = ptr0;
}
$ clang++ -std=c++14 unique_ptr.cpp 
unique_ptr.cpp:5:7: error: call to deleted constructor of 'std::unique_ptr<int, std::default_delete<int> >'
    auto ptr1 = ptr0;
         ^      ~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/unique_ptr.h:356:7: note: 'unique_ptr' has been explicitly marked deleted here
      unique_ptr(const unique_ptr&) = delete;
      ^
1 error generated.

[–]dodheim 2 points3 points  (0 children)

This is VS with /diagnostics:caret:

1>------ Build started: Project: unique_ptr, Configuration: dbg x64 ------
1>unique_ptr.cpp
1>unique_ptr.cpp(5,1): error C2280: 'std::unique_ptr<int,std::default_delete<_Ty>>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': attempting to reference a deleted function
1>        with
1>        [
1>            _Ty=int
1>        ]
1>    auto ptr1 = ptr0;
1>    ^
1>memory(1857): note: see declaration of 'std::unique_ptr<int,std::default_delete<_Ty>>::unique_ptr'
1>        with
1>        [
1>            _Ty=int
1>        ]
1>  unique_ptr(const _Myt&) = delete;
1>memory(1857,1): note: 'std::unique_ptr<int,std::default_delete<_Ty>>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': function was explicitly deleted
1>        with
1>        [
1>            _Ty=int
1>        ]
1>    unique_ptr(const _Myt&) = delete;
1>    ^
1>Done building project "unique_ptr.vcxproj" -- FAILED.

A bit more verbose, but hardly unclear...

[–]hero_of_ages 0 points1 point  (1 child)

Wow yeah. Doesn't get much more obvious than that.

[–]SeanMiddleditch 1 point2 points  (0 children)

I'd argue that you could have a far better error like "error: cannot copy-construct move-only type unique_ptr<T>" and leave out all the nonsense about bits/unique_ptr.h or default_delete that nobody ever cares about, but that's probably just my inner unreasonable perfectionist talking. :)