[cfe-commits] [PATCH] Introduces DynTypedMatcher as a new concept that replaces the UntypedBaseMatcher and TypedMatcher. Due to DynTypedNode the basic dynamically typed matcher interface can now be simplified.

Sean Silva reviews at llvm-reviews.chandlerc.com
Mon Sep 3 07:27:24 PDT 2012


  > ================
  > Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:353-355
  > @@ -367,4 +352,5 @@
  > +                             this, &Builder)) {
  >          BoundNodesTree BoundNodes = Builder.build();
  >          MatchVisitor Visitor(ActiveASTContext, It->second);
  >          BoundNodes.visitMatches(&Visitor);
  >        }
  > ----------------
  > Sean Silva wrote:
  >> I know this isn't directly related to this patch, but this is really the nerve-center of the matching process, and deserves to be better explained (or cross-referenced to where it is explained).
  > Hm, which parts need more explanations? Anything specific?

  It's not that it's that hard to understand, but that it's hard to
  find. So, for example, I think it would be good to mention it in the
  comment at the top of the file. Also, I think a comment on match()
  should definitely say "this is the core of the matching process", or
  something like that. In other words, in order to understand the code,
  this is the best place to start at (AFAIK), and the reader should be
  aware of that so that they don't waste their time.

  If there is another place which is a better place to start, then that
  should be the thing called out in the comment at the top of the file
  of course.

  --Sean Silva

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



More information about the cfe-commits mailing list