Summary: | Use std::move in AtomicString | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||||||
Component: | Web Template Framework | Assignee: | 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
Mikhail Pozdnyakov
2013-04-24 05:21:46 PDT
Created attachment 199418 [details]
WIP
Newly added file is not yet included into proj files.
I like it, but I think it should just go into TypeTraits.h directly. Created attachment 199454 [details]
patch
Put 'move()' to TypeTraits.h as requested by Anders.
What is the difference with std::move? I'm wondering if we can just use it directly. (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. Good point. We could just state that any C++ compiles that supports rvalue references must also have an implementation of std::move. Comment on attachment 199454 [details]
patch
Looks like we should use std::move in AtomicString instead of introducing our own.
(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. > 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.
Created attachment 199631 [details]
patch v3
Use std::move.
(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 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. Created attachment 199660 [details]
to be landed
(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). 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 on attachment 199660 [details] to be landed Clearing flags on attachment: 199660 Committed r149112: <http://trac.webkit.org/changeset/149112> All reviewed patches have been landed. Closing bug. > 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.
|