[patch] [C/C++] Add a new warning flag -Wself-assign

[Le-Chun Wu]

On Wed, Jun 23, 2010 at 5:54 PM, Paul Koning <Paul_Koning@dell.com> wrote:
>> > Given that self-assign is the documented way to suppress a warning
>> (for
>> > uninitialized variable) is it appropriate for -Wall to turn this on?
>>  It
>> > would mean that code that was specifically fixed to avoid warnings
>> under
>> > -Wall will now get warnings again on that exact same line.  I don't
>> > object to the feature itself, but I do object to that aspect.
>> >
>>
>> This patch actually removes the paragraph in invoke.texi that
>> recommends using self-initialization to suppress uninitialized
>> variable warnings. Yes, this would be a disruptive change to people
>> who use self-initialization, but in my opinion, self-initialization
>> doesn't really make sense and is not a good way to fix or work around
>> uninitialized variable warnings. (One can initialize a variable with
>> other more meaningful value as easily.)
>
> Yes, and that's usually fine.  But there is a performance difference -- a small one admittedly.  If the initialization is in fact not necessary, then the existing coding convention suppresses the warning with no run-time overhead, while the proposed workaround you describe does have a run-time cost.
>

I would think most of the uninitialized warnings triggered are genuine
bugs and the programmers should just fix them. The self-init
workaround was probably used primarily to suppress the false positives
that were often caused by the analysis not understanding the
predicates that guard the definitions and uses of variables. Now that
David Li has checked in the patch that makes the uninitialized
variable analysis predicate-aware (r158835), the false positive rate
should go down quite a lot. And even for the rare cases where people
still have to suppress the warnings, as you said, the performance
difference of initializing a variable to a real value should be pretty
small (but the code would make more sense). So I still think that we
should include -Wself-assign in -Wall, and that people who use
self-initialization (with -Wall) should evaluate whether the false
positives still exist, and either remove the self-initialization or
change the initialization to a more meaningful value.

Thanks,

Le-chun