Bug 32363 - [Chromium] Disallow assignment of WebVector<T> to WebVector<T>
Summary: [Chromium] Disallow assignment of WebVector<T> to WebVector<T>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-10 00:28 PST by Yury Semikhatsky
Modified: 2009-12-17 09:07 PST (History)
4 users (show)

See Also:


Attachments
patch (947 bytes, patch)
2009-12-10 00:33 PST, Yury Semikhatsky
fishd: review-
Details | Formatted Diff | Diff
patch addressing reviewers comments (1.03 KB, patch)
2009-12-14 08:06 PST, Yury Semikhatsky
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2009-12-10 00:28:52 PST
As comment in the top part of WebVector.h says WebVector::swap should be used in such cases. If you write by mistake something as follows:

    WebVector<WebString> a(3);
    WebVector<WebString> b;
    b = a;

operator= automatically generated by compiler will be invoked as it takes precedence over template version
 
    template <typename C>
    WebVector<T>& operator=(const C& other)

The default assignment operator will lead to a.m_ptr == b.m_ptr and to destructor being called twice for the container elements. So either operator= should have valid implementation or be disallowed explicitly.
Comment 1 Yury Semikhatsky 2009-12-10 00:33:03 PST
Created attachment 44598 [details]
patch

Not sure where I could place unit test for this.
Comment 2 WebKit Review Bot 2009-12-10 00:36:28 PST
style-queue ran check-webkit-style on attachment 44598 [details] without any errors.
Comment 3 Eric Seidel (no email) 2009-12-10 00:57:29 PST
Comment on attachment 44598 [details]
patch

We have wtf/Noncopyable for this sort of thing.  But that can't be used here, so this is probably fine.
Comment 4 Darin Fisher (:fishd, Google) 2009-12-11 10:12:16 PST
Comment on attachment 44598 [details]
patch

Actually, the intent was to allow copying just as you can copy a std::vector or a WTF::Vector.  I think we should implement this assignment operator.
Comment 5 Yury Semikhatsky 2009-12-14 08:06:39 PST
Created attachment 44797 [details]
patch addressing reviewers comments
Comment 6 WebKit Review Bot 2009-12-14 08:10:44 PST
style-queue ran check-webkit-style on attachment 44797 [details] without any errors.
Comment 7 Yury Semikhatsky 2009-12-14 08:14:21 PST
(In reply to comment #4)
> (From update of attachment 44598 [details])
> Actually, the intent was to allow copying just as you can copy a std::vector or
> a WTF::Vector.  I think we should implement this assignment operator.
Done. I've added "non-template" operator=.
Comment 8 Yury Semikhatsky 2009-12-17 09:07:00 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/public/WebVector.h
Committed r52255