Subject: Re: [PATCH 8/6] CodingGuidelines: also mention MAYBE_UNUSED

From: Junio C Hamano <[email-masked]>
Date: Thu, 29 Aug 2024 11:06:22 -0700

Jeff King <[email-masked]> writes:

>> +/*
>> + * MAYBE_UNUSED marks a function parameter that may be unused, but
>> + * whose use is not an error.
>> + *
>> + * Depending on a configuration, all uses of a function parameter may
>> + * become #ifdef'ed away.  Marking such a parameter with UNUSED would
>> + * give a warning in a compilation where the parameter is indeed used,
>> + * and not marking such a parameter would give a warning in a
>> + * compilation where the parameter is unused.
>> + */
>>  #define MAYBE_UNUSED __attribute__((__unused__))
>
> This is all good as pertains to function parameters. But the original
> reason we added MAYBE_UNUSED was actually for static functions that were
> auto-generated by the commit-slab macros. Saying "...marks a function
> parameter" implies to me that it's the only use. I don't know if we want
> to be more expansive here or not. Adding auto-generated macro functions
> should be quite a rarity, I'd think.

True.  You can annotate types, variables, and functions with the
attributes as well.  How about saying something like this

    MAYBE_UNUSED marks a function parameter that may be unused but
    whose use is not an error.  It also can be applied to functions,
    types and variables.

and then keep the explanation of why you may want to use the maybe-
variant as-is, using a function parameter as an example?  Or I could
rewrite "parameter" and "function parameter" in it with "thing"
(with double quotes around), like:

    Depending on a configuration, all uses of a "thing" may become
    #ifdef'ed away....

Unlike the use of deprecated attribute, our definition of
MAYBE_UNUSED is not guarded with anything.  Shouldn't we at least do

    #if defined(__GNUC__)
    #define MAYBE_UNUSED __attribute__((__unused__))
    #else
    #define MAYBE_UNUSED /* noop */
    #endif

or something, by the way?

Thanks.