Re: [PATCH 3/6] hash.h: introduce `unsafe_hash_algo()`

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Nov 22, 2024, 20:37 UTC
Message-ID: <Z0DrfjZ_ktEwwE1w@tapette.crustytoothpaste.net>
In-Reply-To: <20241122082523.GB1179306@coredump.intra.peff.net>

On 2024-11-22 at 08:25:23, Jeff King wrote:

Show 7 quoted lines

> OK, here's a quick and dirty conversion that applies on top of Taylor's
> patches 1 and 2 (and then patches 3 and on would need to adjust to the
> new world order).
> 
> There are quite a lot of spots that needed to be adjusted, but the most
> part it was just s/->\([a-z]\)_fn/->fn.\1/.  (I'll leave adjusting the
> unsafe_ variants as an exercise for the reader).

Yeah, I saw that there were about 50 callers of the hash functions, which on one hand isn't too many to change, but on the other hand also surprised me. I don't remember there being that many callers when I did the SHA-256 work, but it could be that I'm remembering poorly.

Show 5 quoted lines

> The naming convention was just what I made up, but one nice effect is
> that it replaces "_fn" with "fn.", so the line lengths remain the same.
> :) It does mean there's a function called "final", which is a keyword in
> C++. I don't know if that matters to us or not (I feel like we were
> trying to avoid those, but it appears as a function elsewhere, too).

It doesn't. I think we had one contributor trying to make the code compile as C++ at one point, but it definitely doesn't now. A quick `git grep` show that we have lots of variables named `new` as well. (In addition, the implicit cast of `void *`, such as from `malloc`, to a pointer of any type is not allowed without a cast in C++, so that would need to be handled, too.)

> So I dunno. It is a one-time pain, but I think the result harmonizes the
> type system with the concept better.

Yeah, I agree. It definitely looks like it could mostly be done with a Perl or Ruby script, even if slightly inconvenient. And if there is a conflict, it should be easy to fix up.

-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA