Re: git: b40c0db6f6d6 - main - Patch up __diagused for when only one of INVARIANTS or WITNESS is defined

On 4/27/22, John Baldwin <jhb@freebsd.org> wrote:
> On 4/27/22 8:50 AM, Mateusz Guzik wrote:
>> On 4/27/22, John Baldwin <jhb@freebsd.org> wrote:
>>> On 4/27/22 6:30 AM, Mateusz Guzik wrote:
>>>> The branch main has been updated by mjg:
>>>>
>>>> URL:
>>>> https://cgit.FreeBSD.org/src/commit/?id=b40c0db6f6d61ed594118d81dc691b9263a7e4d7
>>>>
>>>> commit b40c0db6f6d61ed594118d81dc691b9263a7e4d7
>>>> Author:     Mateusz Guzik <mjg@FreeBSD.org>
>>>> AuthorDate: 2022-04-27 13:29:12 +0000
>>>> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
>>>> CommitDate: 2022-04-27 13:29:12 +0000
>>>>
>>>>       Patch up __diagused for when only one of INVARIANTS or WITNESS is
>>>> defined
>>>>
>>>>       Reported by:    John F Carr<jfc@mit.edu>
>>>> ---
>>>>    sys/sys/systm.h | 9 ++++++---
>>>>    1 file changed, 6 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/sys/sys/systm.h b/sys/sys/systm.h
>>>> index f2ffa7e6b815..6ca9ee886562 100644
>>>> --- a/sys/sys/systm.h
>>>> +++ b/sys/sys/systm.h
>>>> @@ -554,10 +554,13 @@ void _gone_in_dev(device_t dev, int major, const
>>>> char *msg);
>>>>    #define gone_in(major, msg)		__gone_ok(major, msg) _gone_in(major,
>>>> msg)
>>>>    #define gone_in_dev(dev, major, msg)	__gone_ok(major, msg)
>>>> _gone_in_dev(dev, major, msg)
>>>>
>>>> -#if defined(INVARIANTS) || defined(WITNESS)
>>>> -#define	__diagused
>>>> -#else
>>>> +#if !defined(INVARIANTS) && !defined(WITNESS)
>>>> +#define	__diagused	__unused
>>>> +#elif ((defined(INVARIANTS) && !defined(WITNESS)) || \
>>>> +	(!defined(INVARIANTS) && defined(WITNESS)))
>>>>    #define	__diagused	__unused
>>>> +#else
>>>> +#define	__diagused
>>>>    #endif
>>>
>>> Hmm, could this just be:
>>>
>>> #if defined(INVARIANTS) && defined(WITNESS)
>>> #define __diagused
>>> #else
>>> #define __diagused __unused
>>> #endif
>>>
>>
>> it does boil down to it and if you want to make the change I'm not
>> going to stand in the way, but then imo it should get a comment that
>> there is no dedicated macro for invariants or witness only so some
>> warnings are possibly silenced when they should not be
>>
>> however, the point here is that the case of only one of these being
>> defined is distinct from the rest and may warrant special treatment. I
>> wanted to preserve the distinction in the, arguably hairy,
>> conditional.
>
> The other option perhaps is to split out a separate __witness_used as
> I suspect the majority of __diagused cases are for INVARIANTS.  In a
> few other places where I didn't want to use bare __unused I added
> option-specific helpers (e.g. __usbdebug_unused).
>

Ye, that's I wished I did the first time around after the bug report :)

I just checked and the reported build failure is the only spot when
building GENERIC (without WITNESS) on amd64 so this may be perfectly
feasible.

-- 
Mateusz Guzik <mjguzik gmail.com>