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

[Emil Velikov]

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:
>>>
>>>> On Sun, Apr 17, 2016 at 2:50 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>> On 16 April 2016 at 02:00, Grazvydas Ignotas <notasas at gmail.com> wrote:
>>>>>> This is mostly for variables that are only used in asserts and cause
>>>>>> unused-but-set-variable warnings in release builds. Could just use
>>>>>> UNUSED directly, but MAYBE_UNUSED should be less confusing and is
>>>>>> similar to what the Linux kernel has.
>>>>>>
>>>>>> And yes __attribute__((unused)) can be used on variables on both GCC 4.2
>>>>>> (oldest supported by mesa) and clang 3.0 (just some random old version,
>>>>>> nut sure what's the minimum for mesa).
>>>>>>
>>>>>> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
>>>>>> ---
>>>>>> I have no commit access, if this patch is ok, please someone push.
>>>>>>
>>>>>>  src/util/macros.h | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/src/util/macros.h b/src/util/macros.h
>>>>>> index 0c8958f..f081bb8 100644
>>>>>> --- a/src/util/macros.h
>>>>>> +++ b/src/util/macros.h
>>>>>> @@ -204,6 +204,8 @@ do {                       \
>>>>>>  #define UNUSED
>>>>>>  #endif
>>>>>>
>>>>>> +#define MAYBE_UNUSED UNUSED
>>>>>> +
>>>>> Hell yeah !
>>>>>
>>>>> A thing that comes to mind ... a while back we've been wondering about
>>>>> (re)naming these just the the way we do in the kernel. Namely
>>>>> __maybe_unused in this case. Can you give that one a try and link a
>>>>> branch.
>>>>
>>>> I hope you mean something like this?
>>>> https://github.com/notaz/mesa/commits/warnings
>>>>
>>>
>>> You guys know that in standard C any identifier (including a
>>> preprocessor define) starting with double underscore is reserved for the
>>> implementation and causes the program's behavior to be undefined?  The
>>> kernel is kind of part of the implementation so they can do whatever
>>> they want, but it seems dubious to do the same in a userspace library.
>>> How about you use a single (or no) underscore if people prefer the
>>> lowercase spelling?
>>>
>> I do recall this. As mentioned before (to Jose I believe) sadly the
>> cat is out of the bag. Both within mesa itself and also in other
>> projects.
>>
>> IIRC the conclusion from last time was along the lines of "[I guess]
>> it's ok if it does not break things". So far scons (linux + windows)
>> look fine - autoconf's normal make looks ok as well (make check takes
>> a while) ;-)
>>
>> Personally I'd opt for consistency across projects. It gives us extra
>> reassurance that things are unlikely to break under our feet.
>> Although yes, it does suck (a bit) that thing have turned out that way.
>
> I agree with Curro.  I'd rather have consistency within Mesa (where
> we've always used ALL_THE_UPPERCASE) than across projects.
>
Either I'm loosing the plot or that hasn't been true for a while. Take
a look at src/util/macros.h and alike.

That said, I feel that this might turn into bikeshedding pretty quick,
so if the above is not enough [to change your mind] so be it.

Emil