On Tue, Apr 26, 2016 at 09:03:03AM -0600, Jeff Law wrote:
> On 04/26/2016 07:08 AM, Jakub Jelinek wrote:
> >Hi!
> >
> >I've noticed a warning during bootstrap:
> >../../gcc/reorg.c: In function ‘void try_merge_delay_insns(rtx_insn*, 
> >rtx_insn*)’:
> >../../gcc/reorg.c:1431:12: warning: name lookup of ‘i’ changed
> >       for (i = 0; i < XVECLEN (PATTERN (insn), 0); i++)
> >            ^
> >../../gcc/reorg.c:1263:7: warning:   matches this ‘i’ under ISO standard 
> >rules
> >   int i, j;
> >       ^
> >../../gcc/reorg.c:1413:25: warning:   matches this ‘i’ under old rules
> >       for (unsigned int i = len - 1; i < len; i--)
> >                         ^
> >It is not fatal, but still ugly.  The problem is that the function has
> >  int i;
> >...
> >  for (i = 0; ...)
> >...
> >  for (unsigned int i = ... )
> >...
> >  for (i = 0; ...)

So pre ISO C++ gave the second decl the same scope as the first one?
that's... exciting ;)

> >This patch just declares the var in the only affected loop, so that the 
> >warning
> >is not emitted, unless we start checking -Wshadow warnings, I think this is
> >good enough.
> >
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> >2016-04-26  Jakub Jelinek  <ja...@redhat.com>
> >
> >     * reorg.c (try_merge_delay_insns): Declare i inside the last
> >     for loop to avoid warning.
> I'd like to declare this in the "obviously OK" category for future changes
> of a similar nature.  But I don't think we can because in the general case
> the value from the loop may be used outside the loop.

 If we enabled -Wshadow first then you could use the compiler to prove
 the transformation correct, but I have no idea how much effort enabling
 -Wshadow would be.

> Makes me wonder if a plugin could help identify the obvious cases where the
> value from the loop isn't used outside the loop, thus allowing someone to
> quickly convert many of these things with a small effort.

Yeah, I think it would generally be great to sync declarations to there
first use, but I thought some people didn't like that idea because it
made backporting hard for them?

Trev

> 
> jeff
> 

Reply via email to