| Re: [eigen] g++ -Wshadow warnings |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/eigen Archives
]
- To: eigen <eigen@xxxxxxxxxxxxxxxxxxx>
- Subject: Re: [eigen] g++ -Wshadow warnings
- From: Gael Guennebaud <gael.guennebaud@xxxxxxxxx>
- Date: Wed, 26 Feb 2014 22:26:23 +0100
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type:content-transfer-encoding; bh=gl/KJ+2yIzO8U9vvB/aAa5u3YF6kinkjb+ITxSZE1Bc=; b=s2t9rfU2KnVl65g4dPq5Ec3HESiTy/5USz3M7ogZUUsrmtA3fiP8rqz8ib3VnHoUgP JjkMviKpXgeKvrkZgsaEiIPmyIWbsqQq0p7CjlWBpWZVsrsti9ssulpsJadFtRseOeOE YD9L/UVOf63zRoKffqfIw2NDSdTkTKE1/Nr2lpyt/QrlM3L0PUkV80NYoDZzaY1rY6Tq jhOQDBmCfJ+s5pnYMhBu5nMEljdrQ7GUC2D5uUvrZI9Yvw5DUh2bVUgukZfU+et3hnGQ buRK0LOePWwoCjB88zC6EaBFKCXdb6WLl25I0gqH3zosGlG7LeaPZb5jOrIZZorf4gQU krfQ==
I'm ok too to have them fixed since I already took some time to fix
them in the Core module:
https://bitbucket.org/eigen/eigen/commits/aec73cb748d3c6700e153a7f344dc67df7c994e8.
However, I'm sure that's the right timing because of the current
refactoring which is happening in the evaluator branch.
gael
On Wed, Feb 26, 2014 at 6:32 PM, Christoph Hertzberg
<chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> 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
> ----------------------------------------------
>
>