[2920/5456] Building CXX object Source/WebCore/CMakeFiles...vedSources/unified-sources/UnifiedSource-f74e0903-7.cpp.o In file included from ../../Source/WebCore/page/SecurityOriginData.h:28, from ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:30, from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: In member function ‘WTF::URL& WTF::URL::operator=(WTF::URL&&)’, inlined from ‘void WebCore::ServiceWorkerRegistrationKey::setScope(WTF::URL&&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:52:44, inlined from ‘static void WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>::constructDeletedValue(WebCore::ServiceWorkerRegistrationKey&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:99, inlined from ‘typename std::enable_if<(! WTF::HashTraitHasCustomDelete<Traits, T>::value)>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; T = WebCore::ServiceWorkerRegistrationKey]’ at WTF/Headers/wtf/HashTraits.h:305:34, inlined from ‘static void WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::customDeleteBucket(WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::TraitType&) [with KeyTraitsArg = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; ValueTraitsArg = WTF::HashTraits<WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:378:42, inlined from ‘typename std::enable_if<WTF::HashTraitHasCustomDelete<Traits, T>::value>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; T = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:297:31, inlined from ‘static void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::deleteBucket(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType&) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:549:85, inlined from ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:1127:21: WTF/Headers/wtf/URL.h:56:7: warning: ‘*(long unsigned int*)((char*)&<unnamed> + offsetof(WTF::URL, WTF::URL::m_isValid))’ is used uninitialized [-Wuninitialized] 56 | class URL { | ^~~ In file included from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h: In member function ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:125: note: ‘<anonymous>’ declared here 109 | static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.setScope(URL(HashTableDeletedValue)); } | ^ In file included from ../../Source/WebCore/page/SecurityOriginData.h:28, from ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:30, from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: In member function ‘WTF::URL& WTF::URL::operator=(WTF::URL&&)’, inlined from ‘void WebCore::ServiceWorkerRegistrationKey::setScope(WTF::URL&&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:52:44, inlined from ‘static void WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>::constructDeletedValue(WebCore::ServiceWorkerRegistrationKey&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:99, inlined from ‘typename std::enable_if<(! WTF::HashTraitHasCustomDelete<Traits, T>::value)>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; T = WebCore::ServiceWorkerRegistrationKey]’ at WTF/Headers/wtf/HashTraits.h:305:34, inlined from ‘static void WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::customDeleteBucket(WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::TraitType&) [with KeyTraitsArg = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; ValueTraitsArg = WTF::HashTraits<WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:378:42, inlined from ‘typename std::enable_if<WTF::HashTraitHasCustomDelete<Traits, T>::value>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; T = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:297:31, inlined from ‘static void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::deleteBucket(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType&) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:549:85, inlined from ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:1127:21: WTF/Headers/wtf/URL.h:56:7: warning: ‘*(long unsigned int*)((char*)&<unnamed> + offsetof(WTF::URL, WTF::URL::m_userEnd))’ is used uninitialized [-Wuninitialized] 56 | class URL { | ^~~ In file included from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h: In member function ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:125: note: ‘<anonymous>’ declared here 109 | static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.setScope(URL(HashTableDeletedValue)); } | ^ In file included from ../../Source/WebCore/page/SecurityOriginData.h:28, from ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:30, from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: In member function ‘WTF::URL& WTF::URL::operator=(WTF::URL&&)’, inlined from ‘void WebCore::ServiceWorkerRegistrationKey::setScope(WTF::URL&&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:52:44, inlined from ‘static void WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>::constructDeletedValue(WebCore::ServiceWorkerRegistrationKey&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:99, inlined from ‘typename std::enable_if<(! WTF::HashTraitHasCustomDelete<Traits, T>::value)>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; T = WebCore::ServiceWorkerRegistrationKey]’ at WTF/Headers/wtf/HashTraits.h:305:34, inlined from ‘static void WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::customDeleteBucket(WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::TraitType&) [with KeyTraitsArg = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; ValueTraitsArg = WTF::HashTraits<WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:378:42, inlined from ‘typename std::enable_if<WTF::HashTraitHasCustomDelete<Traits, T>::value>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; T = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:297:31, inlined from ‘static void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::deleteBucket(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType&) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:549:85, inlined from ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:1127:21: WTF/Headers/wtf/URL.h:56:7: warning: ‘*(long unsigned int*)((char*)&<unnamed> + offsetof(WTF::URL, WTF::URL::m_hostEnd))’ is used uninitialized [-Wuninitialized] 56 | class URL { | ^~~ In file included from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h: In member function ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:125: note: ‘<anonymous>’ declared here 109 | static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.setScope(URL(HashTableDeletedValue)); } | ^ In file included from ../../Source/WebCore/page/SecurityOriginData.h:28, from ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:30, from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: In member function ‘WTF::URL& WTF::URL::operator=(WTF::URL&&)’, inlined from ‘void WebCore::ServiceWorkerRegistrationKey::setScope(WTF::URL&&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:52:44, inlined from ‘static void WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>::constructDeletedValue(WebCore::ServiceWorkerRegistrationKey&)’ at ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:99, inlined from ‘typename std::enable_if<(! WTF::HashTraitHasCustomDelete<Traits, T>::value)>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; T = WebCore::ServiceWorkerRegistrationKey]’ at WTF/Headers/wtf/HashTraits.h:305:34, inlined from ‘static void WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::customDeleteBucket(WTF::KeyValuePairHashTraits<KeyTraitsArg, ValueTraitsArg>::TraitType&) [with KeyTraitsArg = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>; ValueTraitsArg = WTF::HashTraits<WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:378:42, inlined from ‘typename std::enable_if<WTF::HashTraitHasCustomDelete<Traits, T>::value>::type WTF::hashTraitsDeleteBucket(T&) [with Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; T = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >]’ at WTF/Headers/wtf/HashTraits.h:297:31, inlined from ‘static void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::deleteBucket(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType&) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:549:85, inlined from ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’ at WTF/Headers/wtf/HashTable.h:1127:21: WTF/Headers/wtf/URL.h:56:7: warning: ‘*(long unsigned int*)((char*)&<unnamed> + offsetof(WTF::URL, WTF::URL::m_pathEnd))’ is used uninitialized [-Wuninitialized] 56 | class URL { | ^~~ In file included from ../../Source/WebCore/workers/service/server/SWScriptStorage.cpp:32, from WebCore/DerivedSources/unified-sources/UnifiedSource-f74e0903-7.cpp:1: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h: In member function ‘void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType*) [with Key = WebCore::ServiceWorkerRegistrationKey; Value = WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> > >; HashFunctions = WTF::DefaultHash<WebCore::ServiceWorkerRegistrationKey>; Traits = WTF::HashMap<WebCore::ServiceWorkerRegistrationKey, WTF::WeakPtr<WebCore::SWServerRegistration> >::KeyValuePairTraits; KeyTraits = WTF::HashTraits<WebCore::ServiceWorkerRegistrationKey>]’: ../../Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:109:125: note: ‘<anonymous>’ declared here 109 | static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.setScope(URL(HashTableDeletedValue)); } | ^ From bug #224381, Alex says: "Very interesting, although unrelated to this change. It looks like it's complaining that URL::URL(HashTableDeletedValueType) doesn't initialize any members but m_string. That's fine because only m_string is used for hash-table-deleted-entry-URLs and probably preferred because it does fewer write operations. Maybe there's a way to silence those warnings or initialize all the members only in builds that would see those warnings." I dunno, it's more robust to just always initialize everything. The warning is complaining because the partially-uninitialized URL gets passed to ServiceWorkerRegistrationKey's copy assignment operator, so the uninitialized data does get used to initialize another URL. It might be harmless if the data is really never read, but now ServiceWorkerRegistrationKey::scope is going to return a partially-initialized URL. I guess it's probably OK because it's only ever used as the HashTableDeletedValueType, but I think initializing everything is the safer way to go in this case. I doubt there will be any noticeable performance impact. Alternatively, we could simply silence the warning, which is probably easy to do.
Created attachment 426455 [details] Patch
Comment on attachment 426455 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426455&action=review > Source/WTF/wtf/URL.h:343 > inline URL::URL(HashTableDeletedValueType) > : m_string(HashTableDeletedValue) > { > + invalidate(); > } Does this look OK, Alex?
(In reply to Michael Catanzaro from comment #0) > Alternatively, we could simply silence the warning, which is probably easy > to do. I'm not sure. If Alex doesn't like my invalidate() patch, then I will need to investigate more to be certain, but I *think* the warning would have to be suppressed in the copy constructor of ServiceWorkerRegistrationKey, which is far from the URL constructor where the uninitialized data is coming from. So it doesn't seem like a great solution. Another option would be to call invalidate() only on Linux, if you really want to avoid fully-initializing the URL on Apple platforms, but this seems like the worst option.
Would it work instead to add a constructor to ServiceWorkerRegistrationKey that takes a HashTableDeletedValue and call that instead? This adds an unnecessary function call that writes 0's unnecessarily in possibly hot code.
Comment on attachment 426455 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426455&action=review >> Source/WTF/wtf/URL.h:343 >> } > > Does this look OK, Alex? This is bad for performance and should be unnecessary. We should suppress the warning instead.
I like Alex’s suggestion.
(In reply to Alex Christensen from comment #4) > Would it work instead to add a constructor to ServiceWorkerRegistrationKey > that takes a HashTableDeletedValue and call that instead? TODO: I will try this.
(In reply to Alex Christensen from comment #4) > Would it work instead to add a constructor to ServiceWorkerRegistrationKey > that takes a HashTableDeletedValue and call that instead? I got something similar to work, though it doesn't involve a constructor.
Created attachment 426619 [details] Patch
Comment on attachment 426619 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426619&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:69 > + friend class HashTraits<WebCore::ServiceWorkerRegistrationKey>; Looks like Clang doesn't like 'friend class' since HashTraits is a struct, oops. I'm surprised that GCC allowed it. Anyway, I bet 'friend struct' will work.
Created attachment 426623 [details] Patch
Comment on attachment 426623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426623&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 > + bool m_isHashTableDeletedValue; This needs to be initialized to false!
Comment on attachment 426623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426623&action=review >> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 >> + bool m_isHashTableDeletedValue; > > This needs to be initialized to false! Ugh, of course, sorry....
Created attachment 426626 [details] Patch
(In reply to Michael Catanzaro from comment #13) > Ugh, of course, sorry.... Especially embarrassing when the goal of the patch was to avoid an uninitialized data copy. I had only one job here....
Comment on attachment 426626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426626&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 > + friend struct HashTraits<WebCore::ServiceWorkerRegistrationKey>; > + > SecurityOriginData m_topOrigin; > URL m_scope; > + bool m_isHashTableDeletedValue { false }; This will enlarge sizeof(ServiceWorkerRegistrationKey) and allocation used in HashMap<ServiceWorkerRegistrationKey, XXX>.
Comment on attachment 426626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426626&action=review >> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 >> + bool m_isHashTableDeletedValue { false }; > > This will enlarge sizeof(ServiceWorkerRegistrationKey) and allocation used in HashMap<ServiceWorkerRegistrationKey, XXX>. How much will it enlarge it?
Comment on attachment 426626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426626&action=review >>> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 >>> + bool m_isHashTableDeletedValue { false }; >> >> This will enlarge sizeof(ServiceWorkerRegistrationKey) and allocation used in HashMap<ServiceWorkerRegistrationKey, XXX>. > > How much will it enlarge it? It is a shame because enlarging it is unnecessary. Instead we can use any unused bit anywhere else in the object.
Comment on attachment 426626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426626&action=review >>> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 >>> + bool m_isHashTableDeletedValue { false }; >> >> This will enlarge sizeof(ServiceWorkerRegistrationKey) and allocation used in HashMap<ServiceWorkerRegistrationKey, XXX>. > > How much will it enlarge it? 8 byte because of alignment. 64 => 72. I don't think we should pay a memory cost to suppress warnings.
Change to review- then.
Comment on attachment 426626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426626&action=review >>>>> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:73 >>>>> + bool m_isHashTableDeletedValue { false }; >>>> >>>> This will enlarge sizeof(ServiceWorkerRegistrationKey) and allocation used in HashMap<ServiceWorkerRegistrationKey, XXX>. >>> >>> How much will it enlarge it? >> >> It is a shame because enlarging it is unnecessary. Instead we can use any unused bit anywhere else in the object. > > 8 byte because of alignment. 64 => 72. I don't think we should pay a memory cost to suppress warnings. Seems indeed unfortunate to add a data member just for this purpose. If the url constructor from a hash table deleting value is no good, then maybe we can use the one of SecurityOriginData (since this is the type of our other data member)?
Comment on attachment 426626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426626&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:-109 > - static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.setScope(URL(HashTableDeletedValue)); } Would this silence your warning? static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = SecurityOriginData { HashTableDeletedValue }; }
(In reply to Chris Dumez from comment #22) > Comment on attachment 426626 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426626&action=review > > > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:-109 > > - static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.setScope(URL(HashTableDeletedValue)); } > > Would this silence your warning? > static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& > slot) { slot.m_topOrigin = SecurityOriginData { HashTableDeletedValue }; } BTW, the warning is about the URL constructor from a HashTableDeletedValue. Fixing it by avoiding to call that constructor doesn't seem like a great fix to me, especially if it is by adding an extra data member to the class where it is used. I would be fine using the SecurityOriginData constructor from a HashTableDeletedValue instead since it does not make the ServiceWorkerRegistrationKey worse in any way and it likely silences your warning. However, the root of the issue is URL. I think we either need to make the URL(HashTableDeletedValue) constructor initialize its data member properly OR decide that it is OK for this particular constructor to not initialize those members (since we don't need them for this particular use case and initializing may hurt perf). In the latter case, it seems silencing the warning at compiler level would be an appropriate fix?
(In reply to Yusuke Suzuki from comment #19) > 8 byte because of alignment. 64 => 72. I don't think we should pay a memory > cost to suppress warnings. Hm, well the warning is there for a reason: we really *are* copying uninitialized data. In this case, I think it's harmless, because it *should* never be read, because the ServiceWorkerRegistrationKey object should only contain a partially-uninitialized URL if the ServiceWorkerRegistrationKey is itself a HashTableDeletedValue, and it seems unlikely that code would try to access ServiceWorkerRegistrationKey::scope on a ServiceWorkerRegistrationKey that is a HashTableDeletedValue. But there is robustness benefit to avoiding this, so code changes are justified IMO. But I think Chris is right, we probably don't need to pay the cost in this particular case. (In reply to Chris Dumez from comment #23) > I would be fine using the SecurityOriginData constructor from a > HashTableDeletedValue instead since it does not make the > ServiceWorkerRegistrationKey worse in any way and it likely silences your > warning. Good point, I didn't notice that. I could certainly try using the SecurityOriginData instead. That would probably work fine. I think it makes sense to rely on the HashTableDeletedValue of any child object when possible. > However, the root of the issue is URL. I think we either need to make the > URL(HashTableDeletedValue) constructor initialize its data member properly Alex is quite opposed to this. > OR decide that it is OK for this particular constructor to not initialize > those members (since we don't need them for this particular use case and > initializing may hurt perf). In the latter case, it seems silencing the > warning at compiler level would be an appropriate fix? In theory, it's safe to leave the data uninitialized, because the HashTableDeletedValue URL should never be copied. But this warning indicates it *does* actually get copied by ServiceWorkerRegistrationKey::operator=. Suppressing the warning in URL itself would potentially hide errors elsewhere. So I think I prefer to try using SecurityOriginData instead.
(Note: I'm uploading a revised patch for review now, but I still need to test a build tomorrow to ensure the warning is truly gone before landing.)
(In reply to Michael Catanzaro from comment #24) > (In reply to Yusuke Suzuki from comment #19) > > 8 byte because of alignment. 64 => 72. I don't think we should pay a memory > > cost to suppress warnings. > > Hm, well the warning is there for a reason: we really *are* copying > uninitialized data. In this case, I think it's harmless, because it *should* > never be read, because the ServiceWorkerRegistrationKey object should only > contain a partially-uninitialized URL if the ServiceWorkerRegistrationKey is > itself a HashTableDeletedValue, and it seems unlikely that code would try to > access ServiceWorkerRegistrationKey::scope on a ServiceWorkerRegistrationKey > that is a HashTableDeletedValue. But there is robustness benefit to avoiding > this, so code changes are justified IMO. > > But I think Chris is right, we probably don't need to pay the cost in this > particular case. > > (In reply to Chris Dumez from comment #23) > > I would be fine using the SecurityOriginData constructor from a > > HashTableDeletedValue instead since it does not make the > > ServiceWorkerRegistrationKey worse in any way and it likely silences your > > warning. > > Good point, I didn't notice that. I could certainly try using the > SecurityOriginData instead. That would probably work fine. I think it makes > sense to rely on the HashTableDeletedValue of any child object when possible. > > > However, the root of the issue is URL. I think we either need to make the > > URL(HashTableDeletedValue) constructor initialize its data member properly > > Alex is quite opposed to this. > > > OR decide that it is OK for this particular constructor to not initialize > > those members (since we don't need them for this particular use case and > > initializing may hurt perf). In the latter case, it seems silencing the > > warning at compiler level would be an appropriate fix? > > In theory, it's safe to leave the data uninitialized, because the > HashTableDeletedValue URL should never be copied. But this warning indicates > it *does* actually get copied by ServiceWorkerRegistrationKey::operator=. > Suppressing the warning in URL itself would potentially hide errors > elsewhere. So I think I prefer to try using SecurityOriginData instead. The compiler output is a bit hard to head but it wasn't obvious to me that the issue was with ServiceWorkerRegistrationKey::operator=() copying. It looked to me that the issue was calling the move constructor of URL: WTF::URL::operator=(WTF::URL&&) here: slot.setScope(URL(HashTableDeletedValue)); I see. I missed that this was due to ServiceWorkerRegistrationKey copying. Shouldn't we be able to move such URL constructed from HashTableDeletedValue?
Created attachment 426632 [details] Patch
Comment on attachment 426632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426632&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:111 > + static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = SecurityOriginData(HashTableDeletedValue); } WebCore::SecurityOriginData
Created attachment 426635 [details] Patch
(In reply to Chris Dumez from comment #26) > The compiler output is a bit hard to head but it wasn't obvious to me that > the issue was with ServiceWorkerRegistrationKey::operator=() copying. It > looked to me that the issue was calling the move constructor of URL: > WTF::URL::operator=(WTF::URL&&) > here: > slot.setScope(URL(HashTableDeletedValue)); Well I think you're right, it's not a copy at all, it's a move. You're better at squinting at obtuse warnings than I am. > I see. I missed that this was due to ServiceWorkerRegistrationKey copying. Well... squinting at the warning, I guess it's not. > Shouldn't we be able to move such URL constructed from HashTableDeletedValue? Well, I agree. My opinion is that objects in WebKit should never have uninitialized data unless we are able to demonstrate a real performance improvement caused by leaving the data uninitialized, which I assume will be very rare. Uninitialized data is dangerous for robustness because programmers do not expect data to be uninitialized. But Alex is worried about performance. I have no doubt that hash tables containing URLs are performance-sensitive, though I do doubt that leaving URLs partially-initialized is really a significant optimization. Anyway, my patches to ServiceWorkerRegistrationKey only make sense if patches to fully-initialize URL will not be accepted.
(In reply to Michael Catanzaro from comment #30) > (In reply to Chris Dumez from comment #26) > > The compiler output is a bit hard to head but it wasn't obvious to me that > > the issue was with ServiceWorkerRegistrationKey::operator=() copying. It > > looked to me that the issue was calling the move constructor of URL: > > WTF::URL::operator=(WTF::URL&&) > > here: > > slot.setScope(URL(HashTableDeletedValue)); > > Well I think you're right, it's not a copy at all, it's a move. You're > better at squinting at obtuse warnings than I am. > > > I see. I missed that this was due to ServiceWorkerRegistrationKey copying. > > Well... squinting at the warning, I guess it's not. > > > Shouldn't we be able to move such URL constructed from HashTableDeletedValue? > > Well, I agree. My opinion is that objects in WebKit should never have > uninitialized data unless we are able to demonstrate a real performance > improvement caused by leaving the data uninitialized, which I assume will be > very rare. Uninitialized data is dangerous for robustness because > programmers do not expect data to be uninitialized. > > But Alex is worried about performance. I have no doubt that hash tables > containing URLs are performance-sensitive, though I do doubt that leaving > URLs partially-initialized is really a significant optimization. Anyway, my > patches to ServiceWorkerRegistrationKey only make sense if patches to > fully-initialize URL will not be accepted. I think this latest patch is completely fine. It does not make things worse in anyway. I have a feeling though that this will bite us again in the future in other places if we don't fix it at URL level. I could buy that URL construction from HashTableDeletedValue may be hot enough that we don't want to initialize those members. That is why I suggested that maybe we want to silence this at compiler level, at URL layer. The move constructor for URL will do the right thing, even if constructed from HashTableDeletedValue. Yes, it will copy some data members that are garbage, but this is fine. It will copy the data member we properly initialized and URL.isDeletedHashTableValue() will still be true. Anyway, something to consider in a follow-up maybe?
Comment on attachment 426635 [details] Patch r=me. If we see this issue in other places, maybe we'll consider silencing the warning at URL layer instead?
Thanks for reviewing. (In reply to Chris Dumez from comment #32) > r=me. If we see this issue in other places, maybe we'll consider silencing > the warning at URL layer instead? I haven't tried, but I think we could do this in URL.h: IGNORE_WARNINGS_BEGIN(uninitialized) class URL { IGNORE_WARNING_END That's the line that's ultimately associated with the warning, so I think that's where the suppression would need to be. That seems non-ideal. Alternatively, we could remove SWScriptStorage.cpp from unified build and suppress -Wuninitialized for the entire file, which seems worse, and is punitive to SWScriptStorage.cpp, which isn't doing anything objectionable at all.
Comment on attachment 426635 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426635&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:111 > + static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = WebCore::SecurityOriginData(HashTableDeletedValue); } This isn’t quite right. What we want to do is to use placement new to put SecurityOriginData(HashTableDeletedValue) on top of m_topOrigin, not do an assignment to replace an existing value. We don't want to destroy the existing value that is there. The whole point of "constructDeletedValue" is that it makes a deleted value out of uninitialized memory, not on top of an already valid object. It would be better to get this right. There are lots of other examples of how to use the placement new syntax to do this correctly.
Comment on attachment 426635 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426635&action=review >> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:111 >> + static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = WebCore::SecurityOriginData(HashTableDeletedValue); } > > This isn’t quite right. What we want to do is to use placement new to put SecurityOriginData(HashTableDeletedValue) on top of m_topOrigin, not do an assignment to replace an existing value. We don't want to destroy the existing value that is there. The whole point of "constructDeletedValue" is that it makes a deleted value out of uninitialized memory, not on top of an already valid object. > > It would be better to get this right. There are lots of other examples of how to use the placement new syntax to do this correctly. Ok. I didn't know that. Looking at existing examples, seems like we want this then? ``` static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { new (NotNull, &slot) WebCore::ServiceWorkerRegistrationKey { WebCore::SecurityOriginData { WTF::HashTableDeletedValue }, { } }; } ```
Comment on attachment 426635 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426635&action=review >> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:111 >> + static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = WebCore::SecurityOriginData(HashTableDeletedValue); } > > This isn’t quite right. What we want to do is to use placement new to put SecurityOriginData(HashTableDeletedValue) on top of m_topOrigin, not do an assignment to replace an existing value. We don't want to destroy the existing value that is there. The whole point of "constructDeletedValue" is that it makes a deleted value out of uninitialized memory, not on top of an already valid object. > > It would be better to get this right. There are lots of other examples of how to use the placement new syntax to do this correctly. OK, sure. I'll look closer tomorrow. Note that the original code does not do that, it assumes that the ServiceWorkerRegistrationKey it is passed is already valid and calls setScope on it, which does a move assignment: void setScope(URL&& scope) { m_scope = WTFMove(scope); }
(In reply to Michael Catanzaro from comment #36) > Comment on attachment 426635 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426635&action=review > > >> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:111 > >> + static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = WebCore::SecurityOriginData(HashTableDeletedValue); } > > > > This isn’t quite right. What we want to do is to use placement new to put SecurityOriginData(HashTableDeletedValue) on top of m_topOrigin, not do an assignment to replace an existing value. We don't want to destroy the existing value that is there. The whole point of "constructDeletedValue" is that it makes a deleted value out of uninitialized memory, not on top of an already valid object. > > > > It would be better to get this right. There are lots of other examples of how to use the placement new syntax to do this correctly. > > OK, sure. I'll look closer tomorrow. > > Note that the original code does not do that, it assumes that the > ServiceWorkerRegistrationKey it is passed is already valid and calls > setScope on it, which does a move assignment: > > void setScope(URL&& scope) { m_scope = WTFMove(scope); } Yes, your new code is not worse than the existing one as far as I can tell. I probably wrote the original code and did not know about this. Looking at our code base, I see a lot of them using placement new and a lot of them using what we had. Anyway, I'll make sure to get this right from now on.
(In reply to Chris Dumez from comment #35) > Comment on attachment 426635 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426635&action=review > > >> Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:111 > >> + static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& slot) { slot.m_topOrigin = WebCore::SecurityOriginData(HashTableDeletedValue); } > > > > This isn’t quite right. What we want to do is to use placement new to put SecurityOriginData(HashTableDeletedValue) on top of m_topOrigin, not do an assignment to replace an existing value. We don't want to destroy the existing value that is there. The whole point of "constructDeletedValue" is that it makes a deleted value out of uninitialized memory, not on top of an already valid object. > > > > It would be better to get this right. There are lots of other examples of how to use the placement new syntax to do this correctly. > > Ok. I didn't know that. Looking at existing examples, seems like we want > this then? > ``` > static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& > slot) > { > new (NotNull, &slot) WebCore::ServiceWorkerRegistrationKey { > WebCore::SecurityOriginData { WTF::HashTableDeletedValue }, { } }; > } > ``` Or maybe even better: ``` static void constructDeletedValue(WebCore:: ServiceWorkerRegistrationKey& slot) { new (NotNull, &slot.m_topOrigin) WebCore::SecurityOriginData(WTF::HashTableDeletedValue); } ```
(In reply to Chris Dumez from comment #38) > static void constructDeletedValue(WebCore:: ServiceWorkerRegistrationKey& > slot) { new (NotNull, &slot.m_topOrigin) > WebCore::SecurityOriginData(WTF::HashTableDeletedValue); } Yes, that looks pretty good.
By the way, there may be some more modern way to do this with "emplace" or something like that; I didn’t really keep up with this aspect of C++ as it evolved, but I have some vague idea that there is a way to do this without using "new" in more modern code.
(In reply to Chris Dumez from comment #35) > Ok. I didn't know that. Looking at existing examples, seems like we want > this then? > ``` > static void constructDeletedValue(WebCore::ServiceWorkerRegistrationKey& > slot) > { > new (NotNull, &slot) WebCore::ServiceWorkerRegistrationKey { > WebCore::SecurityOriginData { WTF::HashTableDeletedValue }, { } }; > } > ``` The only place I've seen placement new used before is to implement boxed types in the GLib public APIs. One gotcha is that when the object is deallocated, its destructor will not be called, so you have to do that manually. I suppose we are fine with that never happening for HashTableDeletedValues. I have not seen a placement new with more than one argument before. I got curious and eventually found we have our own special placement new in StdLibExtras.h to assert that the location is non-null, which seems like a good idea. That was hard to find! > By the way, there may be some more modern way to do this with "emplace" or something like that; I didn’t really keep up with this aspect of C++ as it evolved, but I have some vague idea that there is a way to do this without using "new" in more modern code. Hm, I'm not sure about that. At least, I don't know how else to do it other than with placement new. emplace operations are defined for containers, e.g. std::vector::emplace_back is similar to push_back. It forwards its args to placement new, instead of copying like a push operation would do. Useful when you're constructing data and then immediately adding it to a container, but internally it is just doing placement new, and we aren't operating on a container in our example case here.
(In reply to Michael Catanzaro from comment #41) > One gotcha is that when the object is > deallocated, its destructor will not be called, so you have to do that > manually. I suppose we are fine with that never happening for > HashTableDeletedValues. That's right. The design of HashTable is that a deleted slot is never destroyed and is only constructed by calling the constructDeletedValue function. The thing you need to be able to do with a deleted slot is to return true from the isDeletedValue trait function and return false for actual valid constructed objects. > Hm, I'm not sure about that. You're right. I was wrong to think "emplace" had anything to do with this.
(In reply to Darin Adler from comment #42) > That's right. The design of HashTable is that a deleted slot is never > destroyed and is only constructed by calling the constructDeletedValue > function. The thing you need to be able to do with a deleted slot is to > return true from the isDeletedValue trait function and return false for > actual valid constructed objects. So with this in mind, I think Chris and I were both wrong about the value of fully-initializing the URL. Our solution for ServiceWorkerRegistrationKey using placement new is not going to be fully-initialized anyway, since we're now only initializing m_topOrigin.
(In reply to Chris Dumez from comment #38) > static void constructDeletedValue(WebCore:: ServiceWorkerRegistrationKey& > slot) { new (NotNull, &slot.m_topOrigin) > WebCore::SecurityOriginData(WTF::HashTableDeletedValue); } > ``` That only constructs the SecurityOriginData data member, though, leaving it in memory allocated for a ServiceWorkerRegistrationKey but not constructing a ServiceWorkerRegistrationKey. We probably need to construct a real ServiceWorkerRegistrationKey. I think this can be done by adding a constructor that takes HashTableDeletedValueType, as you originally suggested. Let's see....
Created attachment 426674 [details] Patch
Committed r276361 (236839@main): <https://commits.webkit.org/236839@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426674 [details].
Comment on attachment 426674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426674&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:79 > +inline ServiceWorkerRegistrationKey::ServiceWorkerRegistrationKey(WTF::HashTableDeletedValueType) > + : m_topOrigin(WTF::HashTableDeletedValue) > +{ > +} This is OK. However, here’s a note for the future: From a hash table implementation point of view, the goal when implementing constructDeletedValue is *not* to construct a valid object. All we want to do is to set a bit indicating that something is a deleted value. We would like to leave the rest of the object uninitialized, for performance reasons. The only operations that will be done on a "deleted value" in a slot are: 1) calls to the isDeletedValue function 2) call to the == function iff safeToCompareToEmptyOrDeleted is set in the hash function traits In particular: these objects are never destroyed, so it’s critical that the constructor not do anything that needs to be un-done. This means that often we want to do something like just set a single bit or a set of bits to a value that we know will never be set that way for any valid value. Thus "not initializing everything" is often a goal, not a bug. Initializing things creates a risk of allocating something that needs to be destroyed, and also means we are writing things we will never read, with a small but unnecessary performance cost. We sometimes do find it handy to implement the constructDeletedValue operation with a constructor, but it really has different requirements that most constructors. We don’t want to initialize things like vtable pointers, and we don’t want to do more than minimum initialization. So I’m not sure that the pattern of using a special constructor is necessarily going to be the best over time.
Comment on attachment 426674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426674&action=review > Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h:77 > +inline ServiceWorkerRegistrationKey::ServiceWorkerRegistrationKey(WTF::HashTableDeletedValueType) > + : m_topOrigin(WTF::HashTableDeletedValue) This unnecessarily initializes m_scope to a null URL. A more efficient approach is to not touch m_scope at all and we should consider that refinement in future.
<rdar://problem/77059533>
(In reply to Darin Adler from comment #48) > This unnecessarily initializes m_scope to a null URL. A more efficient > approach is to not touch m_scope at all and we should consider that > refinement in future. Bug #224975