[Mesa-dev] [PATCH] fix gcc 8 parenthesis warning

[Timothy Arceri]

On 24/03/18 02:55, Eric Engestrom wrote:
> On Friday, 2018-03-23 08:09:46 -0700, Ian Romanick wrote:
>> On 03/23/2018 03:52 AM, Eric Engestrom wrote:
>>> On Friday, 2018-03-23 11:01:23 +0100, Marc Dietrich wrote:
>>>> fixes warnings like this:
>>>> [184/1137] Compiling C++ object 'src/compiler/glsl/glsl at sta/lower_jumps.cpp.o'.
>>>> In file included from ../src/mesa/main/mtypes.h:48,
>>>>                   from ../src/compiler/glsl_types.h:149,
>>>>                   from ../src/compiler/glsl/lower_jumps.cpp:59:
>>>> ../src/compiler/glsl/lower_jumps.cpp: In member function '{anonymous}::block_record {anonymous}::ir_lower_jumps_visitor::visit_block(exec_list*)':
>>>> ../src/compiler/glsl/list.h:650:17: warning: unnecessary parentheses in declaration of 'node' [-Wparentheses]
>>>
>>> This is gonna be a *very* annoying warning...
>>>
>>>>      for (__type *(__inst) = (__type *)(__list)->head_sentinel.next; \
>>>
>>> These parentheses are here for a reason: to make sure we can't pass in
>>> something that would break the code or give it an unexpected behaviour.
>>
>> This is usually to avoid things with side effects.  Would anything with
>> a side-effect even compile here?  Or is there some other case that I'm
>> missing?
> 
> I can't think of anything right now off the top of my head, so I'll
> leave my vote blank for now.

I've switched to Fedora 28 on one of my machines and this warning is 
very annoying because I pops up everywhere the list is used. If there 
are no objections I'm going to push this patch in a couple of days.


> 
>>
>>> I would be inclined to NAK this patch and request we kill this warning
>>> at build system level instead.
>>> Shame when compilers self-sabotage like that :/
>>
>> This is very annoying because -Wparentheses gives useful warnings when
>> you don't have () but should. :(
> 
> I know, this is what I mean by self-sabotage: they introduce a useful
> warning, then later add a bunch of spam to it for arguably no reason,
> forcing devs to turn it off to be able to still see the other warnings.
> 
> IMO the new warning can be useful, but should've been off by default
> (ie. not in -Wall) and turned on via a new flag, instead of hijacking
> the existing one.
> 
>>
>>>>                   ^
>>>> ../src/compiler/glsl/lower_jumps.cpp:510:7: note: in expansion of macro 'foreach_in_list'
>>>>         foreach_in_list(ir_instruction, node, list) {
>>>>         ^~~~~~~~~~~~~~~
>>>>
>>>> Signed-off-by: Marc Dietrich <marvin24 at gmx.de>
>>>> ---
>>>>   src/compiler/glsl/list.h | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/compiler/glsl/list.h b/src/compiler/glsl/list.h
>>>> index f77fe12991..2bfa273554 100644
>>>> --- a/src/compiler/glsl/list.h
>>>> +++ b/src/compiler/glsl/list.h
>>>> @@ -647,12 +647,12 @@ inline void exec_node::insert_before(exec_list *before)
>>>>   #endif
>>>>   
>>>>   #define foreach_in_list(__type, __inst, __list)      \
>>>> -   for (__type *(__inst) = (__type *)(__list)->head_sentinel.next; \
>>>> +   for (__type *__inst = (__type *)(__list)->head_sentinel.next; \
>>>>           !(__inst)->is_tail_sentinel();               \
>>>>           (__inst) = (__type *)(__inst)->next)
>>>>   
>>>>   #define foreach_in_list_reverse(__type, __inst, __list)   \
>>>> -   for (__type *(__inst) = (__type *)(__list)->tail_sentinel.prev; \
>>>> +   for (__type *__inst = (__type *)(__list)->tail_sentinel.prev; \
>>>>           !(__inst)->is_head_sentinel();                    \
>>>>           (__inst) = (__type *)(__inst)->prev)
>>>>   
>>>> -- 
>>>> 2.16.2
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>