[WTF] Add GenericHashKey
Created attachment 450363 [details] Patch
Created attachment 450364 [details] Patch
I like this idea, helping fit any type into our hash table that requires empty and deleted values, not as sure about the name. If I think of a better name I will suggest it.
Created attachment 450380 [details] Patch
(In reply to Darin Adler from comment #3) > I like this idea, helping fit any type into our hash table that requires > empty and deleted values, not as sure about the name. If I think of a better > name I will suggest it. Nice!
Comment on attachment 450380 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450380&action=review Can’t help thinking how much better some of this code would look with more use of auto, auto*, and auto&. > Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp:195 > + Range& range = m_map.add(data.m_key, Range { }).iterator->value; When I see call sites like this, I wonder if we should overload add to have the empty value as the default. It’s annoying to have to write Range { } here. > Source/JavaScriptCore/runtime/TypeLocationCache.cpp:59 > + } else > + location = iterator->value; I often like to reverse these things so that the shorter half of the if statement goes first. But really, this looks like it should be rewritten to use ensure to me. > Source/WTF/wtf/GenericHashKey.h:77 > + if (m_value.index() != 0) > + return true; Why not write this using holds_alternative instead of index? I think it would compile to the exact same code, but it would not depend on the ordering of the alternative types in the variant.
Comment on attachment 450380 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450380&action=review >> Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp:195 >> + Range& range = m_map.add(data.m_key, Range { }).iterator->value; > > When I see call sites like this, I wonder if we should overload add to have the empty value as the default. It’s annoying to have to write Range { } here. For now, in this patch, I'll just use the existing interface. >> Source/JavaScriptCore/runtime/TypeLocationCache.cpp:59 >> + location = iterator->value; > > I often like to reverse these things so that the shorter half of the if statement goes first. > > But really, this looks like it should be rewritten to use ensure to me. Sounds good! Changed. >> Source/WTF/wtf/GenericHashKey.h:77 >> + return true; > > Why not write this using holds_alternative instead of index? I think it would compile to the exact same code, but it would not depend on the ordering of the alternative types in the variant. Sounds good, changed!
Committed r288807 (246583@trunk): <https://commits.webkit.org/246583@trunk>
<rdar://problem/88257247>