Re: Formal review request for Hash2

From: Peter Dimov (pdimov_at_[hidden])
Date: 2024-12-03 13:56:01

Alexander Grund wrote:
> Am 03.12.24 um 12:47 schrieb Ivan Matek via Boost:
> >> And you never do this
> >>
> >>      boost::hash2::hash_append( h, {}, v.x );
> >>      boost::hash2::hash_append( h, {}, v.y );
> >>      boost::hash2::hash_append( h, {}, v.z );
> >>
> >> You do this instead
> >>
> >>      boost::hash2::hash_append( h, f, v.x );
> >>      boost::hash2::hash_append( h, f, v.y );
> >>      boost::hash2::hash_append( h, f, v.z );
> >
> > Yes, but then again I am passing f multiple times. I don't want it to
> > sound like I am making a big deal out of this, I can just do something
> > like(and I actually use this idiom a lot) const auto append = [&]
> > (const auto& val){hash_append (h, f, val);} and use that "partially
> > applied" helper, in case I actually need to do hash_append multiple
> > times.
> I think there is space for such a helper:
>      boost::hash2::hasher append(h, f); // Or ...::hasher append(h) for default
> flavor
>      append(v.x);
>      append(v.y);
>      append(v.z);

The usual approach here is to make hash_append variadic, so that the
above becomes

    boost::hash2::hash_append( h, f, v.x, v.y, v.z );

I think that this is optimizing the wrong thing, and have never understood
the aversion to spelling this out (it's just two Ctrl+V keystrokes), but people
seem to prefer it strongly for some reason.

If you define your operator== as

return

    v1.x == v2.x &&
    v1.y == v2.y &&
    v1.z == v2.z;

(one member per line), it's better to make tag_invoke one call per line as
well, so that the two match visually and you can see the discrepancies.

And if you use the `auto rep()` idiom, there's no repetition needed in either.

Although, now that I think of it, if your operator== is

return std::tie(v1.x, v1.y, v1.z) == std::tie(v2.x, v2.y, v2.z);

then it would be better for tag_invoke to use a variadic hash_append, so
that the same (v1.x, v1.y, v1.z) list is used in both.

So I suppose I'll be adding the variadic overload, after all.