================
Comment at: unittests/AST/SourceLocationTest.cpp:24
@@ +23,3 @@
+/// \brief Base class for verifying some property of nodes found by a matcher.
+class MatchVerifier : public MatchFinder::MatchCallback {
+public:
----------------
Philip Craig wrote:
> Manuel Klimek wrote:
> > I see overlap in the implementation with both TestVisitor.h and
> > RecursiveASTVisitorTest.cpp (which already has location testing).
> >
> > Do you think we can pull out something common?
> There's bits of overlap between all of these unit tests, and I'm not sure yet
> which bits should be common.
>
> I started using TestVisitor.h, but the structure of that code doesn't lend
> itself well to adding in range checking. I couldn't see a way to add it
> without either rewriting it all, or duplicating a lot of code.
>
> The secondary issue is that TestVisitor.h doesn't use ASTMatchers, and I
> think they are quite useful for this sort of testing. I'm assuming that
> ASTMatchers shouldn't be used for testing RAV, since it's essentially another
> lay on top of RAV, so there may be things you can't test that way.
Yes, you're right. My main concern was that I wanted to make sure you know of
the other instances, and didn't see a quick way to integrate them. I can poke
at that later...
================
Comment at: unittests/AST/SourceLocationTest.cpp:186
@@ +185,3 @@
+TEST(SourceLocation, KNRParamLocation) {
+ LocationVerifier<ParmVarDecl> Verifier;
+ Verifier.expectLocation(1, 8).expectId("i");
----------------
Both here and below having the extra expactLocation seems superfluous, if we do
not also want to support multiple locations (which probably would require a
slightly different interface again).
I would just put the stuff into the constructor. Unless you have plans for
adding much more to the interface...
http://llvm-reviews.chandlerc.com/D72
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
