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

Description Mikhail Pozdnyakov 2013-04-24 05:21:46 PDT
SSIA.
Comment 1 Mikhail Pozdnyakov 2013-04-24 05:54:52 PDT
Created attachment 199418 [details]
WIP

Newly added file is not yet included into proj files.
Comment 2 Anders Carlsson 2013-04-24 07:23:23 PDT
I like it, but I think it should just go into TypeTraits.h directly.
Comment 3 Mikhail Pozdnyakov 2013-04-24 07:53:28 PDT
Created attachment 199454 [details]
patch

Put 'move()' to TypeTraits.h as requested by Anders.
Comment 4 Alexey Proskuryakov 2013-04-24 08:55:13 PDT
What is the difference with std::move? I'm wondering if we can just use it directly.
Comment 5 Benjamin Poulain 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.
Comment 6 Anders Carlsson 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.
Comment 7 Benjamin Poulain 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.
Comment 8 Mikhail Pozdnyakov 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.
Comment 9 Benjamin Poulain 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.
Comment 10 Mikhail Pozdnyakov 2013-04-25 01:28:10 PDT
Created attachment 199631 [details]
patch v3

Use std::move.
Comment 11 Mikhail Pozdnyakov 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?
Comment 12 Anders Carlsson 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.
Comment 13 Mikhail Pozdnyakov 2013-04-25 07:41:02 PDT
Created attachment 199660 [details]
to be landed
Comment 14 Anders Carlsson 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).
Comment 15 WebKit Commit Bot 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.
Comment 16 WebKit Commit Bot 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>
Comment 17 WebKit Commit Bot 2013-04-25 08:16:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Alexey Proskuryakov 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.