[patch] Add a new warning flag -Wself-assign
Le-Chun Wu
lcwu@google.com
Thu Jun 10 01:17:00 GMT 2010
On Wed, Jun 9, 2010 at 11:56 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 9 June 2010 20:32, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Wed, Jun 9, 2010 at 11:25 AM, Le-Chun Wu <lcwu@google.com> wrote:
>>> It should be pretty easy to modify this patch to not warn about
>>> self-initialization of local variables with this flag. However, what I
>>> don't understand is why a user would want to work around (or suppress)
>>> an uninitialized warning by doing self-initialization. They could have
>>> just initialized a local variable to some other more meaningful value.
>>> Initializing a variable with itself doesn't seem like a good way to
>>> fix an uninitialized warning.
>>
>> Well this is the documented way of disabling the warning. If we want
>> to change this, we should get rid of -Winit-self at the same time.
>> And remove the documentation for this special case. I don't remember
>> the full history behind my change of the documentation to mention this
>> special case but from what I remember the source at one point did not
>> warn about "int t = t +1;". And that I was fixing that case with
>> keeping the special case of "int i = i;".
>
> That is broken in C++ anyway http://gcc.gnu.org/PR34772, so who cares?
> Let's get rid of this special case for Wuninitialized, get rid of
> -Winit-self and let -Wself-assign warn for everything.
>
> I would recommend that you add [C/C++] to the subject and/or CC
> relevant maintainers.
Will do when I send out the revised patch. Looking at MAINTAINERS
files, I couldn't figure out who the relevant maintainers are for
constant-folding code. Should they be middle-end or C/C++ frontend
maintainers?
>
> The idea of the patch seems ok to me but the implementation seems
> expensive. Adding yet another bit to one of the most used parts of the
> whole compiler and the whole folding wrap-up are a bit scary. But I am
> sure maintainers can give a more concrete answer and suggest
> alternative implementations.
While my experiments appear to suggest there should be no compile-time
implications with this patch, I do welcome alternative ideas. Let's
see what other people/maintainers say about it.
>
> One thing:
>
> + warning_at (location, OPT_Wself_assign, G_("%qE is assigned to itself"),
> + lhs);
>
> You do not need G_() here.
Will fix.
>
>
> +Wself-assign
> +Common Var(warn_self_assign) Init(0) Warning
> +Warn when a variable is assigned to itself
> This is not common but only C/C++, it should go in c.opt
Will fix.
Thanks,
Le-chun
More information about the Gcc-patches
mailing list