Re: [PATCH 22/348] Fix -Wsahdow warnings

Joel Brobecker
2011-11-23T20:16:35Z

> > Also, given the quantity, I'd suggest holding off until after 7.4 is branched.
> > [just a suggestion though]
>
> Heh, OTOH ...
> It will make backporting fixes into the 7.4 branch harder if a lot go
> in right after it's branched.
> That suggests holding off until 7.4 is close to going out.
>
> OTOOH, holding off too long will just make it harder for you to keep
> your patches up to date.
> But I wouldn't hold up 7.4 for these changes.
>
> In the end I think the high order bit is not making it too difficult
> to backport fixes into the 7.4 branch.

I agree that the priority is to make merging easy.

I'm also starting to question the benefits vs cost of enabling
-Wshadow. In particular, warnings that for us to change the name
of function parameters such as "block_found" or "index" make me feel
like this is going too far.

I think it's useful to review the warnings, because it did find some
real situation where we were shadowing another variable from an outer
scope. But I tend to disagree with a good portion of the warnings
I am seeing right now. Add the fact that the warnings also depend
on the host's system includes, and you might be as uncomfortable
as I am...

--
Joel