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

[Richard Trieu]

On Tue, Aug 14, 2012 at 3:31 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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).
>
const string s = "Foo " + s + " bar"; is the motivating case for this
change, which is uncaught by current -Wuninitialized.  It's caught by
VisitCallExpr() where it any argument that is just a DeclRefExpr is checked.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120814/59aaaaca/attachment.html>