Re: [Mesa-dev] building with -Wshadow
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
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
Re: [Mesa-dev] building with -Wshadow
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
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
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
[Mesa-dev] building with -Wshadow
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
