[cfe-commits] [Patch] Make warning about uninitialized field include the field name

[Hans Wennborg]

On Thu, Sep 20, 2012 at 6:42 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Thu, Sep 20, 2012 at 5:58 AM, Hans Wennborg <hans at chromium.org> wrote:
>> On Tue, Sep 18, 2012 at 5:41 PM, Hans Wennborg <hans at chromium.org> wrote:
>>> The attached patch makes the warnings about uninitialized fields
>>> include the field name. This makes the wording more informative, and
>>> more consistent with the other uninitialized warnings.
>
> Looks reasonable (though is there a particular use case where the
> caret diagnostics don't indicate the right variable, etc? that makes
> having the name in the diagnostic particularly useful?)

No, I just wanted it to be consistent with the other warnings, and I
found it more informative when the name was included too.

>>> I'm not sure if we really need to do the LookupResult thing here; I
>>> copy-pasted it from SemaDecl.cpp:6329. It would be great if someone
>>> could comment on that.
>
> So far as I can tell this doesn't appear to be necessary in either
> case. Using DRE->getNameInfo().getName() directly in the
> SemaDecl.cpp:6329 situation doesn't regress any tests. I'd be inclined
> to change it there & use the same technique for your patch. At least
> that way if someone figures out why it was necessary, they'll ought to
> be able to add a test.

Sounds good to me.

> (consider this sign off, if you like - if you want a more informed
> opinion on the LookupResult issue, you could wait for that)

Thanks for the review! Committed in r164366.

 - Hans