<div class="gmail_quote">On Tue, Aug 14, 2012 at 4:15 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><div class="gmail_quote">On Tue, Aug 14, 2012 at 4:01 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><div class="gmail_quote">On Tue, Aug 14, 2012 at 3:31 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div>On Mon, Aug 6, 2012 at 6:47 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div>On Mon, Aug 6, 2012 at 4:09 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div>On Mon, Aug 6, 2012 at 3:29 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modify the self-reference visitor to mark pass-by-reference to function calls as uninitialized use. This will warn on code such as:<div><div><br></div><div>const string Foo = "prefix" + Foo + "suffix";</div>
</div></blockquote><div><br></div></div><div>Can we put this under a more specific -W flag? -Wuninitialized is intended to be essentially free of false positives.</div></div></blockquote></div><div>Would this be more suited for a different uninitialized group or a new flag altogether?</div>
</div></blockquote><div><br></div></div><div>I think a different warning flag would probably be best; neither -Wuninitialized nor -Wconditional-uninitialized really fits well.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div>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.</div>
</div></blockquote></div><div>At least for classes, this seems to be a reasonable check. I'm not sure if it would be useful for other cases.</div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote">
<div><br></div><div><div>--- lib/Sema/SemaDecl.cpp<span style="white-space:pre-wrap"> </span>(revision 161345)</div><div>+++ lib/Sema/SemaDecl.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -6223,7 +6223,7 @@</div><div> </div><div> void VisitImplicitCastExpr(ImplicitCastExpr *E) {</div><div> if ((!isRecordType && E->getCastKind() == CK_LValueToRValue) ||</div><div>- (isRecordType && E->getCastKind() == CK_NoOp))</div>
<div>+ (E->getCastKind() == CK_NoOp))</div></div><div><br></div><div>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?</div>
</div>
</blockquote></div></div>T -> const T conversion when passed by reference.
</blockquote></div></div><br><div>OK, but does that mean we don't catch:</div><div><br></div><div>const string s = "Foo " + s + " bar";</div><div><br></div><div>? It would make more sense to me to check const glvalue arguments to function call (including overloaded operator calls etc).</div>
</blockquote></div></div></div>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.
</blockquote></div><br></div></div><div>OK, then it'd make more sense to me for that check to look through casts (at least lvalue bitcasts, no-op conversions and lvalue-to-rvalue conversions, but possibly others) than to start a search from no-op conversions.</div>
</blockquote></div><br><div>Changed the checks on casts to include the above, and do so for all casts, not just implicit casts.</div><div>Added test cases for lvalue bit cast.</div><div><br></div><div>What would be the proper flag for warning? -Wreference-uninitialized? -Wpossibly-uninitalized?</div>