Bug 92137

Summary: Create a specialized pair for use in HashMap iterators
Product: WebKit Reporter: Caio Marcelo de Oliveira Filho <cmarcelo>
Component: Web Template FrameworkAssignee: Caio Marcelo de Oliveira Filho <cmarcelo>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric, mjs, rniwa, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 82784    
Attachments:
Description Flags
Patch
none
Updated version for EWS
none
Patch for landing
none
Patch for landing none

Description Caio Marcelo de Oliveira Filho 2012-07-24 10:20:51 PDT
Create a specialized struct for use in HashMap iterators
Comment 1 Caio Marcelo de Oliveira Filho 2012-07-24 10:42:24 PDT
Created attachment 154102 [details]
Patch
Comment 2 Ryosuke Niwa 2012-07-24 10:48:44 PDT
Comment on attachment 154102 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154102&action=review

> Source/WTF/wtf/HashTable.h:294
> +    template<typename T, typename U> inline void hashTableSwap(KeyValuePair<T, U>& a, KeyValuePair<T, U>& b)

Do we need to keep the one with std::pair?

> Source/WTF/wtf/HashTraits.h:195
> +        KeyValuePair() : first(), second() { }
> +        KeyValuePair(const KeyTypeArg& key, const ValueTypeArg& value) : first(key), second(value) { }
> +
> +        template <typename OtherKeyType, typename OtherValueType>
> +        KeyValuePair(const KeyValuePair<OtherKeyType, OtherValueType>& other) : first(other.first), second(other.second) { }

Please put each variable initialization on a separate line.

> Source/WebKit2/Platform/CoreIPC/ArgumentCoders.h:76
> +    static void encode(ArgumentEncoder* encoder, const WTF::KeyValuePair<KeyType, ValueType>& pair)

Do we need to keep the one with std::pair?
Comment 3 Caio Marcelo de Oliveira Filho 2012-07-24 11:41:03 PDT
Comment on attachment 154102 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154102&action=review

Thanks for the comments.

>> Source/WTF/wtf/HashTable.h:294
>> +    template<typename T, typename U> inline void hashTableSwap(KeyValuePair<T, U>& a, KeyValuePair<T, U>& b)
> 
> Do we need to keep the one with std::pair?

Not anymore.

>> Source/WTF/wtf/HashTraits.h:195
>> +        KeyValuePair(const KeyValuePair<OtherKeyType, OtherValueType>& other) : first(other.first), second(other.second) { }
> 
> Please put each variable initialization on a separate line.

Fixed.

>> Source/WebKit2/Platform/CoreIPC/ArgumentCoders.h:76
>> +    static void encode(ArgumentEncoder* encoder, const WTF::KeyValuePair<KeyType, ValueType>& pair)
> 
> Do we need to keep the one with std::pair?

Yes, it is still needed: we pass thru IPC other structures that build on top of std::pair.
Comment 4 Caio Marcelo de Oliveira Filho 2012-07-24 12:09:58 PDT
Created attachment 154116 [details]
Updated version for EWS
Comment 5 Caio Marcelo de Oliveira Filho 2012-07-25 11:51:15 PDT
Created attachment 154409 [details]
Patch for landing
Comment 6 Caio Marcelo de Oliveira Filho 2012-07-25 11:53:37 PDT
Created attachment 154411 [details]
Patch for landing
Comment 7 WebKit Review Bot 2012-07-25 15:15:28 PDT
Comment on attachment 154411 [details]
Patch for landing

Clearing flags on attachment: 154411

Committed r123667: <http://trac.webkit.org/changeset/123667>
Comment 8 WebKit Review Bot 2012-07-25 15:15:33 PDT
All reviewed patches have been landed.  Closing bug.