[PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-11 Thread Mark Wielaard
On Sat, Sep 10, 2016 at 09:51:57AM -0400, Eric Gallager wrote: > On 9/10/16, Ian Lance Taylor wrote: > > I'm not sure about the patch to configure.ac/configure. The last I > > looked -Wshadow would warn if a local variable shadows a global > > variable. That can cause a pointless build break if

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-11 Thread Manuel López-Ibáñez
On 11/09/16 14:02, Mark Wielaard wrote: -Wshadow-local which warns if a local variable shadows another local variable or parameter, -Wshadow-compatible-local which warns if a local variable shadows another local variable or parameter whose type is compatible with that of the shadowing

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-11 Thread Mark Wielaard
e gcc itself). If you could try it on some and report what the rate of false positive is (plus maybe some examples to show whether one might or might not want to fix them anyway) then we can see if it makes sense. But I believe that should be done independent from introducing the new warnings themsel

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-12 Thread Eric Gallager
On 9/11/16, Manuel López-Ibáñez wrote: > On 11/09/16 14:02, Mark Wielaard wrote: >> -Wshadow-local which warns if a local variable shadows another local >> variable or parameter, >> >> -Wshadow-compatible-local which warns if a local variable shadows >> another local variable or parameter

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-12 Thread Jim Meyering
On Mon, Sep 12, 2016 at 7:05 AM, Eric Gallager wrote: > On 9/11/16, Manuel López-Ibáñez wrote: >> On 11/09/16 14:02, Mark Wielaard wrote: >>> -Wshadow-local which warns if a local variable shadows another local >>> variable or parameter, >>> >>> -Wshadow-compatible-local which warns if a lo

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-13 Thread Jason Merrill
On Mon, Sep 12, 2016 at 1:38 PM, Jim Meyering wrote: > On Mon, Sep 12, 2016 at 7:05 AM, Eric Gallager wrote: >> On 9/11/16, Manuel López-Ibáñez wrote: >>> On 11/09/16 14:02, Mark Wielaard wrote: -Wshadow-local which warns if a local variable shadows another local variable or parame

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-14 Thread Mark Wielaard
On Wed, 2016-09-14 at 00:00 -0400, Jason Merrill wrote: > I wonder about spelling the options as > -Wshadow={local,compatible-local} rather than with a dash, but > otherwise the patch looks fine. That is a much nicer way to write the option. But if I do that I would like to keep the old names as a

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-14 Thread Eric Gallager
On 9/14/16, Jason Merrill wrote: > On Mon, Sep 12, 2016 at 1:38 PM, Jim Meyering wrote: >> On Mon, Sep 12, 2016 at 7:05 AM, Eric Gallager >> wrote: >>> On 9/11/16, Manuel López-Ibáñez wrote: On 11/09/16 14:02, Mark Wielaard wrote: > -Wshadow-local which warns if a local variable shad

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-14 Thread Ian Lance Taylor
On Wed, Sep 14, 2016 at 1:30 AM, Mark Wielaard wrote: > On Wed, 2016-09-14 at 00:00 -0400, Jason Merrill wrote: >> I wonder about spelling the options as >> -Wshadow={local,compatible-local} rather than with a dash, but >> otherwise the patch looks fine. > > That is a much nicer way to write the o

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-09-14 Thread Mark Wielaard
On Wed, 2016-09-14 at 05:39 -0700, Ian Lance Taylor wrote: > On Wed, Sep 14, 2016 at 1:30 AM, Mark Wielaard wrote: > > On Wed, 2016-09-14 at 00:00 -0400, Jason Merrill wrote: > >> I wonder about spelling the options as > >> -Wshadow={local,compatible-local} rather than with a dash, but > >> otherw

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-10-24 Thread Jim Meyering
On Wed, Sep 14, 2016 at 5:49 AM, Mark Wielaard wrote: > On Wed, 2016-09-14 at 05:39 -0700, Ian Lance Taylor wrote: >> On Wed, Sep 14, 2016 at 1:30 AM, Mark Wielaard wrote: >> > On Wed, 2016-09-14 at 00:00 -0400, Jason Merrill wrote: >> >> I wonder about spelling the options as >> >> -Wshadow={loc

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-10-24 Thread Mark Wielaard
On Mon, Oct 24, 2016 at 04:26:49PM -0700, Jim Meyering wrote: > I have been using these new options (locally patched) to good effect. > While the vast majority of warning-triggering code has been > technically correct, using these has uncovered at least 4 or 5 real > bugs in code I care about. Awe

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-10-30 Thread Mark Wielaard
On Tue, Oct 25, 2016 at 01:57:09AM +0200, Mark Wielaard wrote: > I think the only thing "blocking" the patch from going in is that > nobody made a decission on how the actual warning option should be > named. I think the suggestion for -Wshadow=[global|local|compatible-local] > is the right one. Wi

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

2016-10-31 Thread Jason Merrill
OK. On Sun, Oct 30, 2016 at 3:53 PM, Mark Wielaard wrote: > On Tue, Oct 25, 2016 at 01:57:09AM +0200, Mark Wielaard wrote: >> I think the only thing "blocking" the patch from going in is that >> nobody made a decission on how the actual warning option should be >> named. I think the suggestion fo