[cfe-commits] [Patch] Warn about self-init of refs using parenthesized initializers

[Richard Trieu]

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 <rtrieu at google.com> 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 <wei.pan at intel.com> 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:* cfe-commits-bounces at cs.uiuc.edu [mailto:
>> cfe-commits-bounces at cs.uiuc.edu] *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 <jordan_rose at apple.com>
>> 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 <hans at chromium.org> 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
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits****
>>
>> ** **
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120928/ed02c5b0/attachment.html>