Bug 115102

Summary: Use std::move in AtomicString
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: Web Template FrameworkAssignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, benjamin, cmarcelo, commit-queue, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
patch
benjamin: review-
patch v3
andersca: review+
to be landed none

Mikhail Pozdnyakov
Reported 2013-04-24 05:21:46 PDT
SSIA.
Attachments
WIP (4.29 KB, patch)
2013-04-24 05:54 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
patch (3.10 KB, patch)
2013-04-24 07:53 PDT, Mikhail Pozdnyakov 		benjamin: review-
DetailsFormatted DiffDiff
patch v3 (2.40 KB, patch)
2013-04-25 01:28 PDT, Mikhail Pozdnyakov 		andersca: review+
DetailsFormatted DiffDiff
to be landed (2.35 KB, patch)
2013-04-25 07:41 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Mikhail Pozdnyakov
Comment 1 2013-04-24 05:54:52 PDT
Created attachment 199418 [details] WIP Newly added file is not yet included into proj files.
Anders Carlsson
Comment 2 2013-04-24 07:23:23 PDT
I like it, but I think it should just go into TypeTraits.h directly.
Mikhail Pozdnyakov
Comment 3 2013-04-24 07:53:28 PDT
Created attachment 199454 [details] patch Put 'move()' to TypeTraits.h as requested by Anders.
Alexey Proskuryakov
Comment 4 2013-04-24 08:55:13 PDT
What is the difference with std::move? I'm wondering if we can just use it directly.
Benjamin Poulain
Comment 5 2013-04-24 14:36:31 PDT
(In reply to comment #4) > What is the difference with std::move? I'm wondering if we can just use it directly. Same question here.
Anders Carlsson
Comment 6 2013-04-24 14:42:35 PDT
Good point. We could just state that any C++ compiles that supports rvalue references must also have an implementation of std::move.
Benjamin Poulain
Comment 7 2013-04-24 14:44:20 PDT
Comment on attachment 199454 [details] patch Looks like we should use std::move in AtomicString instead of introducing our own.
Mikhail Pozdnyakov
Comment 8 2013-04-25 00:07:12 PDT
(In reply to comment #5) > (In reply to comment #4) > > What is the difference with std::move? I'm wondering if we can just use it directly. > > Same question here. Ok, I was just thinking that one of the reasons for having WTF is that we do not want to use STL, this assumption seems to be wrong however.
Benjamin Poulain
Comment 9 2013-04-25 00:25:55 PDT
> Ok, I was just thinking that one of the reasons for having WTF is that we do not want to use STL, this assumption seems to be wrong however. The STL is pretty crappy when it come to ADT. Apart from that, I don't think there is any problem with using STL.
Mikhail Pozdnyakov
Comment 10 2013-04-25 01:28:10 PDT
Created attachment 199631 [details] patch v3 Use std::move.
Mikhail Pozdnyakov
Comment 11 2013-04-25 04:32:59 PDT
(In reply to comment #9) > > Ok, I was just thinking that one of the reasons for having WTF is that we do not want to use STL, this assumption seems to be wrong however. > > The STL is pretty crappy when it come to ADT. Apart from that, I don't think there is any problem with using STL. btw, why do we use WTF TypeTraits? could we use std::type_traits as well?
Anders Carlsson
Comment 12 2013-04-25 07:21:58 PDT
Comment on attachment 199631 [details] patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=199631&action=review > Source/WTF/wtf/text/AtomicString.h:26 > +#if COMPILER_SUPPORTS(CXX_RVALUE_REFERENCES) > +#include <utility> > +#endif I think we can just always include this.
Mikhail Pozdnyakov
Comment 13 2013-04-25 07:41:02 PDT
Created attachment 199660 [details] to be landed
Anders Carlsson
Comment 14 2013-04-25 08:04:09 PDT
(In reply to comment #11) > (In reply to comment #9) > > > Ok, I was just thinking that one of the reasons for having WTF is that we do not want to use STL, this assumption seems to be wrong however. > > > > The STL is pretty crappy when it come to ADT. Apart from that, I don't think there is any problem with using STL. I think using STL for type traits is a great idea! > > btw, why do we use WTF TypeTraits? could we use std::type_traits as well? I believe that the STL type traits are C++11 only (but we could change the WTF type traits to emulate them when building with C++98).
WebKit Commit Bot
Comment 15 2013-04-25 08:15:33 PDT
The commit-queue encountered the following flaky tests while processing attachment 199660 [details]: svg/as-image/img-relative-height.html bug 114140 (author: zimmermann@kde.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 16 2013-04-25 08:16:54 PDT
Comment on attachment 199660 [details] to be landed Clearing flags on attachment: 199660 Committed r149112: <http://trac.webkit.org/changeset/149112>
WebKit Commit Bot
Comment 17 2013-04-25 08:16:56 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 18 2013-04-25 09:46:53 PDT
> Ok, I was just thinking that one of the reasons for having WTF is that we do not want to use STL, this assumption seems to be wrong however. The main reasons for reimplementing some containers and algorithms are: 1. We can fine tune performance to our needs. 2. We can disable exceptions and still have reasonable behavior.
Note You need to log in before you can comment on or make changes to this bug.