On 11/04/18 12:13, Timothy Arceri wrote:
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.

Also for what its worth if we forced a minimum version of GCC 4.4 with mesa we could use #pragma GCC diagnostic ignored "-Wparentheses" for this type of thing. Unfortunately we seem forever stuck on 4.2 due to the decisions of one OS. Prior to 4.4 the pragma can only be applied at the function level.

[1] https://cgit.freedesktop.org/mesa/mesa/commit/?id=370e356ebab4885fc19b2b1d1de2816b6cd4dfc8






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

Reply via email to