[cfe-commits] [Patch] Mark pass-by-reference as uninitialized variable use on initialization

[Richard Smith]

On Mon, Aug 6, 2012 at 6:47 PM, Richard Trieu <rtrieu at google.com> wrote:

> On Mon, Aug 6, 2012 at 4:09 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Mon, Aug 6, 2012 at 3:29 PM, Richard Trieu <rtrieu at google.com> wrote:
>>
>>> Modify the self-reference visitor to mark pass-by-reference to function
>>> calls as uninitialized use.  This will warn on code such as:
>>>
>>> const string Foo = "prefix" + Foo + "suffix";
>>>
>>
>> Can we put this under a more specific -W flag? -Wuninitialized is
>> intended to be essentially free of false positives.
>>
> Would this be more suited for a different uninitialized group or a new
> flag altogether?
>

I think a different warning flag would probably be best; neither
-Wuninitialized nor -Wconditional-uninitialized really fits well.


> I'm a little concerned that this uninitialized-variables warning is
>> deviating from the behavior of the CFG-based warning, which explicitly
>> excludes variables passed by reference to functions from its list of uses,
>> so if this is effective at finding bugs, perhaps we should add this check
>> there too.
>>
> At least for classes, this seems to be a reasonable check.  I'm not sure
> if it would be useful for other cases.
>
>>
>> --- lib/Sema/SemaDecl.cpp (revision 161345)
>> +++ lib/Sema/SemaDecl.cpp (working copy)
>> @@ -6223,7 +6223,7 @@
>>
>>      void VisitImplicitCastExpr(ImplicitCastExpr *E) {
>>        if ((!isRecordType && E->getCastKind() == CK_LValueToRValue) ||
>> -          (isRecordType && E->getCastKind() == CK_NoOp))
>> +          (E->getCastKind() == CK_NoOp))
>>
>> Drop these parens... But I'm not really clear on what this is checking
>> for. Presumably qualification conversions, but why are those a good place
>> to look for self-initialization?
>>
> T -> const T conversion when passed by reference.


OK, but does that mean we don't catch:

const string s = "Foo " + s + " bar";

? It would make more sense to me to check const
glvalue arguments to function call (including overloaded operator calls
etc).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120814/da05d98a/attachment.html>