We iterated the patched on the force: https://forge.sourceware.org/gcc/gcc-TEST/pulls/54/files The version present there now LGTM.
On Mon, Jun 9, 2025 at 12:16 PM Tomasz Kaminski <tkami...@redhat.com> wrote: > > > On Fri, Jun 6, 2025 at 9:07 AM Giuseppe D'Angelo < > giuseppe.dang...@kdab.com> wrote: > >> Hi Tomasz, >> >> Thank you for reviewing the original patch! >> >> I'm attaching a second version, hopefully addressing what you've >> highlighed. I've also pushed it on Forge: >> >> https://forge.sourceware.org/gcc/gcc-TEST/pulls/52 > > Posted review there, but major change is, for type that are wrappers over > integer: > * use hash<Integer> to hash a value, instead of jus returning it > * specialize __is_fash_hash as true for them. > >> >> >> >> On 24/04/2025 15:30, Tomasz Kaminski wrote: >> > Hi, >> > >> > I am reattaching the original patch below, as I wasn't on the mailing >> > list when it was sent. >> > Thank you for submitting the patch and apologies for the late response. >> > >> > The major comment I have is that these are new C++26 classes, so we can >> > use requires __is_hash_enabled_for<_Tp> and define only enabled >> > specialization. In other cases we will be using a primary template that >> > is disabled. >> > >> > Also, argument_type and result_type typedefs are no longer present >> since >> > C++20, so you can remove them, and also remove __hash_base base classes >> > that were responsible for providing them. >> >> Sounds good to me, I've done this cleanup. >> >> > + >> > > + template<typename _Arg, typename... _Args> >> > > + static size_t __hash(const _Arg& __arg, const _Args&... >> __args) >> > > + { >> > > + const size_t __arg_hash = hash<_Arg>{}(__arg); >> > > + size_t __result = _Hash_impl::hash(__arg_hash); >> > ## COMMENT >> > I think you can implement this using fold expression, as: >> > (_Hash_impl::__hash_combine(hash<_Arg>{}(__args), __result), ...); >> >> Ok, I just wanted to make it work on C++11, but there's no real need for >> that I guess... >> >> > > + __hash_combine(__result, __args...); >> > > + return __result; >> > > + } >> > > + }; >> > > +#endif // C++11 >> > > + >> >> Thank you, >> >> -- >> Giuseppe D'Angelo >> >
