Make hasher work with tuple-like classes
Created attachment 404063 [details] Patch
Created attachment 404065 [details] Patch
Comment on attachment 404065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404065&action=review Thanks for finishing the job. You can tell from all the FIXME that I tried this and couldn’t figure it out. Where’s the part of this patch where you use this in Color? > Source/WTF/ChangeLog:25 > + adhear to WebKit style as IsTypeComplete<>. adhere > Source/WTF/wtf/StdLibExtras.h:424 > +// Based on 'Detecting in C++ whether a type is defined, part 3: SFINAE and incomplete types' > +// <https://devblogs.microsoft.com/oldnewthing/20190710-00/?p=102678> > +template<typename, typename = void> constexpr bool IsTypeComplete = false; > +template<typename T> constexpr bool IsTypeComplete<T, std::void_t<decltype(sizeof(T))>> = true; Why rename IsTypeComplete? If we’re just previewing C++20, could we use the real name and inject in the std namespace? Or maybe this is the way to go.
(In reply to Darin Adler from comment #3) > Comment on attachment 404065 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=404065&action=review > > Thanks for finishing the job. You can tell from all the FIXME that I tried > this and couldn’t figure it out. > > Where’s the part of this patch where you use this in Color? Since I have not landed that one yet (still needs a final r+), I figured I land this first, and land the fix for WTF first. > > > Source/WTF/ChangeLog:25 > > + adhear to WebKit style as IsTypeComplete<>. > > adhere > > > Source/WTF/wtf/StdLibExtras.h:424 > > +// Based on 'Detecting in C++ whether a type is defined, part 3: SFINAE and incomplete types' > > +// <https://devblogs.microsoft.com/oldnewthing/20190710-00/?p=102678> > > +template<typename, typename = void> constexpr bool IsTypeComplete = false; > > +template<typename T> constexpr bool IsTypeComplete<T, std::void_t<decltype(sizeof(T))>> = true; > > Why rename IsTypeComplete? If we’re just previewing C++20, could we use the > real name and inject in the std namespace? Or maybe this is the way to go. I don't think it's a c++20 thing, just a useful trait that Raymond Chen blogged about. Thanks.
Committed r264270: <https://trac.webkit.org/changeset/264270>
<rdar://problem/65410178>
Re-opened since this is blocked by bug 214228
Reverting due to broken build, failure details in Slack.
Created attachment 404090 [details] Patch + `constexpr` => `inline constexpr` template variable fix
(In reply to Yusuke Suzuki from comment #9) > Created attachment 404090 [details] > Patch + `constexpr` => `inline constexpr` template variable fix Internal one is built with this fix.
Committed r264277: <https://trac.webkit.org/changeset/264277>
Comment on attachment 404065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404065&action=review I've relanded the patch with `inline constexpr` fix, and note the rationale why this is necessary here :) > Source/WTF/wtf/Hasher.h:110 > +template<typename, typename = void> constexpr bool HasBeginFunctionMember = false; > +template<typename T> constexpr bool HasBeginFunctionMember<T, std::void_t<decltype(std::declval<T>().begin())>> = true; We need `inline constexpr` to make this template-variable inline-variable (C++ is super complicated!) https://stackoverflow.com/questions/14391272/does-constexpr-imply-inline > Source/WTF/wtf/Hasher.h:129 > +template<typename T> constexpr bool HasGetFunctionMember<T, std::void_t<decltype(std::declval<T>().template get<0>())>> = true; Ditto. Work-around with `inline constexpr`. >> Source/WTF/wtf/StdLibExtras.h:424 >> +template<typename T> constexpr bool IsTypeComplete<T, std::void_t<decltype(sizeof(T))>> = true; > > Why rename IsTypeComplete? If we’re just previewing C++20, could we use the real name and inject in the std namespace? Or maybe this is the way to go. Ditto :)
Comment on attachment 404065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404065&action=review >> Source/WTF/wtf/Hasher.h:110 >> +template<typename T> constexpr bool HasBeginFunctionMember<T, std::void_t<decltype(std::declval<T>().begin())>> = true; > > We need `inline constexpr` to make this template-variable inline-variable (C++ is super complicated!) > https://stackoverflow.com/questions/14391272/does-constexpr-imply-inline This is the first time I have ever heard of an inline variable! >>> Source/WTF/wtf/StdLibExtras.h:424 >>> +template<typename T> constexpr bool IsTypeComplete<T, std::void_t<decltype(sizeof(T))>> = true; >> >> Why rename IsTypeComplete? If we’re just previewing C++20, could we use the real name and inject in the std namespace? Or maybe this is the way to go. > > Ditto :) Sam clarified that is_type_complete is not in C++20.
Comment on attachment 404065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404065&action=review >>> Source/WTF/wtf/Hasher.h:110 >>> +template<typename T> constexpr bool HasBeginFunctionMember<T, std::void_t<decltype(std::declval<T>().begin())>> = true; >> >> We need `inline constexpr` to make this template-variable inline-variable (C++ is super complicated!) >> https://stackoverflow.com/questions/14391272/does-constexpr-imply-inline > > This is the first time I have ever heard of an inline variable! inline variable is one of the best feature in C++17! Sometimes we, C++ developers, saw super weird linker errors about `static XXX::value` variables defined in the header because, 1. One translation unit is accessing it via reference. 2. One translation unit is building it as a constant value and do not generate a data for that. Then linker causes link failures because (1) cannot find the referenced data. Inline variable solves this problem by ensuring that this variable is not referenced like (1).