<div dir="ltr">On Wed, May 15, 2013 at 4:55 PM, Samuel Benzaquen <span dir="ltr"><<a href="mailto:sbenza@google.com" target="_blank">sbenza@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">On Wed, May 15, 2013 at 1:57 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div>On Tue, May 14, 2013 at 9:35 PM, Samuel Benzaquen <span dir="ltr"><<a href="mailto:sbenza@google.com" target="_blank">sbenza@google.com</a>></span> wrote:<br>

</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><br>
<br>
================<br>
Comment at: lib/ASTMatchers/Dynamic/DynMatchers.cpp:35<br>
@@ +34,3 @@<br>
+          ast_matchers::internal::BoundNodesTreeBuilder *Builder) const {<br>
+    return M1->matches(DynNode, Finder, Builder) ||<br>
+        M2->matches(DynNode, Finder, Builder);<br>
----------------<br>
</div><div>Manuel Klimek wrote:<br>
> 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.<br>



</div>The ones that are easier to reimplement are the ones that are way too generic, like anything() or equals().<br>
<br>
anything() returns a polymorphic matcher which is not a DynTypedMatcher.<br>
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.<br>


</blockquote><div><br></div></div></div><div>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.</div>

</div></div></div></blockquote></div><div><div class="gmail_extra"><br></div><div class="gmail_extra">Ok. I understand your point.</div><div class="gmail_extra">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.</div>

</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">

<div class="gmail_quote"><div>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?)</div>


</div></div></div>
</blockquote></div></div><br></div><div class="gmail_extra">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?</div>

<div class="gmail_extra">I don't know if it is simpler to code/maintain, just wanted to know if you would be ok with that possibility.</div></div></blockquote><div style><br>If I parsed that sentence correctly, I think I'm OK in principle, but I'd have to see the implementation :)</div>
<div style><br></div></div></div></div>