HashMap should work with move-only keys
Created attachment 211568 [details] Patch
Attachment 211568 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/HashMap.h', u'Source/WTF/wtf/HashTable.h', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp']" exit_code: 1 Source/WTF/wtf/HashMap.h:102: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:107: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:138: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:341: Missing spaces around >> [whitespace/operators] [3] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1808071
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1911042
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1801228
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1862233
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1878084
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1808089
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1875220
Comment on attachment 211568 [details] Patch Attachment 211568 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1896179
Created attachment 211844 [details] Patch
Attachment 211844 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/HashMap.h', u'Source/WTF/wtf/HashTable.h', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp']" exit_code: 1 Source/WTF/wtf/HashTable.h:405: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:103: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:108: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:109: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:132: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:140: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:143: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:345: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:358: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:378: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:379: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:392: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 12 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 211844 [details] Patch Attachment 211844 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1824129
Created attachment 211848 [details] Patch
Attachment 211848 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/HashMap.h', u'Source/WTF/wtf/HashTable.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderRegion.cpp', u'Source/WebCore/rendering/RenderRegion.h', u'Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp', u'Source/WebCore/svg/SVGDocumentExtensions.cpp', u'Source/WebCore/svg/SVGDocumentExtensions.h', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp']" exit_code: 1 Source/WTF/wtf/HashTable.h:405: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:103: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:108: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:109: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:132: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:140: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:143: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:345: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:358: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:378: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:379: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:392: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 12 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 211849 [details] Patch
Attachment 211849 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/HashMap.h', u'Source/WTF/wtf/HashTable.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp', u'Source/WebCore/rendering/RenderRegion.cpp', u'Source/WebCore/rendering/RenderRegion.h', u'Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp', u'Source/WebCore/svg/SVGDocumentExtensions.cpp', u'Source/WebCore/svg/SVGDocumentExtensions.h', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp']" exit_code: 1 Source/WTF/wtf/HashTable.h:405: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:103: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:108: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:109: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:132: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:140: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:143: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:345: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:358: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:378: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:379: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:392: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 12 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 211849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211849&action=review > Source/WTF/wtf/HashMap.h:428 > - auto HashMap<T, U, V, W, MappedTraits>::take(const KeyType& key) -> MappedPassOutType > + auto HashMap<T, U, V, W, MappedTraits>::take(const KeyType& key) -> MappedType So this doesn’t break callers that expect the return type to be PassRefPtr?
Comment on attachment 211849 [details] Patch Attachment 211849 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1870201
Comment on attachment 211849 [details] Patch Attachment 211849 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1919100
Comment on attachment 211849 [details] Patch Attachment 211849 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1824186
Comment on attachment 211849 [details] Patch Attachment 211849 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1872259
(In reply to comment #18) > (From update of attachment 211849 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=211849&action=review > > > Source/WTF/wtf/HashMap.h:428 > > - auto HashMap<T, U, V, W, MappedTraits>::take(const KeyType& key) -> MappedPassOutType > > + auto HashMap<T, U, V, W, MappedTraits>::take(const KeyType& key) -> MappedType > > So this doesn’t break callers that expect the return type to be PassRefPtr? It does, but I don’t think there were any callers that expected that.
Created attachment 211911 [details] Patch
Attachment 211911 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/HashMap.h', u'Source/WTF/wtf/HashTable.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/html/InputType.cpp', u'Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.cpp', u'Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp', u'Source/WebCore/rendering/RenderRegion.cpp', u'Source/WebCore/rendering/RenderRegion.h', u'Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp', u'Source/WebCore/svg/SVGDocumentExtensions.cpp', u'Source/WebCore/svg/SVGDocumentExtensions.h', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp']" exit_code: 1 Source/WTF/wtf/HashTable.h:405: Missing spaces around && [whitespace/operators] [3] Source/WebCore/html/InputType.cpp:95: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WTF/wtf/HashMap.h:103: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:108: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:109: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:132: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:140: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:143: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:345: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:358: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:378: This { should be at the end of the previous line [whitespace/braces] [4] Source/WTF/wtf/HashMap.h:379: Missing spaces around >> [whitespace/operators] [3] Source/WTF/wtf/HashMap.h:392: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 13 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 211911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211911&action=review >> Source/WTF/wtf/HashMap.h:140 >> + template<typename K, typename V> >> + AddResult inlineSet(K&&, V&&); > > Missing spaces around && [whitespace/operators] [3] Might read better on one line instead of two. >> Source/WTF/wtf/HashMap.h:143 >> + template<typename K, typename V> >> + AddResult inlineAdd(K&&, V&&); > > Missing spaces around && [whitespace/operators] [3] Might read better on one line instead of two. > Source/WebCore/html/InputType.cpp:90 > -static PassOwnPtr<InputTypeFactoryMap> createInputTypeFactoryMap() > +static OwnPtr<InputTypeFactoryMap> createInputTypeFactoryMap() The pattern here should change so we aren’t calling HashMap::add repeatedly, since those calls will be inlined. Need to do this in a loop instead. I guess I can make that change after you land. I would suggest we make a static const table that has two function pointers, one to the function that returns the type name and the other being the create function.
Committed r155963: <http://trac.webkit.org/changeset/155963>
This patch broke the Windows build: WebKitGraphics.cpp 1>C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Include\private\wtf/HashMap.h(238): error C2679: binary '=' : no operator found which takes a right-hand operand of type 'int' (or there is no acceptable conversion) C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Include\WebCore/COMPtr.h(84): could be 'COMPtr<T> &COMPtr<T>::operator =(const COMPtr<T> &)' with [ T=IClassFactory ] C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Include\WebCore/COMPtr.h(85): or 'COMPtr<T> &COMPtr<T>::operator =(T *)' with [ T=IClassFactory ] while trying to match the argument list '(COMPtr<T>, int)' with [ T=IClassFactory ] C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Include\private\wtf/HashTable.h(889) : see reference to function template instantiation 'void WTF::HashMapTranslator<ValueTraits,HashFunctions>::translate<WTF::KeyValuePair<KeyTypeArg,ValueTypeArg>,const _GUID&,T&>(WTF::KeyValuePair<KeyTypeArg,ValueTypeArg> &,U,V)' being compiled with [ ValueTraits=WTF::HashMapValueTraits<CLSIDHashTraits,WTF::HashTraits<COMPtr<IClassFactory>>>, HashFunctions=CLSIDHash, KeyTypeArg=_GUID, ValueTypeArg=COMPtr<IClassFactory>, T=int, U=const _GUID &, V=int & ] C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Include\private\wtf/HashMap.h(358) : see reference to function template instantiation 'WTF::HashTableAddResult<IteratorType> WTF::HashTable<Key,Value,Extractor,HashFunctions,Traits,KeyTraits>::add<WTF::HashMapTranslator<ValueTraits,HashFunctions>,const _GUID&,T&>(const _GUID,Extra)' being compiled with [ IteratorType=WTF::HashTableIterator<_GUID,WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>>,CLSIDHash,WTF::HashMapValueTraits<CLSIDHashTraits,WTF::HashTraits<COMPtr<IClassFactory>>>,CLSIDHashTraits>, Key=_GUID, Value=WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>, Extractor=WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>>, HashFunctions=CLSIDHash, Traits=WTF::HashMapValueTraits<CLSIDHashTraits,WTF::HashTraits<COMPtr<IClassFactory>>>, KeyTraits=CLSIDHashTraits, ValueTraits=WTF::HashMapValueTraits<CLSIDHashTraits,WTF::HashTraits<COMPtr<IClassFactory>>>, T=int, Extra=int & ] C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Include\private\wtf/HashMap.h(386) : see reference to function template instantiation 'WTF::HashTableAddResult<IteratorType> WTF::HashMap<KeyArg,MappedArg,HashArg,KeyTraitsArg>::inlineAdd<const _GUID&,T&>(K,V)' being compiled with [ IteratorType=WTF::HashTableIterator<_GUID,WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>>,CLSIDHash,WTF::HashMapValueTraits<CLSIDHashTraits,WTF::HashTraits<COMPtr<IClassFactory>>>,CLSIDHashTraits>, KeyArg=CLSID, MappedArg=COMPtr<IClassFactory>, HashArg=CLSIDHash, KeyTraitsArg=CLSIDHashTraits, T=int, K=const _GUID &, V=int & ] ..\..\win\WebKitCOMAPI.cpp(59) : see reference to function template instantiation 'WTF::HashTableAddResult<IteratorType> WTF::HashMap<KeyArg,MappedArg,HashArg,KeyTraitsArg>::add<int>(const _GUID &,T &&)' being compiled with [ IteratorType=WTF::HashTableIterator<_GUID,WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<_GUID,COMPtr<IClassFactory>>>,CLSIDHash,WTF::HashMapValueTraits<CLSIDHashTraits,WTF::HashTraits<COMPtr<IClassFactory>>>,CLSIDHashTraits>, KeyArg=CLSID, MappedArg=COMPtr<IClassFactory>, HashArg=CLSIDHash, KeyTraitsArg=CLSIDHashTraits, T=int ]
I do not want to sound confrontational, but was there a reason to ignore the several red bubbles and land this anyway?
(In reply to comment #29) > I do not want to sound confrontational, but was there a reason to ignore the several red bubbles and land this anyway? The turnaround time for getting this working was just too long and I wanted to land it because it’s blocking other changes. I was around in #webkit when i landed it.