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

From: Junio C Hamano <[email-masked]>
Date: Tue, 01 Feb 2022 09:58:31 -0800

Johannes Sixt <[email-masked]> writes:

>> 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.