[PATCH] Fixes and features for Consumed analysis

Christian Wailes chriswailes at google.com
Wed Aug 21 11:39:06 PDT 2013


The tests that for making sure warnings are issued when attributes are
attached to the wrong kind of thing aren't in warn-consumed-analysis.cpp.
Like I said, they are in warn-consumed-parsing.cpp.

I'm splitting the patch now.

- Chris


On Wed, Aug 21, 2013 at 10:25 AM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Wed, Aug 21, 2013 at 12:39 PM, Christian Wailes
> <chriswailes at google.com> wrote:
> > With regards to the tests, those already exist.  They are in the
> > warn-consumed-parsing.cpp file.  As for the switch statements, I'll move
> > those to fully-covered switches.
>
> I do not see any such tests in SemaCXX\warn-consumed-analysis.cpp.
> The only tests I see there are testing other semantics; eg) invocation
> of method 'operator*' on object 'var' while it is in the 'consumed'
> state
>
> > I was not claiming that keeping patches limited is a bad idea, or was of
> > limited benefit in general.  Yes, I should have submitted this as
> multiple
> > patches, but to go back now and dig the different parts out of the commit
> > out would be time consuming, for very little benefit in this case.  All
> of
> > the things touched are inside my experimental code, or are the removal of
> > simple asserts.  I'd really like to focus on producing actual work, and
> keep
> > this as a lesson for next time.
>
> I understand the desire to move forward, but splitting patches into
> individual units of work is a clang patch submission criteria.  I
> believe it does have utility in this case since at least one of those
> patches would already be accepted (the assert removals) while the
> other two patches could have better-focused discussion involving the
> proper reviewers.  Please split this into multiple patches and we can
> continue to review the fixes you've made.
>
> Thanks!
>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130821/0aa87c2f/attachment.html>


More information about the cfe-commits mailing list