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.
On Fri, Sep 28, 2012 at 3:53 PM, Richard Trieu <[email protected]> wrote: > 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. > > On Fri, Sep 28, 2012 at 1:34 PM, Pan, Wei <[email protected]> wrote: > >> I discovered this starting from the following example**** >> >> ** ** >> >> extern int &foo();**** >> >> int &m([&]() { m = foo(); return m; }());**** >> >> ** ** >> >> This should be also warned too.**** >> >> ** ** >> >> *From:* [email protected] [mailto: >> [email protected]] *On Behalf Of *Richard Smith >> *Sent:* Friday, September 28, 2012 4:15 PM >> *To:* Jordan Rose; Richard Trieu >> *Cc:* llvm cfe >> *Subject:* Re: [cfe-commits] [Patch] Warn about self-init of refs using >> parenthesized initializers**** >> >> ** ** >> >> I don't think the patch is right. My recollection of this class is >> (though perhaps Richard Trieu can correct me):**** >> >> ** ** >> >> HandleExpr is intended to be called for the expression which is the >> top-level initializer of the declaration.**** >> >> 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).**** >> >> Visit* are called for all evaluated subexpressions (including ones we >> don't consider to be "used").**** >> >> ** ** >> >> 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:**** >> >> ** ** >> >> int n(n); // Warn here, or is this treated like int n = n;?**** >> >> int n{n}; // Warn here, or is this treated like int n = n;?**** >> >> ** ** >> >> ** ** >> >> 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: >> **** >> >> ** ** >> >> int &r = static_cast<int&>(r);**** >> >> ** ** >> >> ... nor ...**** >> >> ** ** >> >> struct S {} s = static_cast<S&>(s);**** >> >> ** ** >> >> On Fri, Sep 28, 2012 at 9:18 AM, Jordan Rose <[email protected]> >> wrote:**** >> >> This still doesn't handle C++11 initialization, does it? >> >> void g() { >> int &a{a}; >> } >> >> Jordan**** >> >> >> >> On Sep 28, 2012, at 4:05 , Hans Wennborg <[email protected]> wrote: >> >> > Hi, >> > >> > As pointed out in Wei's email [1], Clang currently fails to warn about >> > self-initialized references when using parenthesized initializers: >> > >> > void f() { >> > int &a(a); >> > } >> > >> > The attached patch fixes this. Please take a look. >> > >> > Thanks, >> > Hans >> > >> > [1]. >> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024582.html**** >> >> > >> <ref_self_init_parenthesized_initializers.patch>_______________________________________________ >> > cfe-commits mailing list >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits**** >> >> ** ** >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
