Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Drop 'PassOutType' type and 'passOut' method from hash traits and con…
Browse files Browse the repository at this point in the history
…tainers, 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}
  • Loading branch information
mikhail.pozdnyakov authored and Commit bot committed May 16, 2016
1 parent 5b9dec1 commit bac7970
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 60 deletions.
16 changes: 8 additions & 8 deletions styleguide/c++/c++11.html
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,14 @@ <h2 id="whitelist"><a name="core-whitelist"></a>C++11 Allowed Features</h2>
<td><a href="https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/POISBQEhGzU">Discussion thread</a></td>
</tr>

<tr>
<td>Trailing Return Types</td>
<td><code>auto <i>function declaration</i> -> <i>return_type</i></code></td>
<td>Allows trailing function return value syntax</td>
<td><a href="http://en.cppreference.com/w/cpp/language/function">Declaring functions</a></td>
<td>Use only where it considerably improves readability. <a href="https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/OQyYSfH9m2M">Discussion thread</a>. <a href="https://groups.google.com/a/chromium.org/forum/#!topic/cxx/Lkp0nubVd0Q">Another discussion thread.</a></td>
</tr>

<tr>
<td>Uniform Initialization Syntax</td>
<td><code><i>type</i> <i>name</i> {[<i>value</i> ..., <i>value</i>]};</code></td>
Expand Down Expand Up @@ -869,14 +877,6 @@ <h3 id="blacklist_stdlib_review"><a name="library-review"></a>C++11 Standard Lib
<td></td>
</tr>

<tr>
<td>Trailing Return Types</td>
<td><code>auto <i>function declaration</i> -> <i>return_type</i></code></td>
<td>Allows trailing function return value syntax</td>
<td><a href="http://en.cppreference.com/w/cpp/language/function">Declaring functions</a></td>
<td><a href="https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/OQyYSfH9m2M">Discussion thread</a></td>
</tr>

<tr>
<td>Type-Generic Math Functions</td>
<td>Functions within <code>&lt;ctgmath&gt;</code></td>
Expand Down
14 changes: 2 additions & 12 deletions third_party/WebKit/Source/platform/heap/HeapAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -467,16 +467,13 @@ template<typename T> struct HashTraits<blink::Member<T>> : SimpleClassHashTraits
using IteratorConstReferenceType = const blink::Member<T>&;
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<typename U>
static void store(const U& value, blink::Member<T>& storage) { storage = value; }

static PeekOutType peek(const blink::Member<T>& value) { return value; }
static PassOutType passOut(const blink::Member<T>& value) { return value; }
};

template<typename T> struct HashTraits<blink::WeakMember<T>> : SimpleClassHashTraits<blink::WeakMember<T>> {
Expand All @@ -495,16 +492,13 @@ template<typename T> struct HashTraits<blink::WeakMember<T>> : SimpleClassHashTr
using IteratorConstReferenceType = const blink::WeakMember<T>&;
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<typename U>
static void store(const U& value, blink::WeakMember<T>& storage) { storage = value; }

static PeekOutType peek(const blink::WeakMember<T>& value) { return value; }
static PassOutType passOut(const blink::WeakMember<T>& value) { return value; }

template<typename VisitorDispatcher>
static bool traceInCollection(VisitorDispatcher visitor, blink::WeakMember<T>& weakMember, ShouldWeakPointersBeMarkedStrongly strongify)
Expand Down Expand Up @@ -532,16 +526,12 @@ template<typename T> struct HashTraits<blink::UntracedMember<T>> : SimpleClassHa
using IteratorConstReferenceType = const blink::UntracedMember<T>&;
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<typename U>
static void store(const U& value, blink::UntracedMember<T>& storage) { storage = value; }

static PeekOutType peek(const blink::UntracedMember<T>& value) { return value; }
static PassOutType passOut(const blink::UntracedMember<T>& value) { return value; }
};

template<typename T, size_t inlineCapacity>
Expand Down
10 changes: 4 additions & 6 deletions third_party/WebKit/Source/wtf/HashMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -125,7 +124,7 @@ class HashMap {
template <typename Collection>
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
Expand Down Expand Up @@ -422,13 +421,12 @@ inline void HashMap<T, U, V, W, X, Y>::clear()
}

template <typename T, typename U, typename V, typename W, typename X, typename Y>
typename HashMap<T, U, V, W, X, Y>::MappedPassOutType
HashMap<T, U, V, W, X, Y>::take(KeyPeekInType key)
auto HashMap<T, U, V, W, X, Y>::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;
}
Expand Down
15 changes: 7 additions & 8 deletions third_party/WebKit/Source/wtf/HashSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -105,9 +104,9 @@ class HashSet {
template <typename Collection>
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 <typename VisitorDispatcher>
void trace(VisitorDispatcher visitor) { m_impl.trace(visitor); }
Expand Down Expand Up @@ -225,25 +224,25 @@ inline void HashSet<T, U, V, W>::clear()
}

template <typename T, typename U, typename V, typename W>
inline typename HashSet<T, U, V, W>::ValuePassOutType HashSet<T, U, V, W>::take(iterator it)
inline auto HashSet<T, U, V, W>::take(iterator it) -> ValueType
{
if (it == end())
return ValueTraits::emptyValue();

ValuePassOutType result = ValueTraits::passOut(const_cast<ValueType&>(*it));
ValueType result = std::move(const_cast<ValueType&>(*it));
remove(it);

return result;
}

template <typename T, typename U, typename V, typename W>
inline typename HashSet<T, U, V, W>::ValuePassOutType HashSet<T, U, V, W>::take(ValuePeekInType value)
inline auto HashSet<T, U, V, W>::take(ValuePeekInType value) -> ValueType
{
return take(find(value));
}

template <typename T, typename U, typename V, typename W>
inline typename HashSet<T, U, V, W>::ValuePassOutType HashSet<T, U, V, W>::takeAny()
inline auto HashSet<T, U, V, W>::takeAny() -> ValueType
{
return take(begin());
}
Expand Down
17 changes: 0 additions & 17 deletions third_party/WebKit/Source/wtf/HashTraits.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ template <typename T> struct GenericHashTraits : GenericHashTraitsBase<std::is_i
template <typename IncomingValueType>
static void store(IncomingValueType&& value, T& storage) { storage = std::forward<IncomingValueType>(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
Expand Down Expand Up @@ -172,10 +167,6 @@ template <typename P> struct HashTraits<OwnPtr<P>> : SimpleClassHashTraits<OwnPt
typedef PassOwnPtr<P> PassInType;
static void store(PassOwnPtr<P> value, OwnPtr<P>& storage) { storage = std::move(value); }

typedef PassOwnPtr<P> PassOutType;
static PassOwnPtr<P> passOut(OwnPtr<P>& value) { return value.release(); }
static PassOwnPtr<P> passOut(std::nullptr_t) { return nullptr; }

typedef typename OwnPtr<P>::PtrType PeekOutType;
static PeekOutType peek(const OwnPtr<P>& value) { return value.get(); }
static PeekOutType peek(std::nullptr_t) { return 0; }
Expand All @@ -199,10 +190,6 @@ template <typename P> struct HashTraits<RefPtr<P>> : SimpleClassHashTraits<RefPt
typedef PassRefPtr<P> PassInType;
static void store(PassRefPtr<P> value, RefPtr<P>& storage) { storage = value; }

typedef PassRefPtr<P> PassOutType;
static PassOutType passOut(RefPtr<P>& value) { return value.release(); }
static PassOutType passOut(std::nullptr_t) { return nullptr; }

typedef P* PeekOutType;
static PeekOutType peek(const RefPtr<P>& value) { return value.get(); }
static PeekOutType peek(std::nullptr_t) { return 0; }
Expand All @@ -221,10 +208,6 @@ struct HashTraits<std::unique_ptr<T>> : SimpleClassHashTraits<std::unique_ptr<T>
using PassInType = std::unique_ptr<T>;
static void store(std::unique_ptr<T>&& value, std::unique_ptr<T>& storage) { storage = std::move(value); }

using PassOutType = std::unique_ptr<T>;
static std::unique_ptr<T>&& passOut(std::unique_ptr<T>& value) { return std::move(value); }
static std::unique_ptr<T> passOut(std::nullptr_t) { return nullptr; }

using PeekOutType = T*;
static PeekOutType peek(const std::unique_ptr<T>& value) { return value.get(); }
static PeekOutType peek(std::nullptr_t) { return nullptr; }
Expand Down
17 changes: 8 additions & 9 deletions third_party/WebKit/Source/wtf/ListHashSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ class ListHashSet : public ConditionalDestructor<ListHashSet<ValueArg, inlineCap
typedef ValueArg ValueType;
typedef HashTraits<ValueType> ValueTraits;
typedef typename ValueTraits::PeekInType ValuePeekInType;
typedef typename ValueTraits::PassOutType ValuePassOutType;

typedef ListHashSetIterator<ListHashSet> iterator;
typedef ListHashSetConstIterator<ListHashSet> const_iterator;
Expand Down Expand Up @@ -173,9 +172,9 @@ class ListHashSet : public ConditionalDestructor<ListHashSet<ValueArg, inlineCap
template <typename Collection>
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 <typename VisitorDispatcher>
void trace(VisitorDispatcher);
Expand Down Expand Up @@ -910,30 +909,30 @@ inline void ListHashSet<T, inlineCapacity, U, V>::clear()
}

template <typename T, size_t inlineCapacity, typename U, typename V>
typename ListHashSet<T, inlineCapacity, U, V>::ValuePassOutType ListHashSet<T, inlineCapacity, U, V>::take(iterator it)
auto ListHashSet<T, inlineCapacity, U, V>::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 T, size_t inlineCapacity, typename U, typename V>
typename ListHashSet<T, inlineCapacity, U, V>::ValuePassOutType ListHashSet<T, inlineCapacity, U, V>::take(ValuePeekInType value)
auto ListHashSet<T, inlineCapacity, U, V>::take(ValuePeekInType value) -> ValueType
{
return take(find(value));
}

template <typename T, size_t inlineCapacity, typename U, typename V>
typename ListHashSet<T, inlineCapacity, U, V>::ValuePassOutType ListHashSet<T, inlineCapacity, U, V>::takeFirst()
auto ListHashSet<T, inlineCapacity, U, V>::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;
Expand Down

0 comments on commit bac7970

Please sign in to comment.