Re: [PATCH v4 1/4] i18n: factorize more 'incompatible options' messages

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 1, 2022, 17:58 UTC
Message-ID: <xmqqee4mfozc.fsf@gitster.g>
In-Reply-To: <f8202cc7-be5c-5efa-2a3b-987114356c6b@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 14 quoted lines

>> Otherwise make it "static inline"?  Or just
>> 
>> #define die_for_incompatible_opt3(o1,n1,o2,n2,o3,n3) \
>> 	die_for_incompatible_opt4((o1), (n1), \
>> 				  (o2), (n2), \
>> 				  (o3), (n3), \
>> 				  0, "")
>> 
>> perhaps?
>
> Please no macros where they are not a clear advantage. Make it a
> function, either static inline, or out-of-line (the latter would be my
> personal preference in this case because the function is not called in a
> hot path).

In this particular case, my personal preference is actually a macro, since

 * there is no type safety lost

 * I find that it make it the most clear that

   - the opt3 variant is a mere convenience wrapper, which does not
     even deserve an entry in the symbol table, of the real thing, and

   - our intention is to keep them closely in sync by not even
     giving future developers a pair of { braces }, in which they
     are tempted into writing extra code before or after calling the
     opt4 variant and make them diverge.

But it is not very strong preference.

A "static inline" wrapper would result in the same code as a macro, and I'd be almost equally happy with it. The code is almost already written, and fixing it is just a matter of inserting "static " in front. My preference between "static inline" and macro is not strong enough to insist on rewriting ;-)

An out-of-line wrapper has a slight disadvantage that it is not immediately obvious that one is a mere wrapper of the other thing by just looking at what is in the *.h file, but I am OK with it as well. If the original patch were written as an out-of-line wrapper, my preference is not strong enough to insist on rewriting, either.

Thanks.