[PATCH] D19754: Allow 'nodebug' on local variables

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Tue May 17 15:05:08 PDT 2016


On Tue, May 17, 2016 at 2:30 PM, Robinson, Paul <paul.robinson at sony.com>
wrote:

> What you are describing is what testing literature refers to as criteria
> for equivalence classes.  There is some level of judgment to that, yes.
>
>
>
> Yep yep, to be sure. I'm just generally trying to encourage the community
> behavior towards being both selective & thorough about testing.
>
>
>
> I have noticed you doing this (not just in this review) and I am very
> appreciative of the principles; when it comes to understanding what a test
> is trying to do, keeping the unnecessary fluff out is very helpful.  You
> have no idea how many times I've had to suss out the intent of a (usually
> comment-free) test after it broke when we merged it into our tree.
> Fortunately that sort of thing has been happening less often, now that more
> of our changes have been integrated upstream, but still, it's great to have
> tests that are very focused….
>
>
>
> ….when they are tests for a bugfix or other comparatively small change.  I
> have to say when it comes to a new-feature kind of patch, I would rather
> have the test err on the side of completeness.
>

I'm not intending to argue against completeness, to be sure.


>   This is partly based on the experience of introducing the 'optnone'
> attribute to Clang, which IIRC popped up with new and surprising cases two
> or three times after its introduction.
>

For the broader discussion about test strategy, I'd love to hear more about
these cases (perhaps on another thread) to make sure I/we/community take
the right lessons from them to improve the process and provide review
feedback that pushes us in a good direction.


>   More thorough tests up front could easily have prevented those
> surprises.  Now here I am again, not with a new attribute but seriously
> expanding the applicability of an attribute, and would like to apply
> previous experience and start out with what I think should be a moderately
> complete test.
>
>
>
> If you're unwilling to accept that argument and insist on minimal upstream
> tests, okay; I can take what I've done and migrate it into our private
> tests, and leave behind only the minimal upstream test.  It will leave me
> with the test I think the feature needs, leaves upstream with the minimal
> test you prefer, and if something breaks it will just take a little longer
> to get that feedback.
>

I think that'd be best, if you've got the option to do so/find it necessary.

Thanks!
- Dave


> Let me know.
>
> Thanks,
>
> --paulr
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160517/e0899faf/attachment.html>


More information about the cfe-commits mailing list