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

[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_cast<UpdatableTile*>(iter->second.get());
+        int i = iter->key.first;
+        int j = iter->key.second;
+        UpdatableTile* tile = static_cast<UpdatableTile*>(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