Re: [webkit-dev] Proposal: WTF HashMap iterators to use key/value instead first/second

2012-10-08 Thread Caio Marcelo de Oliveira Filho 
Hello,

Follow up: this change landed in http://trac.webkit.org/changeset/130612.

Cheers,

-- 
Caio Marcelo de Oliveira Filho
openBossa @ INdT - Instituto Nokia de Tecnologia
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Proposal: WTF HashMap iterators to use key/value instead first/second

2012-08-29 Thread Joe Mason 
I'm in favour of this change, as WTF::HashMap is not std::map and having the 
interface be almost like std::map in some ways, but totally unlike it in other 
ways, is more confusing than a clean break would be.

From: webkit-dev-boun...@lists.webkit.org [webkit-dev-boun...@lists.webkit.org] 
on behalf of Caio Marcelo de Oliveira Filho [caio.olive...@openbossa.org]
Sent: Tuesday, August 28, 2012 6:24 PM
To: webkit-dev@lists.webkit.org
Subject: [webkit-dev] Proposal: WTF HashMap iterators to use key/value  instead 
first/second

Hello,

I would like to propose we change HashMap iterators to use 'key' and
'value' instead of 'first' and 'second'. The work is being tracked in
the bug https://bugs.webkit.org/show_bug.cgi?id=82784.

The idea came from https://bugs.webkit.org/show_bug.cgi?id=71063#c2. I
argue that this change will make code more readable, specially in
situations where either the key or the value is a pair. One example of
this situation is in
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

 GetterSetterMap::AddResult result =
map.add(node-name().impl(), pair);
 if (!result.isNewEntry)
-result.iterator-second.second = node;
+result.iterator-value.second = node;

another one is in
Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp

 for (CCLayerTilingData::TileMap::const_iterator iter =
m_tiler-tiles().begin(); iter != m_tiler-tiles().end(); ++iter) {
-int i = iter-first.first;
-int j = iter-first.second;
-UpdatableTile* tile = static_castUpdatableTile*(iter-second.get());
+int i = iter-key.first;
+int j = iter-key.second;
+UpdatableTile* tile = static_castUpdatableTile*(iter-value.get());

I also think that in non-pair key and non-pair value situations the
readability is improved.

One downside of this change is that STL uses first and second in its
maps so that would confuse people. After inspecting the whole
repository for 'first' and 'second', the usage of std::map in WebKit
is an exception not a rule, IMHO this makes the confusion less
troublesome.

Are there other downsides am I missing? What do you guys think? Unless
we figure this is a bad change for the project, I intend to land it.

I've attempted to land this previously, I found out it caused a major
breakage in some internal builds. I apologize for that inconvenience
and the time lost.

I'm looking into ways to make the transition smoother, but until now
couldn't find a solution that doesn't require C++11. Either way, if we
land this change, I'll sync with developers from the affected ports
that I know could be internally affected to avoid more breakage.


Best regards,

--
Caio Marcelo de Oliveira Filho
openBossa @ INdT - Instituto Nokia de Tecnologia
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

-
This transmission (including any attachments) may contain confidential 
information, privileged material (including material protected by the 
solicitor-client or other applicable privileges), or constitute non-public 
information. Any use of this information by anyone other than the intended 
recipient is prohibited. If you have received this transmission in error, 
please immediately reply to the sender and delete this information from your 
system. Use, dissemination, distribution, or reproduction of this transmission 
by unintended recipients is not authorized and may be unlawful.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Proposal: WTF HashMap iterators to use key/value instead first/second

2012-08-29 Thread Maciej Stachowiak 

I like the idea of this change. Using key and value makes things more readable 
at the site of use than a generic first/second. It's unfortunate that it is 
hard to deploy, but I think it's worth it to work through those challenges.  `

-Maciej

On Aug 28, 2012, at 3:24 PM, Caio Marcelo de Oliveira Filho 
caio.olive...@openbossa.org wrote:

 Hello,
 
 I would like to propose we change HashMap iterators to use 'key' and
 'value' instead of 'first' and 'second'. The work is being tracked in
 the bug https://bugs.webkit.org/show_bug.cgi?id=82784.
 
 The idea came from https://bugs.webkit.org/show_bug.cgi?id=71063#c2. I
 argue that this change will make code more readable, specially in
 situations where either the key or the value is a pair. One example of
 this situation is in
 Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
 
 GetterSetterMap::AddResult result =
 map.add(node-name().impl(), pair);
 if (!result.isNewEntry)
 -result.iterator-second.second = node;
 +result.iterator-value.second = node;
 
 another one is in
 Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
 
 for (CCLayerTilingData::TileMap::const_iterator iter =
 m_tiler-tiles().begin(); iter != m_tiler-tiles().end(); ++iter) {
 -int i = iter-first.first;
 -int j = iter-first.second;
 -UpdatableTile* tile = 
 static_castUpdatableTile*(iter-second.get());
 +int i = iter-key.first;
 +int j = iter-key.second;
 +UpdatableTile* tile = static_castUpdatableTile*(iter-value.get());
 
 I also think that in non-pair key and non-pair value situations the
 readability is improved.
 
 One downside of this change is that STL uses first and second in its
 maps so that would confuse people. After inspecting the whole
 repository for 'first' and 'second', the usage of std::map in WebKit
 is an exception not a rule, IMHO this makes the confusion less
 troublesome.
 
 Are there other downsides am I missing? What do you guys think? Unless
 we figure this is a bad change for the project, I intend to land it.
 
 I've attempted to land this previously, I found out it caused a major
 breakage in some internal builds. I apologize for that inconvenience
 and the time lost.
 
 I'm looking into ways to make the transition smoother, but until now
 couldn't find a solution that doesn't require C++11. Either way, if we
 land this change, I'll sync with developers from the affected ports
 that I know could be internally affected to avoid more breakage.
 
 
 Best regards,
 
 -- 
 Caio Marcelo de Oliveira Filho
 openBossa @ INdT - Instituto Nokia de Tecnologia
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Proposal: WTF HashMap iterators to use key/value instead first/second

2012-08-28 Thread Caio Marcelo de Oliveira Filho 
Hello,

I would like to propose we change HashMap iterators to use 'key' and
'value' instead of 'first' and 'second'. The work is being tracked in
the bug https://bugs.webkit.org/show_bug.cgi?id=82784.

The idea came from https://bugs.webkit.org/show_bug.cgi?id=71063#c2. I
argue that this change will make code more readable, specially in
situations where either the key or the value is a pair. One example of
this situation is in
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

 GetterSetterMap::AddResult result =
map.add(node-name().impl(), pair);
 if (!result.isNewEntry)
-result.iterator-second.second = node;
+result.iterator-value.second = node;

another one is in
Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp

 for (CCLayerTilingData::TileMap::const_iterator iter =
m_tiler-tiles().begin(); iter != m_tiler-tiles().end(); ++iter) {
-int i = iter-first.first;
-int j = iter-first.second;
-UpdatableTile* tile = static_castUpdatableTile*(iter-second.get());
+int i = iter-key.first;
+int j = iter-key.second;
+UpdatableTile* tile = static_castUpdatableTile*(iter-value.get());

I also think that in non-pair key and non-pair value situations the
readability is improved.

One downside of this change is that STL uses first and second in its
maps so that would confuse people. After inspecting the whole
repository for 'first' and 'second', the usage of std::map in WebKit
is an exception not a rule, IMHO this makes the confusion less
troublesome.

Are there other downsides am I missing? What do you guys think? Unless
we figure this is a bad change for the project, I intend to land it.

I've attempted to land this previously, I found out it caused a major
breakage in some internal builds. I apologize for that inconvenience
and the time lost.

I'm looking into ways to make the transition smoother, but until now
couldn't find a solution that doesn't require C++11. Either way, if we
land this change, I'll sync with developers from the affected ports
that I know could be internally affected to avoid more breakage.


Best regards,

-- 
Caio Marcelo de Oliveira Filho
openBossa @ INdT - Instituto Nokia de Tecnologia
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev