Re: [Mesa-dev] building with -Wshadow

2016-02-29 Thread Emil Velikov 
On 10 February 2016 at 18:50, Ian Romanick  wrote:
> On 02/10/2016 10:48 AM, Ian Romanick wrote:
>> On 02/09/2016 08:43 PM, Dave Airlie wrote:
>>> I was just messing with warning flags on virglrenderer and noticed
>>> -Wshadow generated a fair few warnings with gallium, so I did a mesa
>>> build with -Wshadow enabled and it was fairly messy,
>>>
>>> do we care? there could be bugs hiding in -Wshadow land.
>>
>> I have mixed feelings about -Wshadow.  In principle, it's a good warning
>> to use, and it can catch real bugs.  However, thanks to standard library
>> functions with jackass names like "index", the signal-to-noise ratio
>> chases even me away. :(
>
> Though acording to the link Emil sent, GCC 4.8 may have fixed this.  Yay!
>
Almost... we're either abusing the C++ standard or GCC/GXX (5.2.0
here) still needs some tweaks.
If we enable the warning for both gcc and g++ we get around 5891
warnings and ~100 (iirc) for gcc only.

So g++ is out of the question, whereas for gcc ... maybe.

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

Re: [Mesa-dev] building with -Wshadow

2016-02-10 Thread Emil Velikov 
Hi Dave,

On 10 February 2016 at 04:43, Dave Airlie  wrote:
> I was just messing with warning flags on virglrenderer and noticed
> -Wshadow generated a fair few warnings with gallium, so I did a mesa
> build with -Wshadow enabled and it was fairly messy,
>
> do we care? there could be bugs hiding in -Wshadow land.
>
Huge thanks for bringing this up.

TL;DR; Yes please :-)

Longer version: Ideally we'll be using (almost) the same flags as
libdrm and xserver at some point. Note that older versions of GCC are
likely to cause a few annoying warnings [1] here and there. Worst case
scenario, we can toggle it off inline in the places where people feel
unhappy with the fix.

-Emil

[1] http://stackoverflow.com/questions/2958457/gcc-wshadow-is-too-strict
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] building with -Wshadow

2016-02-10 Thread Roland Scheidegger 
Am 10.02.2016 um 05:43 schrieb Dave Airlie:
> I was just messing with warning flags on virglrenderer and noticed
> -Wshadow generated a fair few warnings with gallium, so I did a mesa
> build with -Wshadow enabled and it was fairly messy,
> 
> do we care? there could be bugs hiding in -Wshadow land.
> 
> Dave.

I never heard of that option until now ;-).
I suppose in general it would be good coding style not to reuse the same
variable names, but maybe there's more to it...

Roland


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

Re: [Mesa-dev] building with -Wshadow

2016-02-10 Thread Ian Romanick 
On 02/09/2016 08:43 PM, Dave Airlie wrote:
> I was just messing with warning flags on virglrenderer and noticed
> -Wshadow generated a fair few warnings with gallium, so I did a mesa
> build with -Wshadow enabled and it was fairly messy,
> 
> do we care? there could be bugs hiding in -Wshadow land.

I have mixed feelings about -Wshadow.  In principle, it's a good warning
to use, and it can catch real bugs.  However, thanks to standard library
functions with jackass names like "index", the signal-to-noise ratio
chases even me away. :(

I'm more than happy to review patches that fix -Wshadow issues, but,
unless the noise of old has been solved, I probably won't build with it
on a regular basis.

> Dave.
> ___
> 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

Re: [Mesa-dev] building with -Wshadow

2016-02-10 Thread Ian Romanick 
On 02/10/2016 10:48 AM, Ian Romanick wrote:
> On 02/09/2016 08:43 PM, Dave Airlie wrote:
>> I was just messing with warning flags on virglrenderer and noticed
>> -Wshadow generated a fair few warnings with gallium, so I did a mesa
>> build with -Wshadow enabled and it was fairly messy,
>>
>> do we care? there could be bugs hiding in -Wshadow land.
> 
> I have mixed feelings about -Wshadow.  In principle, it's a good warning
> to use, and it can catch real bugs.  However, thanks to standard library
> functions with jackass names like "index", the signal-to-noise ratio
> chases even me away. :(

Though acording to the link Emil sent, GCC 4.8 may have fixed this.  Yay!

> I'm more than happy to review patches that fix -Wshadow issues, but,
> unless the noise of old has been solved, I probably won't build with it
> on a regular basis.
> 
>> Dave.
> ___
> 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] building with -Wshadow

2016-02-09 Thread Dave Airlie 
I was just messing with warning flags on virglrenderer and noticed
-Wshadow generated a fair few warnings with gallium, so I did a mesa
build with -Wshadow enabled and it was fairly messy,

do we care? there could be bugs hiding in -Wshadow land.

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