PSA: transitioning from scoped_ptr to std::unique_ptr

[Daniel Cheng]

We’re removing scoped_ptr from Chromium. scoped_ptr is already a typedef for std::unique_ptr, and now we're going to mass-replace scoped_ptr with std::unique_ptr. This will take place over the next few weeks. New code should:

• Use std::unique_ptr instead of scoped_ptr: they work exactly the same
  
• #include <utility> instead of "base/memory/scoped_ptr.h"
  
• Use base::WrapUnique() instead of make_scoped_ptr(), which is in base/memory/ptr_util.h.
  

FAQ

Q: Are there any gotchas with std::unique_ptr?

A: std::unique_ptr behaves exactly the same as scoped_ptr, but one thing to be careful of is accessing a std::unique_ptr (e.g. testing if it’s null) while it is being destroyed: the behavior is undefined [1], and different implementations will do different things.

Q: Will std::unique_ptr work with base::Bind and base::Callback?

A: Yes, std::unique_ptr can be bound to a callback and passed the same way it is today.

    std::unique_ptr<int> x(new int(6));

    base::Closure cb = base::Bind(&TakesInt, base::Passed(&x));

Q: Why is make_scoped_ptr’s replacement called base::WrapUnique() instead of base::MakeUnique()?

A: C++14 adds a std::make_unique() with different semantics [2]. To avoid confusion, we’ve renamed it to base::WrapUnique() to reflect its functionality of wrapping a raw pointer in a smart pointer.

- Daniel, with thanks to danakj, amistry, and everyone who helped make this possible!

[1] https://cplusplus.github.io/LWG/lwg-defects.html#2224

[2] http://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique

[Sylvain Defresne]

On Thu, Mar 31, 2016 at 6:57 PM, Daniel Cheng <dch...@chromium.org> wrote:

We’re removing scoped_ptr from Chromium. scoped_ptr is already a typedef for std::unique_ptr, and now we're going to mass-replace scoped_ptr with std::unique_ptr. This will take place over the next few weeks. New code should:

• Use std::unique_ptr instead of scoped_ptr: they work exactly the same
  
• #include <utility> instead of "base/memory/scoped_ptr.h"
  

Shouldn't this be <memory> instead of <utility>?

http://en.cppreference.com/w/cpp/memory/unique_ptr

 

• Use base::WrapUnique() instead of make_scoped_ptr(), which is in base/memory/ptr_util.h.
  

FAQ

Q: Are there any gotchas with std::unique_ptr?

A: std::unique_ptr behaves exactly the same as scoped_ptr, but one thing to be careful of is accessing a std::unique_ptr (e.g. testing if it’s null) while it is being destroyed: the behavior is undefined [1], and different implementations will do different things.

Q: Will std::unique_ptr work with base::Bind and base::Callback?

A: Yes, std::unique_ptr can be bound to a callback and passed the same way it is today.

    std::unique_ptr<int> x(new int(6));

    base::Closure cb = base::Bind(&TakesInt, base::Passed(&x));

Q: Why is make_scoped_ptr’s replacement called base::WrapUnique() instead of base::MakeUnique()?

A: C++14 adds a std::make_unique() with different semantics [2]. To avoid confusion, we’ve renamed it to base::WrapUnique() to reflect its functionality of wrapping a raw pointer in a smart pointer.

- Daniel, with thanks to danakj, amistry, and everyone who helped make this possible!

[1] https://cplusplus.github.io/LWG/lwg-defects.html#2224

[2] http://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

-- Sylvain

[Dana Jansens]

On Thu, Mar 31, 2016 at 4:39 PM, Sylvain Defresne <sdef...@chromium.org> wrote:

On Thu, Mar 31, 2016 at 6:57 PM, Daniel Cheng <dch...@chromium.org> wrote:

We’re removing scoped_ptr from Chromium. scoped_ptr is already a typedef for std::unique_ptr, and now we're going to mass-replace scoped_ptr with std::unique_ptr. This will take place over the next few weeks. New code should:

• Use std::unique_ptr instead of scoped_ptr: they work exactly the same
  
• #include <utility> instead of "base/memory/scoped_ptr.h"
  

Shouldn't this be <memory> instead of <utility>?

http://en.cppreference.com/w/cpp/memory/unique_ptr

Oops, that's my fault. Too much std::move() on my brain. You're right, <memory>.

[Daniel Cheng]

Yes, you're right, that should be #include <memory>. I guess I should have proofread my draft one more time.

Daniel

[Peter Kasting]

On Thu, Mar 31, 2016 at 3:57 PM, Daniel Cheng <dch...@chromium.org> wrote:

Q: Why is make_scoped_ptr’s replacement called base::WrapUnique() instead of base::MakeUnique()?

A: C++14 adds a std::make_unique() with different semantics [2]. To avoid confusion, we’ve renamed it to base::WrapUnique() to reflect its functionality of wrapping a raw pointer in a smart pointer.

Am I right in thinking the current plan here is to introduce a MakeUnique() that works like std::make_unique(), encourage people to use that, gradually switch away from WrapUnique(), and when we move to C++14 rewrite MakeUnique() as std::make_unique()?

PK

[Dana Jansens]

That's proposed here: https://codereview.chromium.org/1532433002/

 

PK

[Matt Giuca]

Wow, I didn't know scoped_ptr had been replaced with a typedef to unique_ptr already. This is like when your parents put healthy food in your normal meals and then after you eat it they say "see! We put healthy food in there and you didn't even notice!" Well done, team.

On a more serious note, the mass-replacement of scoped_ptr with std::unique_ptr could be the first candidate for the git hyper-blame ignore list (.git-blame-ignore-revs), so when you run git hyper-blame on some Chromium code you won't be told about the mass-replacement CL.

Matt

[Daniel Cheng]

On Thu, Mar 31, 2016 at 10:32 PM Matt Giuca <mgi...@chromium.org> wrote:

Wow, I didn't know scoped_ptr had been replaced with a typedef to unique_ptr already. This is like when your parents put healthy food in your normal meals and then after you eat it they say "see! We put healthy food in there and you didn't even notice!" Well done, team.

On a more serious note, the mass-replacement of scoped_ptr with std::unique_ptr could be the first candidate for the git hyper-blame ignore list (.git-blame-ignore-revs), so when you run git hyper-blame on some Chromium code you won't be told about the mass-replacement CL.

Matt

Out of curiosity, how well does hyper-blame scale with the number of commits to ignore? There's some scripts to automate the conversion, but they aren't perfect, so it's still better to break the work into smaller commits.

Also, if anyone wants to help out with the migration, I've started a spreadsheet to track things:

https://docs.google.com/spreadsheets/d/19zyXGmZzdn9muv8ldpfOrgPaiHsxL4OHyGcnKTxcVq4/edit#gid=0. Instructions for automating the conversion are in the second sheet.

For now, I've just dumped the top-level source directories into the spreadsheet, but the larger ones (e.g. //chrome and //content) should have subdirectories split out and done incrementally.

Daniel

[Matt Giuca]

On Sat, 2 Apr 2016 at 04:21 Daniel Cheng <dch...@chromium.org> wrote:

On Thu, Mar 31, 2016 at 10:32 PM Matt Giuca <mgi...@chromium.org> wrote:

Wow, I didn't know scoped_ptr had been replaced with a typedef to unique_ptr already. This is like when your parents put healthy food in your normal meals and then after you eat it they say "see! We put healthy food in there and you didn't even notice!" Well done, team.

On a more serious note, the mass-replacement of scoped_ptr with std::unique_ptr could be the first candidate for the git hyper-blame ignore list (.git-blame-ignore-revs), so when you run git hyper-blame on some Chromium code you won't be told about the mass-replacement CL.

Matt

Out of curiosity, how well does hyper-blame scale with the number of commits to ignore? There's some scripts to automate the conversion, but they aren't perfect, so it's still better to break the work into smaller commits.

At the moment, it is linear in the number of ignored commits that affect the file in question. (It runs git blame, then if it sees lines blamed on an ignored commit, it runs blame again, repeat until no more lines are blamed on ignored commits.) So if one file repeatedly has refactors done, it could end up being a problem. But I figured most large refactors would be broken up by file, so would contribute a max of one extra blame operation to each file. If this turns out not to be the case, we have some ideas about speeding it up.

Don't change your workflow to suit hyper-blame. If there becomes a performance problem I'll try to address it.