Jeff King <peff@xxxxxxxx> writes:
> On Thu, Aug 29, 2024 at 11:27:39AM -0700, Junio C Hamano wrote:
>
>> Just like we only define UNUSED macro when __GNUC__ is defined,
>> and fall back to an empty definition otherwise, we should do the
>> same for MAYBE_UNUSED.
>>
>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>> ---
>> * Before I forget that we have discussed this, just as a
>> documentation (read: this is not a patch to be applied).
>>
>> I think this only matters when a compiler satisfies all three
>> traits:
>>
>> - does not define __GNUC__
>> - does have its own __attribute__() macro
>> - barfs on __attribute__((__unused__))
>>
>> Otherwise we will define __attribute__(x) away to empty to cause
>> no harm.
>>
>> Since we have survived without complaints without such a guard
>> for quite some time, it may be a sign that no compiler that knows
>> __attribute__() that people ever tried to compile Git with barfs
>> with __attribute__((__unused__)). I dunno.
>
> Yeah, I was surprised that this didn't have a guard and was not
> currently barfing on other compilers. And the answer is that we already
> turn __attribute__ into a noop on non-GNUC platforms.
Plus these non-GNUC platforms either
(1) do not have their own __attribute__, which lets us turn
__attribute__() into noop, or
(2) have their own __attribute__, but they happen to support
__attribute__((__unused__)).
If somebody has __attribute__() and does not support (__unused__) in
it, use of MAYBE_UNUSED would be broken (maybe their __attribute__()
supports other things but not unused).
> Which made me wonder if UNUSED really needs its guards. It does, because
> it is defined early in the file, before the __attribute__ handling. I
> don't think we want to move it down, since it needs to be available for
> use by inline'd compat wrappers. But arguably we should move the
> attribute macro earlier in the file?
And moving __attribute__ definition earlier in the file would not
help such a platform with broken __attribute__((__unused__))
> I don't know that it is really worth spending too much time futzing
> with, though.
I am inclined to think it is not. So let's scrap the patch. The
list archive will hopefully remember when it becomes necessary ;-)
