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

[Eric Engestrom]

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 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
> > 
>