Bug 177789 - WeakPtr should have a move constructor
Summary: WeakPtr should have a move constructor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-02 15:26 PDT by Geoffrey Garen
Modified: 2017-10-02 17:43 PDT (History)
9 users (show)

See Also:

Attachments
Patch (1.34 KB, patch)
2017-10-02 15:27 PDT, Geoffrey Garen 		zalan: review+
ggaren: commit-queue+
Details | Formatted Diff | Diff
Patch (1.40 KB, patch)
2017-10-02 15:59 PDT, Geoffrey Garen 		no flags Details | Formatted Diff | Diff
Patch (2.69 KB, patch)
2017-10-02 16:54 PDT, Geoffrey Garen 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2017-10-02 15:26:48 PDT 
WeakPtr should have a move constructor
Comment 1 Geoffrey Garen 2017-10-02 15:27:37 PDT 
Created attachment 322457 [details]
Patch
Comment 2 zalan 2017-10-02 15:43:19 PDT 
Comment on attachment 322457 [details]
Patch

ofc
Comment 3 Chris Dumez 2017-10-02 15:52:39 PDT 
Comment on attachment 322457 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=322457&action=review

> Source/WTF/wtf/WeakPtr.h:77
> +    WeakPtr& operator=(const WeakPtr&) = default;

No WeakPtr& operator=(const WeakPtr&&) = default; ?
Comment 4 Chris Dumez 2017-10-02 15:53:21 PDT 
Comment on attachment 322457 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=322457&action=review

> Source/WTF/wtf/WeakPtr.h:70
>      WeakPtr(const WeakPtr& o) : m_ref(o.m_ref) { }

Can this one be = default; as well?
Comment 5 Geoffrey Garen 2017-10-02 15:59:42 PDT 
Created attachment 322464 [details]
Patch
Comment 6 Chris Dumez 2017-10-02 16:02:25 PDT 
Comment on attachment 322464 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=322464&action=review

I think that if we just drop the custom copy constructor / assignment operator then we'd get implicit move constructor / assignment operators, no?

> Source/WTF/wtf/WeakPtr.h:77
> +    WeakPtr& operator=(const WeakPtr&) = default;

Why don't we want the WeakPtr& operator=(const WeakPtr&&) = default; too?
Comment 7 Chris Dumez 2017-10-02 16:03:27 PDT 
Comment on attachment 322464 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=322464&action=review

> Source/WTF/ChangeLog:3
> +        WeakPtr should have a move constructor

It would be good to API-test this change.
Comment 8 Geoffrey Garen 2017-10-02 16:54:48 PDT 
Created attachment 322473 [details]
Patch
Comment 9 Chris Dumez 2017-10-02 16:57:37 PDT 
Comment on attachment 322473 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2017-10-02 17:42:09 PDT 
Comment on attachment 322473 [details]
Patch

Clearing flags on attachment: 322473

Committed r222764: <http://trac.webkit.org/changeset/222764>
Comment 11 WebKit Commit Bot 2017-10-02 17:42:10 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2017-10-02 17:43:18 PDT 
<rdar://problem/34781637>