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 >