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 >
