Re: [eigen] g++ -Wshadow warnings

[Christoph Hertzberg <chtz@xxxxxxxxxxxxxxxxxxxxxxxx>]

On 26.02.2014 17:54, Marcus D. Hanwell wrote:
> On Wed, Feb 26, 2014 at 7:38 AM, Christoph Hertzberg
> <chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > On 26.02.2014 13:34, Jitse Niesen wrote:
> > > On Mon, 24 Feb 2014, Brad Bell wrote:
> > >
> > > > Does eigen wish to support the -Wshadow flag on the g++ compiler ?
> > >
> > >
> > > Yes, why not.
> >
> >
> > I just (locally) added
> >    ei_add_cxx_compiler_flag("-Wshadow")
> > to the main CMakeList. At the moment, compiling the tests floods the screen
> > with warnings. I don't think any of these will be hard to fix, but I guess
> > we should postpone that to after 3.2.1.
> It would be great to see these fixed up, we currently suppress the
> errors from Eigen. If it slips I will see if I can find a little time
> to help if patches would be well received.

I do not object having them fixed, but generally I think most -Wshadow 
warnings are just stupid. Especially, naming local variables like 
functions is hardly a source of errors. And I personally prefer naming 
constructor arguments the same as the members they initialize (I don't 
really like m_ prefixes or _ suffixes).

I agree that shadowing member variables (outside simple constructors) or 
previous local variables is a likely source of errors.
An example where I wish that something would warn is this:
http://ideone.com/5qDhY
(neither of -Wall, -Wextra, -Wshadow raises a warning!)

So my badmouthing aside, if we agree to avoid -Wshadow warnings it is 
mostly a thing someone with write-access shall do in one sweep; 
preferably after announcing it in order to avoid merge conflicts.


Christoph


--
----------------------------------------------
Dipl.-Inf., Dipl.-Math. Christoph Hertzberg
Cartesium 0.049
Universität Bremen
Enrique-Schmidt-Straße 5
28359 Bremen

Tel: +49 (421) 218-64252
----------------------------------------------