[PATCH] Completely revamp node binding for AST matchers.

Manuel Klimek klimek at google.com
Tue Jun 18 07:36:45 PDT 2013



================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:3051
@@ +3050,3 @@
+
+TEST(LoopingMatchers, DoNotOverwritePreviousMatchResultOnFailure) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
----------------
Edwin Vane wrote:
> Manuel Klimek wrote:
> > Edwin Vane wrote:
> > > There are a lot of tests here all following the same pattern: do a bind and have a sub-matcher fail that doesn't cause the whole expression to fail. Are these tests meant to be an exhaustive list of matchers that can fail? Why did you choose these particular tests?
> > All changes made in this CL that were not "simple refactorings" have been test driven.
> > 
> > That is: I write a test, see it fail, then implement enough to make it work.
> I understand what you're doing now. Is it possible to add a comment to illustrate the philosophy behind the differences of these EXPECTs so somebody in the future knows what can/should be added to this test case? I know they just grew organically for you but there must be some common thread that made you pick these different matchers.
A most excellent idea. Done.


http://llvm-reviews.chandlerc.com/D992



More information about the cfe-commits mailing list