[PATCH] Fixes and features for Consumed analysis

Aaron Ballman aaron at aaronballman.com
Wed Aug 21 10:25:55 PDT 2013


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



More information about the cfe-commits mailing list