Re: [PATCH 2/5] roaring.[ch]: apply Git specific changes to the roaring API

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 21, 2022, 16:58 UTC
Message-ID: <xmqq5yhg64vn.fsf@gitster.g>
In-Reply-To: <990f84f9-fdd9-0d0a-4fc0-d0dbd19ee5a9@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 10 quoted lines

>> Hmph.... could "a typedef can become a place to give definitive
>> documentation for the class of callback functions" be a good reason
>> why we would want one?  I dunno.
>> 
>> In the posted patch, readers cannot tell what kind of three
>> parameters they are supposed to give to write_fn().
>
> This is exactly my reasoning. Having a clear definition gives us an
> opportunity to document what each parameter is for, even if it is
> just a variable name.

Yeah, I completely agree with you on that line of reasoning.

> In the unlikely event that we needed to modify this callback
> signature, changing it in one place makes it clear that we cover
> all connected uses instead of tracking all of these anonymous
> functions across multiple methods.

Well, the compiler will help flagging a caller that forgot to convert, even if there is no typedef.

The parameter list may not have to be updated for a function that takes a callback function of this type as its parameter if you did not use a typedef, but where it in turn makes a call to that callback function, the compiler will notice the argument mismatch, which you have to adjust to the new calling convention anyway.

What is somewhat sad is that even with typedef, the implementation of a callback function cannot use the type, but the longhand that underlies the typedef, to define it, but that is not something we can fix, or are interested in fixing ;-).

Thanks.