PSA: OwnPtr::clear() will be renamed to reset()

44 views
Skip to first unread message

Yuta Kitamura

unread,
May 26, 2016, 4:01:05 AM5/26/16
to blink-dev
Hi Blink devs,

Yes, yes, you probably remember my name already...

This time I'm going to rename OwnPtr::clear() to reset(), so it matches the name of std::unique_ptr's equivalent. Unless I receive objections, I will make this change sometime tomorrow in Japan time (in 20 hours or so).

This is supposed to be the last update in OwnPtr. The next update will be the complete removal of OwnPtr, which consists of the following changes applied in the whole code base at once:
  • Rename OwnPtr and PassOwnPtr to std::unique_ptr,
  • Rename leakPtr() to release(), and
  • Rename adoptPtr() to wrapUnique().
Due to the nature of the change, I'm planning to do this in some weekend, with the tree closed during the work (~a few hours). If it (unfortunately) turns out the work can't get completed, I will revert the tree back to the original state, and seek for the next possibility.

I will notify blink-dev and chromium-dev when I'm ready to do the bulk update.

Please let me know any concerns and suggestions on this plan.

Q & A

Q: I want to convert OwnPtrs in my module earlier than the mass update; can I do that?

A: Yes, feel free to do that. Please cc me in the CL so I can coordinate with it.

Thanks for your patience and cooperation,
Yuta

Pavel Feldman

unread,
May 26, 2016, 1:47:19 PM5/26/16
to Yuta Kitamura, blink-dev
On Thu, May 26, 2016 at 1:00 AM, Yuta Kitamura <yu...@chromium.org> wrote:
Hi Blink devs,

Yes, yes, you probably remember my name already...

This time I'm going to rename OwnPtr::clear() to reset(), so it matches the name of std::unique_ptr's equivalent. Unless I receive objections, I will make this change sometime tomorrow in Japan time (in 20 hours or so).

This is supposed to be the last update in OwnPtr. The next update will be the complete removal of OwnPtr, which consists of the following changes applied in the whole code base at once:
  • Rename OwnPtr and PassOwnPtr to std::unique_ptr,
  • Rename leakPtr() to release(), and
  • Rename adoptPtr() to wrapUnique().
Should we do something smart here to convert

OwnPtr<Foo> foo = adoptPtr(new Foo());

into

std::unique_ptr<Foo> foo(new Foo());

?

Yuta Kitamura

unread,
May 27, 2016, 1:08:47 AM5/27/16
to Pavel Feldman, blink-dev
On Fri, May 27, 2016 at 2:47 AM, Pavel Feldman <pfel...@chromium.org> wrote:

Should we do something smart here to convert

OwnPtr<Foo> foo = adoptPtr(new Foo());

into

std::unique_ptr<Foo> foo(new Foo());

?

Yes, that seems better. I will try to do that where possible.

Alan Cutter

unread,
May 30, 2016, 1:44:51 AM5/30/16
to Yuta Kitamura, Pavel Feldman, blink-dev
I recommend blocking the removal of OwnPtr on making it a typedef of std::unique_ptr (you'll need to use `using` as it's a templated class).

Yuta Kitamura

unread,
May 30, 2016, 3:21:04 AM5/30/16
to Alan Cutter, blink-dev
On Mon, May 30, 2016 at 2:44 PM, Alan Cutter <alanc...@chromium.org> wrote:
I recommend blocking the removal of OwnPtr on making it a typedef of std::unique_ptr (you'll need to use `using` as it's a templated class).

The biggest concern I have is that MSVC still can't handle template alias gracefully. I saw some internal compiler errors on MSVC 2015 caused by the use of template alias.

Another (smaller) reason of not typedef'ing is that it can be confusing to add OwnPtr::release() which is identical to std::unique_ptr::release(), as OwnPtr::release() had the different semantics previously.

 

Daniel Cheng

unread,
May 30, 2016, 3:35:58 PM5/30/16
to Yuta Kitamura, Alan Cutter, blink-dev
On Mon, May 30, 2016 at 12:21 AM Yuta Kitamura <yu...@chromium.org> wrote:
On Mon, May 30, 2016 at 2:44 PM, Alan Cutter <alanc...@chromium.org> wrote:
I recommend blocking the removal of OwnPtr on making it a typedef of std::unique_ptr (you'll need to use `using` as it's a templated class).

The biggest concern I have is that MSVC still can't handle template alias gracefully. I saw some internal compiler errors on MSVC 2015 caused by the use of template alias.

For simpler templates like OwnPtr, it shouldn't be a problem. That's what we did for scoped_ptr and there were no issues on the bots.
 

Another (smaller) reason of not typedef'ing is that it can be confusing to add OwnPtr::release() which is identical to std::unique_ptr::release(), as OwnPtr::release() had the different semantics previously.

Once using a type alias is possible, it's trivial to do a mass sed to finish up the work. The transition period will be pretty short, and Blink developers are going to have to get used to the different semantics of std::unique_ptr::release() either way.

Daniel 

Yuta Kitamura

unread,
May 30, 2016, 11:31:41 PM5/30/16
to Daniel Cheng, Alan Cutter, blink-dev
On Tue, May 31, 2016 at 4:35 AM, Daniel Cheng <dch...@chromium.org> wrote:
On Mon, May 30, 2016 at 12:21 AM Yuta Kitamura <yu...@chromium.org> wrote:
On Mon, May 30, 2016 at 2:44 PM, Alan Cutter <alanc...@chromium.org> wrote:
I recommend blocking the removal of OwnPtr on making it a typedef of std::unique_ptr (you'll need to use `using` as it's a templated class).

The biggest concern I have is that MSVC still can't handle template alias gracefully. I saw some internal compiler errors on MSVC 2015 caused by the use of template alias.

For simpler templates like OwnPtr, it shouldn't be a problem. That's what we did for scoped_ptr and there were no issues on the bots.

Huh, really? When I made PassOwnPtr an alias of OwnPtr, that didn't work on MSVC due to internal compiler errors. I assume that'd be pretty similar to aliasing OwnPtr with std::unique_ptr.

Maybe 2013 vs 2015 difference? I know both 2013 and 2015 fail with template alias, but they fail differently.

 

Another (smaller) reason of not typedef'ing is that it can be confusing to add OwnPtr::release() which is identical to std::unique_ptr::release(), as OwnPtr::release() had the different semantics previously.

Once using a type alias is possible, it's trivial to do a mass sed to finish up the work. The transition period will be pretty short, and Blink developers are going to have to get used to the different semantics of std::unique_ptr::release() either way.

Unfortunately the transition work won't be as simple as a mass sed. We'll need to adjust #includes, or do smarter replacement for cases Pavel has pointed out, so preparing for that change will take several days.
 
Reply all
Reply to author
Forward
0 new messages