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@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 <marvi...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to