I have started a new thread with a patch for the self-init checker that will work better with references and CC-ed everyone here.<br><br><div class="gmail_quote">On Fri, Sep 28, 2012 at 3:53 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">After talking with Richard Smith, I will be working on some changes to the SelfReferenceVisitor. The HandleExpr was a bit of a hack when first introduced and should be removed. The call to the self reference checker should be moved later into the function so that the initial Expr will be processed first (this processing will get rid of the ParenListExpr). This should clean up the visitor a bit. Stay tuned for a patch.<div class="HOEnZb">
<div class="h5"><div>
<br></div><div>On Fri, Sep 28, 2012 at 1:34 PM, Pan, Wei <span dir="ltr"><<a href="mailto:wei.pan@intel.com" target="_blank">wei.pan@intel.com</a>></span> wrote:<br><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I discovered this starting from the following example<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">extern int &foo();<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">int &m([&]() { m = foo(); return m; }());<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">This should be also warned too.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> <a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a>]
<b>On Behalf Of </b>Richard Smith<br>
<b>Sent:</b> Friday, September 28, 2012 4:15 PM<br>
<b>To:</b> Jordan Rose; Richard Trieu<br>
<b>Cc:</b> llvm cfe<br>
<b>Subject:</b> Re: [cfe-commits] [Patch] Warn about self-init of refs using parenthesized initializers<u></u><u></u></span></p>
</div><div><div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">I don't think the patch is right. My recollection of this class is (though perhaps Richard Trieu can correct me):<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">HandleExpr is intended to be called for the expression which is the top-level initializer of the declaration.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">HandleValue is intended to be called for an expression which is "used" (either directly in the initialization of the result, or as the operand of an lvalue-to-rvalue conversion).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Visit* are called for all evaluated subexpressions (including ones we don't consider to be "used").<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">So... I think that we shouldn't be calling HandleExpr (nor HandleValue) from VisitParenListExpr, since it doesn't imply a use, in the required sense. I think the right way to deal with this is to add special cases for ParenListExpr and
InitListExpr to HandleExpr. We also need to decide whether to warn in these cases:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">int n(n); // Warn here, or is this treated like int n = n;?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">int n{n}; // Warn here, or is this treated like int n = n;?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">FWIW, it looks like we're missing an IgnoreParenCasts call from HandleExpr, and the IgnoreParenImpCasts call in HandleValue should be IgnoreParenCasts. We don't currently warn on cases like these, for instance:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">int &r = static_cast<int&>(r);<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">... nor ...<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">struct S {} s = static_cast<S&>(s);<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">On Fri, Sep 28, 2012 at 9:18 AM, Jordan Rose <<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">This still doesn't handle C++11 initialization, does it?<br>
<br>
void g() {<br>
int &a{a};<br>
}<br>
<br>
Jordan<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<br>
On Sep 28, 2012, at 4:05 , Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
<br>
> Hi,<br>
><br>
> As pointed out in Wei's email [1], Clang currently fails to warn about<br>
> self-initialized references when using parenthesized initializers:<br>
><br>
> void f() {<br>
> int &a(a);<br>
> }<br>
><br>
> The attached patch fixes this. Please take a look.<br>
><br>
> Thanks,<br>
> Hans<br>
><br>
> [1]. <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024582.html" target="_blank">
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024582.html</a><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal">> <ref_self_init_parenthesized_initializers.patch>_______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div>
</div>
</blockquote></div><br>
</div>
</div></div></blockquote></div><br>