[Mesa-dev] [PATCH 1/7] util: add MAYBE_UNUSED for config dependent variables

[Ian Romanick]

On 04/19/2016 12:47 PM, Emil Velikov wrote:
> On 19 April 2016 at 20:35, Ian Romanick <idr at freedesktop.org> wrote:
>> On 04/19/2016 12:32 PM, Chad Versace wrote:
>>> On Tue 19 Apr 2016, Ian Romanick wrote:
>>>> On 04/19/2016 12:15 PM, Chad Versace wrote:
>>>>> On Tue 19 Apr 2016, Emil Velikov wrote:
>>>>>> On 19 April 2016 at 19:38, Ian Romanick <idr at freedesktop.org> wrote:
>>>>>>> On 04/19/2016 07:48 AM, Emil Velikov wrote:
>>>>>>>> On 18 April 2016 at 19:01, Ian Romanick <idr at freedesktop.org> wrote:
>>>>>>>>> On 04/18/2016 08:06 AM, Emil Velikov wrote:
>>>>>>>>>> On 18 April 2016 at 04:43, Francisco Jerez <currojerez at riseup.net> wrote:
>>>>>>>>>>> Grazvydas Ignotas <notasas at gmail.com> writes:
>>>>>
>>>>> FYI, systemd uses a nice convention for attributes that is nicer on the
>>>>> eyes than ALL_CAPS and doesn't violate the standard with
>>>>> double-underscore. systemd declares attributes as _lower_case_.
>>>>>
>>>>> For example:
>>>>>
>>>>>   #define _maybe_unused_ __attribute__((unused))
>>>>
>>>> That wouldn't be terrible.  If there is general support changing, we
>>>> should change all of everything at once... and document the new practice.
>>>>
>>>> My goal previously in the thread is to explain how and why we got to
>>>> where we are... and how that supports the patch as is. :)
>>>
>>> I also support that patch as-is. But...
>>>
>>> MAYBE_UNUSED tells you the variable is *unimportant* by YELLING. It
>>> *accentuates* the unimportant rather than diminishing it.
>>>
>>> I prefer to reserve ALL_CAPS for items that need TO SCREAM AND GET YOUR
>>> ATTENION; things like FIXME, TODO, and macros like MAX() that suffer
>>> from multiple-expansion. Using ALL_CAPS for innocuous items like an
>>> "unused" attribute hurts my eyes, and distracts me from more important
>>> things in the code.
>>
>> That's fair, and I don't disagree.  I think I set the acceptance
>> criteria.  Patches welcome. :D
>>
> Strange. Your earlier replies read exactly the opposite - like you
> wanted to have all the SHOUTING. I guess for the future I'll start
> switching the caps lock. It seems to help a getting point across.

I think we've learned a hard lesson over the years that gradually
changing style patch-by-patch sucks.  The code base should, above all
else, be self consistent.  Otherwise people copy style from the wrong
thing, and the whole thing turns into a mess.  To that end, individual
patches should maintain the status quo.

If we decide to change a point of style, we should

1. Document it.

2. Update the whole code base.

I didn't always feel this way, but the mixing and matching has just
gotten so irritating.  We picked the old method for two reasons.

- It's irritating to cherry-pick patches back across style changes.  I
think this is probably less irritating than what we have now.

- It can be a lot of work to update the style of the whole code base.  I
think tools exist now so that most changes can be automated.  This isn't
always the case.

> Also there are many other macros that we're missing fro MSVC - packed,
> printf, constructor... Some of their semantics do differ from the gcc
> version, so I'm not surprised that people did not try getting them in,
> considering the length/direction this discussion turned out.
> 
> Emil
> P.S. I'm not trying to be cheeky here, just pointing out why sometimes
> things end up in /dev/null