From bac797052d9a9ab890940f7c199baa155d6ddeea Mon Sep 17 00:00:00 2001 From: "mikhail.pozdnyakov" Date: Mon, 16 May 2016 08:48:10 -0700 Subject: [PATCH] Drop 'PassOutType' type and 'passOut' method from hash traits and containers, move semantic to be used instead. This CL depends on https://codereview.chromium.org/1964093003 Also this CL updates the styleguide to allow trailing return types (as was agreed at https://groups.google.com/a/chromium.org/forum/#!topic/cxx/Lkp0nubVd0Q). BUG=567139 Review-Url: https://codereview.chromium.org/1974443002 Cr-Commit-Position: refs/heads/master@{#393839} --- styleguide/c++/c++11.html | 16 ++++++++-------- .../WebKit/Source/platform/heap/HeapAllocator.h | 14 ++------------ third_party/WebKit/Source/wtf/HashMap.h | 10 ++++------ third_party/WebKit/Source/wtf/HashSet.h | 15 +++++++-------- third_party/WebKit/Source/wtf/HashTraits.h | 17 ----------------- third_party/WebKit/Source/wtf/ListHashSet.h | 17 ++++++++--------- 6 files changed, 29 insertions(+), 60 deletions(-) diff --git a/styleguide/c++/c++11.html b/styleguide/c++/c++11.html index 8dd13d7a3119c..7999007f6062e 100644 --- a/styleguide/c++/c++11.html +++ b/styleguide/c++/c++11.html @@ -259,6 +259,14 @@

C++11 Allowed Features

Discussion thread + +Trailing Return Types +auto function declaration -> return_type +Allows trailing function return value syntax +Declaring functions +Use only where it considerably improves readability. Discussion thread. Another discussion thread. + + Uniform Initialization Syntax type name {[value ..., value]}; @@ -869,14 +877,6 @@

C++11 Standard Lib - -Trailing Return Types -auto function declaration -> return_type -Allows trailing function return value syntax -Declaring functions -Discussion thread - - Type-Generic Math Functions Functions within <ctgmath> diff --git a/third_party/WebKit/Source/platform/heap/HeapAllocator.h b/third_party/WebKit/Source/platform/heap/HeapAllocator.h index 73a9c0020e71d..8b43e8b53fa2e 100644 --- a/third_party/WebKit/Source/platform/heap/HeapAllocator.h +++ b/third_party/WebKit/Source/platform/heap/HeapAllocator.h @@ -467,16 +467,13 @@ template struct HashTraits> : SimpleClassHashTraits using IteratorConstReferenceType = const blink::Member&; static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return *x; } static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return *x; } - // FIXME: Similarly, there is no need for a distinction between PeekOutType - // and PassOutType without reference counting. + using PeekOutType = T*; - using PassOutType = T*; template static void store(const U& value, blink::Member& storage) { storage = value; } static PeekOutType peek(const blink::Member& value) { return value; } - static PassOutType passOut(const blink::Member& value) { return value; } }; template struct HashTraits> : SimpleClassHashTraits> { @@ -495,16 +492,13 @@ template struct HashTraits> : SimpleClassHashTr using IteratorConstReferenceType = const blink::WeakMember&; static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return *x; } static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return *x; } - // FIXME: Similarly, there is no need for a distinction between PeekOutType - // and PassOutType without reference counting. + using PeekOutType = T*; - using PassOutType = T*; template static void store(const U& value, blink::WeakMember& storage) { storage = value; } static PeekOutType peek(const blink::WeakMember& value) { return value; } - static PassOutType passOut(const blink::WeakMember& value) { return value; } template static bool traceInCollection(VisitorDispatcher visitor, blink::WeakMember& weakMember, ShouldWeakPointersBeMarkedStrongly strongify) @@ -532,16 +526,12 @@ template struct HashTraits> : SimpleClassHa using IteratorConstReferenceType = const blink::UntracedMember&; static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return *x; } static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return *x; } - // FIXME: Similarly, there is no need for a distinction between PeekOutType - // and PassOutType without reference counting. using PeekOutType = T*; - using PassOutType = T*; template static void store(const U& value, blink::UntracedMember& storage) { storage = value; } static PeekOutType peek(const blink::UntracedMember& value) { return value; } - static PassOutType passOut(const blink::UntracedMember& value) { return value; } }; template diff --git a/third_party/WebKit/Source/wtf/HashMap.h b/third_party/WebKit/Source/wtf/HashMap.h index 77a1a3b1dfc25..c9f6796499257 100644 --- a/third_party/WebKit/Source/wtf/HashMap.h +++ b/third_party/WebKit/Source/wtf/HashMap.h @@ -59,7 +59,6 @@ class HashMap { private: typedef typename MappedTraits::PassInType MappedPassInType; - typedef typename MappedTraits::PassOutType MappedPassOutType; typedef typename MappedTraits::PeekOutType MappedPeekType; typedef HashArg HashFunctions; @@ -125,7 +124,7 @@ class HashMap { template void removeAll(const Collection& toBeRemoved) { WTF::removeAll(*this, toBeRemoved); } - MappedPassOutType take(KeyPeekInType); // efficient combination of get with remove + MappedType take(KeyPeekInType); // efficient combination of get with remove // An alternate version of find() that finds the object by hashing and // comparing with some other type, to avoid the cost of type @@ -422,13 +421,12 @@ inline void HashMap::clear() } template -typename HashMap::MappedPassOutType -HashMap::take(KeyPeekInType key) +auto HashMap::take(KeyPeekInType key) -> MappedType { iterator it = find(key); if (it == end()) - return MappedTraits::passOut(MappedTraits::emptyValue()); - MappedPassOutType result = MappedTraits::passOut(it->value); + return MappedTraits::emptyValue(); + MappedType result = std::move(it->value); remove(it); return result; } diff --git a/third_party/WebKit/Source/wtf/HashSet.h b/third_party/WebKit/Source/wtf/HashSet.h index 7cba178df98a1..c2d6813783079 100644 --- a/third_party/WebKit/Source/wtf/HashSet.h +++ b/third_party/WebKit/Source/wtf/HashSet.h @@ -43,7 +43,6 @@ class HashSet { typedef TraitsArg ValueTraits; typedef typename ValueTraits::PeekInType ValuePeekInType; typedef typename ValueTraits::PassInType ValuePassInType; - typedef typename ValueTraits::PassOutType ValuePassOutType; public: typedef typename ValueTraits::TraitType ValueType; @@ -105,9 +104,9 @@ class HashSet { template void removeAll(const Collection& toBeRemoved) { WTF::removeAll(*this, toBeRemoved); } - ValuePassOutType take(iterator); - ValuePassOutType take(ValuePeekInType); - ValuePassOutType takeAny(); + ValueType take(iterator); + ValueType take(ValuePeekInType); + ValueType takeAny(); template void trace(VisitorDispatcher visitor) { m_impl.trace(visitor); } @@ -225,25 +224,25 @@ inline void HashSet::clear() } template -inline typename HashSet::ValuePassOutType HashSet::take(iterator it) +inline auto HashSet::take(iterator it) -> ValueType { if (it == end()) return ValueTraits::emptyValue(); - ValuePassOutType result = ValueTraits::passOut(const_cast(*it)); + ValueType result = std::move(const_cast(*it)); remove(it); return result; } template -inline typename HashSet::ValuePassOutType HashSet::take(ValuePeekInType value) +inline auto HashSet::take(ValuePeekInType value) -> ValueType { return take(find(value)); } template -inline typename HashSet::ValuePassOutType HashSet::takeAny() +inline auto HashSet::takeAny() -> ValueType { return take(begin()); } diff --git a/third_party/WebKit/Source/wtf/HashTraits.h b/third_party/WebKit/Source/wtf/HashTraits.h index 695d5cd7269fe..b5154b38ca1d2 100644 --- a/third_party/WebKit/Source/wtf/HashTraits.h +++ b/third_party/WebKit/Source/wtf/HashTraits.h @@ -109,11 +109,6 @@ template struct GenericHashTraits : GenericHashTraitsBase static void store(IncomingValueType&& value, T& storage) { storage = std::forward(value); } - // Type for return value of functions that transfer ownership, such as take. - typedef T PassOutType; - static T&& passOut(T& value) { return std::move(value); } - static T&& passOut(T&& value) { return std::move(value); } - // Type for return value of functions that do not transfer ownership, such // as get. // FIXME: We could change this type to const T& for better performance if we @@ -172,10 +167,6 @@ template struct HashTraits> : SimpleClassHashTraits PassInType; static void store(PassOwnPtr

value, OwnPtr

& storage) { storage = std::move(value); } - typedef PassOwnPtr

PassOutType; - static PassOwnPtr

passOut(OwnPtr

& value) { return value.release(); } - static PassOwnPtr

passOut(std::nullptr_t) { return nullptr; } - typedef typename OwnPtr

::PtrType PeekOutType; static PeekOutType peek(const OwnPtr

& value) { return value.get(); } static PeekOutType peek(std::nullptr_t) { return 0; } @@ -199,10 +190,6 @@ template struct HashTraits> : SimpleClassHashTraits PassInType; static void store(PassRefPtr

value, RefPtr

& storage) { storage = value; } - typedef PassRefPtr

PassOutType; - static PassOutType passOut(RefPtr

& value) { return value.release(); } - static PassOutType passOut(std::nullptr_t) { return nullptr; } - typedef P* PeekOutType; static PeekOutType peek(const RefPtr

& value) { return value.get(); } static PeekOutType peek(std::nullptr_t) { return 0; } @@ -221,10 +208,6 @@ struct HashTraits> : SimpleClassHashTraits using PassInType = std::unique_ptr; static void store(std::unique_ptr&& value, std::unique_ptr& storage) { storage = std::move(value); } - using PassOutType = std::unique_ptr; - static std::unique_ptr&& passOut(std::unique_ptr& value) { return std::move(value); } - static std::unique_ptr passOut(std::nullptr_t) { return nullptr; } - using PeekOutType = T*; static PeekOutType peek(const std::unique_ptr& value) { return value.get(); } static PeekOutType peek(std::nullptr_t) { return nullptr; } diff --git a/third_party/WebKit/Source/wtf/ListHashSet.h b/third_party/WebKit/Source/wtf/ListHashSet.h index 1afd63b5bf917..f819061294994 100644 --- a/third_party/WebKit/Source/wtf/ListHashSet.h +++ b/third_party/WebKit/Source/wtf/ListHashSet.h @@ -74,7 +74,6 @@ class ListHashSet : public ConditionalDestructor ValueTraits; typedef typename ValueTraits::PeekInType ValuePeekInType; - typedef typename ValueTraits::PassOutType ValuePassOutType; typedef ListHashSetIterator iterator; typedef ListHashSetConstIterator const_iterator; @@ -173,9 +172,9 @@ class ListHashSet : public ConditionalDestructor void removeAll(const Collection& other) { WTF::removeAll(*this, other); } - ValuePassOutType take(iterator); - ValuePassOutType take(ValuePeekInType); - ValuePassOutType takeFirst(); + ValueType take(iterator); + ValueType take(ValuePeekInType); + ValueType takeFirst(); template void trace(VisitorDispatcher); @@ -910,30 +909,30 @@ inline void ListHashSet::clear() } template -typename ListHashSet::ValuePassOutType ListHashSet::take(iterator it) +auto ListHashSet::take(iterator it) -> ValueType { if (it == end()) return ValueTraits::emptyValue(); m_impl.remove(it.getNode()); - ValuePassOutType result = ValueTraits::passOut(it.getNode()->m_value); + ValueType result = std::move(it.getNode()->m_value); unlinkAndDelete(it.getNode()); return result; } template -typename ListHashSet::ValuePassOutType ListHashSet::take(ValuePeekInType value) +auto ListHashSet::take(ValuePeekInType value) -> ValueType { return take(find(value)); } template -typename ListHashSet::ValuePassOutType ListHashSet::takeFirst() +auto ListHashSet::takeFirst() -> ValueType { ASSERT(!isEmpty()); m_impl.remove(m_head); - ValuePassOutType result = ValueTraits::passOut(m_head->m_value); + ValueType result = std::move(m_head->m_value); unlinkAndDelete(m_head); return result;