[cfe-commits] [Patch] Warn about self-initialization of reference variables

[Richard Smith]

On Fri, Aug 17, 2012 at 3:15 AM, Hans Wennborg <hans at chromium.org> wrote:

> On Thu, Aug 16, 2012 at 9:24 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > LGTM, modulo a couple of small things:
>
> Thanks for the review!
>
> >> -  if (!VDecl->hasLocalStorage() &&
> >> -      (isa<InitListExpr>(Init) || !VDecl->getType()->isRecordType()))
> >> -    CheckSelfReference(RealDecl, Init);
> >> +  if (!VDecl->hasLocalStorage() || VDecl->getType()->isReferenceType())
> >> +    if (isa<InitListExpr>(Init) || !VDecl->getType()->isRecordType())
> >> +      CheckSelfReference(RealDecl, Init);
> >
> > '&&' instead of nested 'if's? It might be clearer to reuse
> !isTrackedVar()
> > (from Analysis/UninitializedValues.cpp) here, but I'm happy for the
> patch to
> > go in either way.
>
> Switched to && instead of nested ifs for now.
>
> !isTrackedVar() doesn't seem to be a one-to-one match here. For
> example, in my if statement I'm checking that it's not a record type,
> whereas !isTrackedVar() would check that it's not a scalar or vector
> type. Happy to work with this post-commit if you think it would be
> better, though.
>

Yeah, isTrackedVar() only corresponds (approximately) to the LHS of your
&&. The RHS is dealing with this check also being performed in SemaInit for
non-InitListExpr record initializations.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120817/46a42b38/attachment.html>