On Wed, May 15, 2013 at 1:57 AM, Manuel Klimek <[email protected]> wrote:
> On Tue, May 14, 2013 at 9:35 PM, Samuel Benzaquen <[email protected]>wrote: > >> >> >> ================ >> Comment at: lib/ASTMatchers/Dynamic/DynMatchers.cpp:35 >> @@ +34,3 @@ >> + ast_matchers::internal::BoundNodesTreeBuilder *Builder) const { >> + return M1->matches(DynNode, Finder, Builder) || >> + M2->matches(DynNode, Finder, Builder); >> ---------------- >> Manuel Klimek wrote: >> > How much harder would it be to not re-implement the matchers? I'd like >> to keep matcher implementations as cohesive as possible, as we want to be >> able to change the implementations when we introduce new features. >> The ones that are easier to reimplement are the ones that are way too >> generic, like anything() or equals(). >> >> anything() returns a polymorphic matcher which is not a DynTypedMatcher. >> I don't have specific Ts to create a Matcher<T> from anything(). I could >> list all possible types (the ones that DynTypedNode supports) and merge >> them, but it seemed to me that a reimplementation of it would be simpler. >> > > I agree it's easier to implement. I don't agree it'll be easier to > maintain. The problem is that some matchers (forEach*, has, etc) have > specific ways in which they affect binding of nodes. > Ok. I understand your point. This CL didn't really handle has/forEach/etc other than putting them on a list of "to reimplement". I'll investigate later how to handle the "adaptative matchers" another way. > I definitely don't want to duplicate that logic anywhere. Thus, I'd prefer > if we'd find a way to support those fully polymorphic matchers by > generating the dynamic ones for all types they can possibly match on > (perhaps we can just wrap them with something that works on > DynTypedMatcher?) > Would it be ok if the solution is to have these be implemented in a dynamic way and have the polymorphic/adaptative type-safe wrappers for statically bound code? I don't know if it is simpler to code/maintain, just wanted to know if you would be ok with that possibility. _Sam
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
