[Mesa-dev] Add an ASSERTED macro to use in place of MAYBE_UNUSED?

[Matt Turner]

On Mon, Apr 22, 2019 at 1:09 PM Kristian Høgsberg <hoegsberg at gmail.com> wrote:
>
> On Mon, Apr 22, 2019 at 12:11 PM Jason Ekstrand <jason at jlekstrand.net> wrote:
> >
> > All,
> >
> > I've seen discussions come up several times lately about whether you should use MAYBE_UNUSED or UNUSED in what scenario and why do we have two of them anyway.  That got me thinking a bit.  Maybe what we actually want instead of MAYBE_UNUSED is something like this:
> >
> > #ifdef NDEBUG
> > #define ASSERTED UNUSED
> > #else
> > #define ASSERTED
> > #endif
> >
> > That way, if you only need a parameter for asserts, you can declare it ASSERTED and it won't warn in release builds will still throw a warning if you do a debug build which doesn't use it.  Of course, there are other times when something is validly MAYBE_UNUSED such as auto-generated code or the genX code we use on Intel.  However, this provides additional meaning and means the compiler warnings are still useful even after you've relegated a value to assert-only.
> >
> > Thoughts?  I'm also open to a better name; that's the best I could do in 5 minutes.
>
> I think that's going in the wrong direction - if anything I think that
> having both UNUSED and MAYBE_UNUSED is redundant and feel that just
> UNUSED would be fine. __attribute__((unused)) doesn't mean "strictly
> not used", it means "don't warn if this isn't used".

I agree that having both UNUSED and MAYBE_UNUSED is silly and I would
be happy to see MAYBE_UNUSED go away.

I think the advantage of Jason's proposal is that we are alerted if
there is actually dead code. E.g., if we remove the assert that used a
variable, we currently won't get a warning from the compiler that the
variable is unused. At least in release builds we would, if we did
what Jason suggests.

Maybe we do what Jason suggests and then remove MAYBE_UNUSED?