[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