<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 14, 2015 at 7:24 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class=""><div dir="ltr">On Mon, Sep 14, 2015 at 10:21 AM Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank" class="cremed">djasper@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">So, back in the day when we implemented the matchers, we decided on actually wanting to remove all the CXX... AST nodes (there are more of them).</div></blockquote><div><br></div></span><div>Note that Richard has paddled back on this and now says the CXX... AST nodes are the right thing.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> I don't know how this would work as recordDecl already exists. But I'd be somewhat hesitant to introduce a cxxRecordDecl matcher if there is still a chance that we want to move away from the CXX prefix.</div></blockquote><div><br></div></span><div>See above. </div></div></div></blockquote><div><br></div><div>So do we change all the others at the same time, e.g. create a cxxConstructExpr() matcher? There it make less sense as there is no ConstructExpr, just CXXConstructExpr.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Also note that AST matchers are used massively in the wild by now and I would be very hesitant to make a change breaking backwards compatibility. 85 failures in the clang repositories themselves sounds scary to me. Not saying we shouldn't do it at all. But we should be very clear on where things should be in the long run.<br></div></blockquote><div><br></div></span><div>Aaron has clarified that that's only 14 outside the AST matcher tests themselves.</div></div></div></blockquote><div><br></div><div>Sure, still a lot IMO :(. But ok, maybe there is just no other way.</div><div><span style="color:rgb(80,0,80)"> </span></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 14, 2015 at 7:03 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Mon, Sep 14, 2015 at 11:49 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>> wrote:<br>
><br>
><br>
> On Mon, Sep 14, 2015, 8:40 AM Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>> wrote:<br>
>><br>
>> On Sat, Sep 12, 2015 at 11:06 PM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> > On Sat, Sep 12, 2015, 9:25 PM Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Sat, Sep 12, 2015 at 8:22 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>><br>
>> >> wrote:<br>
>> >> ><br>
>> >> ><br>
>> >> > On Fri, Sep 11, 2015 at 10:39 PM Aaron Ballman<br>
>> >> > <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>><br>
>> >> > wrote:<br>
>> >> >><br>
>> >> >> On Fri, Sep 11, 2015 at 4:30 PM, Richard Smith<br>
>> >> >> <<a href="mailto:richard@metafoo.co.uk" target="_blank" class="cremed">richard@metafoo.co.uk</a>><br>
>> >> >> wrote:<br>
>> >> >> > I don't think CXXRecordDecl is an anachronism, so much as an<br>
>> >> >> > implementation<br>
>> >> >> > detail; it makes sense to use a smaller class when in C mode, as<br>
>> >> >> > we<br>
>> >> >> > don't<br>
>> >> >> > need most of the features and complexity that CXXRecordDecl brings<br>
>> >> >> > with<br>
>> >> >> > it.<br>
>> >> >> > But... as a user of clang matchers, I don't think I'd want to care<br>
>> >> >> > about<br>
>> >> >> > the<br>
>> >> >> > difference, and it'd be more convenient if I could nest (say) a<br>
>> >> >> > hasMethod<br>
>> >> >> > matcher within a recordDecl matcher, since it's completely obvious<br>
>> >> >> > what<br>
>> >> >> > that<br>
>> >> >> > should mean. If I have a matcher that says:<br>
>> >> >> ><br>
>> >> >> >   recordDecl(or(hasMethod(...), hasField(...)))<br>
>> >> >> ><br>
>> >> >> > I would expect that to work in both C and C++ (and the only way it<br>
>> >> >> > could<br>
>> >> >> > match in C would be on a record with the specified field, since<br>
>> >> >> > the<br>
>> >> >> > hasMethod matcher would always fail).<br>
>> >> >><br>
>> >> >> Okay, so then it sounds like we want recordDecl to *mean*<br>
>> >> >> RecordDecl,<br>
>> >> >> but we want the traversal and narrowing matchers that currently take<br>
>> >> >> a<br>
>> >> >> CXXRecordDecl to instead take a RecordDecl and handle the CXX part<br>
>> >> >> transparently? This means we would not need to add a cxxRecordDecl()<br>
>> >> >> matcher, but could still access CXX-only functionality (like access<br>
>> >> >> control, base classes, etc) through recordDecl()?<br>
>> >> ><br>
>> >> ><br>
>> >> > I'm against that proposal. I think we have tried to make the matchers<br>
>> >> > more<br>
>> >> > "user friendly" in the past, and all those attempts have failed<br>
>> >> > miserably;<br>
>> >> > in the end, users will do ast-dump to see what they want to match,<br>
>> >> > and<br>
>> >> > then<br>
>> >> > be confused when the matchers do follow the AST 99% of the time, but<br>
>> >> > try<br>
>> >> > to<br>
>> >> > be smart 1% of the time.<br>
>> >> > I think given that we want to keep CXXRecordDecl, the right solution<br>
>> >> > is<br>
>> >> > to<br>
>> >> > have a cxxRecordDecl() matcher.<br>
>> >><br>
>> >> Personally, I think this makes the most sense, at least to me. The<br>
>> >> recommendation I've always heard (and given) is to use -ast-dump and<br>
>> >> write matchers from there. (Consequently, the more I work with type<br>
>> >> traversal matchers, the more I wish we had -ast-dump-types to give<br>
>> >> even *more* information for writing matchers.)<br>
>> >><br>
>> >> But the question still remains, what do we do with recordDecl? Right<br>
>> >> now, it means CXXRecordDecl instead of RecordDecl. If we change it to<br>
>> >> mean RecordDecl instead, there's a chance we'll break existing,<br>
>> >> reasonable code. Are we okay with that risk? If we're at least<br>
>> >> conceptually okay with it, I could make the change locally and see<br>
>> >> just how much of our own code breaks, and report back. But if that<br>
>> >> turns out to be problematic, do we want to deprecate recordDecl and<br>
>> >> replace it with structDecl as our fallback position? Or is there a<br>
>> >> better solution?<br>
>> >><br>
>> >> Basically, I see a few ways to solve this (and there may be other ways<br>
>> >> I'm not thinking about yet):<br>
>> >><br>
>> >> 1) Undocument/deprecate recordDecl, add structDecl, unionDecl, and<br>
>> >> cxxRecordDecl. This does not match the AST because we have no<br>
>> >> StructDecl or UnionDecl types. The more I think about this option, the<br>
>> >> less I like it. It's easy to implement, but seems hard to relate to<br>
>> >> the AST.<br>
>> >> 2) Make recordDecl match RecordDecl, don't touch other matchers. Add<br>
>> >> way to distinguish unions from structs (e.g., isUnion(), isStruct()),<br>
>> >> add cxxRecordDecl. This matches the AST most closely, but may break<br>
>> >> code. I think that I prefer this approach, but it depends heavily on<br>
>> >> what "may break code" looks like in practice.<br>
>> >> 3) Make recordDecl match RecordDecl, fix other matchers that currently<br>
>> >> take CXXRecordDecl to instead take RecordDecl and handle sensibly when<br>
>> >> possible. Add a way to distinguish unions from structs, add<br>
>> >> cxxRecordDecl. This doesn't match the AST because we will have<br>
>> >> matchers taking a RecordDecl when the AST would require a<br>
>> >> CXXRecordDecl, but is likely to break less code.<br>
>> ><br>
>> ><br>
>> > That sums it up. My preferences are 2, 3, 1 in that order :)<br>
>><br>
>> I've attached a patch that implements #2, but it comes with ~85 errors<br>
>> from C++ matchers that use recordDecl to mean cxxRecordDecl.<br>
>><br>
>> <a href="http://pastebin.com/bxkRcqBV" rel="noreferrer" target="_blank" class="cremed">http://pastebin.com/bxkRcqBV</a><br>
>><br>
>> If this is an acceptable failure rate, I can also update the failing<br>
>> matchers to use cxxRecordDecl instead of recordDecl where applicable.<br>
>> Doing some spot-checking of the failing code, the failures are ones we<br>
>> anticipated, such as:<br>
>><br>
>> constructorDecl(ofClass(recordDecl(<br>
>> hasDeclaration(recordDecl(hasMethod(hasName("begin")),<br>
>> hasMethod(hasName("end"))))<br>
>> etc<br>
><br>
><br>
> +Daniel for another opinion. I think this is fine, but I'd  prefer not to<br>
> end up in a meme :)<br>
<br>
</div></div>FWIW, the vast majority of the errors are in ASTMatchersTests. There<br>
were only a few in actual real-world uses (71 in tests, 14 in real<br>
code).<br>
<span><font color="#888888"><br>
~Aaron<br>
</font></span><div><div><br>
><br>
>> ~Aaron<br>
>><br>
>> ><br>
>> >><br>
>> >> ~Aaron<br>
>> >><br>
>> >><br>
>> >> > Richard: if CXXRecordDecl was really an implementation detail, it<br>
>> >> > would<br>
>> >> > be<br>
>> >> > hidden behind a RecordDecl class, as an implementation detail. The<br>
>> >> > reasons<br>
>> >> > why we don't want it to be an implementation detail in the code<br>
>> >> > (performance, data structure size) don't matter - in the end, it's in<br>
>> >> > the<br>
>> >> > AST API.<br>
>> >> ><br>
>> >> >><br>
>> >> >><br>
>> >> >> ~Aaron<br>
>> >> >><br>
>> >> >> ><br>
>> >> >> > On Fri, Sep 11, 2015 at 6:30 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>><br>
>> >> >> > wrote:<br>
>> >> >> >><br>
>> >> >> >> Richard! We need an informed opinion :D<br>
>> >> >> >><br>
>> >> >> >> On Fri, Sep 11, 2015 at 3:07 PM Aaron Ballman<br>
>> >> >> >> <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>><br>
>> >> >> >> wrote:<br>
>> >> >> >>><br>
>> >> >> >>> Ping?<br>
>> >> >> >>><br>
>> >> >> >>> On Tue, Sep 8, 2015 at 9:26 AM, Manuel Klimek<br>
>> >> >> >>> <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>><br>
>> >> >> >>> wrote:<br>
>> >> >> >>> > On Tue, Sep 8, 2015 at 3:23 PM Aaron Ballman<br>
>> >> >> >>> > <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>><br>
>> >> >> >>> > wrote:<br>
>> >> >> >>> >><br>
>> >> >> >>> >> On Tue, Sep 8, 2015 at 9:18 AM, Manuel Klimek<br>
>> >> >> >>> >> <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>><br>
>> >> >> >>> >> wrote:<br>
>> >> >> >>> >> > On Tue, Sep 8, 2015 at 2:23 PM Aaron Ballman<br>
>> >> >> >>> >> > <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>><br>
>> >> >> >>> >> > wrote:<br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> On Tue, Sep 8, 2015 at 5:40 AM, Manuel Klimek<br>
>> >> >> >>> >> >> <<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>><br>
>> >> >> >>> >> >> wrote:<br>
>> >> >> >>> >> >> > Yea, we had that discussion a few times, and I can never<br>
>> >> >> >>> >> >> > remember<br>
>> >> >> >>> >> >> > why<br>
>> >> >> >>> >> >> > we<br>
>> >> >> >>> >> >> > ended up in the state we're in.<br>
>> >> >> >>> >> >> > We definitely had a time where we switched to just using<br>
>> >> >> >>> >> >> > the<br>
>> >> >> >>> >> >> > exact<br>
>> >> >> >>> >> >> > same<br>
>> >> >> >>> >> >> > name<br>
>> >> >> >>> >> >> > as the node's class name for the matchers.<br>
>> >> >> >>> >> >> > I *think* we didn't do it for cxxRecordDecl, because<br>
>> >> >> >>> >> >> > Richard<br>
>> >> >> >>> >> >> > said<br>
>> >> >> >>> >> >> > that's<br>
>> >> >> >>> >> >> > a<br>
>> >> >> >>> >> >> > relic we should get rid of anyway, but I'm not sure.<br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> FWIW, I think the state we're in is the worst of all<br>
>> >> >> >>> >> >> worlds.<br>
>> >> >> >>> >> >> It's<br>
>> >> >> >>> >> >> not<br>
>> >> >> >>> >> >> intuitive that recordDecl() doesn't match a struct in C<br>
>> >> >> >>> >> >> mode,<br>
>> >> >> >>> >> >> and<br>
>> >> >> >>> >> >> as<br>
>> >> >> >>> >> >> it stands, there is no way to match a struct or union<br>
>> >> >> >>> >> >> declaration<br>
>> >> >> >>> >> >> in C<br>
>> >> >> >>> >> >> at all.<br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> > Agreed. Best intentions. Worst possible outcome. That's<br>
>> >> >> >>> >> > software<br>
>> >> >> >>> >> > development<br>
>> >> >> >>> >> > :)<br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> >> ><br>
>> >> >> >>> >> >> > On Fri, Sep 4, 2015 at 8:32 PM Aaron Ballman<br>
>> >> >> >>> >> >> > <<a href="mailto:aaron@aaronballman.com" target="_blank" class="cremed">aaron@aaronballman.com</a>><br>
>> >> >> >>> >> >> > wrote:<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> It turns out that the recordDecl() AST matcher doesn't<br>
>> >> >> >>> >> >> >> match<br>
>> >> >> >>> >> >> >> RecordDecl objects; instead, it matches CXXRecordDecl<br>
>> >> >> >>> >> >> >> objects.<br>
>> >> >> >>> >> >> >> This<br>
>> >> >> >>> >> >> >> is... unfortunate... as it makes writing AST matchers<br>
>> >> >> >>> >> >> >> more<br>
>> >> >> >>> >> >> >> complicated<br>
>> >> >> >>> >> >> >> because of having to translate between<br>
>> >> >> >>> >> >> >> recordDecl()/CXXRecordDecl.<br>
>> >> >> >>> >> >> >> It<br>
>> >> >> >>> >> >> >> also makes it impossible to match a struct or union<br>
>> >> >> >>> >> >> >> declaration<br>
>> >> >> >>> >> >> >> in C<br>
>> >> >> >>> >> >> >> or ObjC. However, given how prevalent recordDecl()'s<br>
>> >> >> >>> >> >> >> use<br>
>> >> >> >>> >> >> >> is<br>
>> >> >> >>> >> >> >> in<br>
>> >> >> >>> >> >> >> the<br>
>> >> >> >>> >> >> >> wild (I'm guessing), changing it at this point would be<br>
>> >> >> >>> >> >> >> a<br>
>> >> >> >>> >> >> >> Bad<br>
>> >> >> >>> >> >> >> Thing.<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> For people trying to write AST matchers for languages<br>
>> >> >> >>> >> >> >> like<br>
>> >> >> >>> >> >> >> C<br>
>> >> >> >>> >> >> >> or<br>
>> >> >> >>> >> >> >> ObjC,<br>
>> >> >> >>> >> >> >> I would like to propose adding:<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> structDecl()<br>
>> >> >> >>> >> >> >> unionDecl()<br>
>> >> >> >>> >> >> >> tagDecl()<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> These will match nicely with the existing enumDecl()<br>
>> >> >> >>> >> >> >> AST<br>
>> >> >> >>> >> >> >> matcher.<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> Additionally, I would like to add cxxRecordDecl() to<br>
>> >> >> >>> >> >> >> match<br>
>> >> >> >>> >> >> >> CXXRecordDecl objects. While it duplicates the<br>
>> >> >> >>> >> >> >> functionality<br>
>> >> >> >>> >> >> >> exposed<br>
>> >> >> >>> >> >> >> by recordDecl(), it more clearly matches the intention<br>
>> >> >> >>> >> >> >> of<br>
>> >> >> >>> >> >> >> which<br>
>> >> >> >>> >> >> >> AST<br>
>> >> >> >>> >> >> >> node it corresponds to.<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> Finally, I would like to undocument recordDecl() and<br>
>> >> >> >>> >> >> >> change<br>
>> >> >> >>> >> >> >> our<br>
>> >> >> >>> >> >> >> existing documentation and AST matcher uses to use<br>
>> >> >> >>> >> >> >> cxxRecordDecl/structDecl() instead. Maybe someday we<br>
>> >> >> >>> >> >> >> can<br>
>> >> >> >>> >> >> >> deprecate<br>
>> >> >> >>> >> >> >> recordDecl() more officially.<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> I'm open to other ideas if there are better ways to<br>
>> >> >> >>> >> >> >> move<br>
>> >> >> >>> >> >> >> forward. If<br>
>> >> >> >>> >> >> >> you think changing the meaning of recordDecl() is<br>
>> >> >> >>> >> >> >> acceptable,<br>
>> >> >> >>> >> >> >> I<br>
>> >> >> >>> >> >> >> can<br>
>> >> >> >>> >> >> >> also go that route (though I would still propose adding<br>
>> >> >> >>> >> >> >> unionDecl()<br>
>> >> >> >>> >> >> >> and cxxRecordDecl() in that case).<br>
>> >> >> >>> >> >> ><br>
>> >> >> >>> >> >> ><br>
>> >> >> >>> >> >> > I think changing recordDecl is acceptable. I believe<br>
>> >> >> >>> >> >> > very<br>
>> >> >> >>> >> >> > few<br>
>> >> >> >>> >> >> > tools<br>
>> >> >> >>> >> >> > will<br>
>> >> >> >>> >> >> > actually start doing wrong things because of it. I'd<br>
>> >> >> >>> >> >> > like<br>
>> >> >> >>> >> >> > more<br>
>> >> >> >>> >> >> > opinions<br>
>> >> >> >>> >> >> > first, though :)<br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> I was giving this more thought over the long weekend, and<br>
>> >> >> >>> >> >> I<br>
>> >> >> >>> >> >> think<br>
>> >> >> >>> >> >> you<br>
>> >> >> >>> >> >> may be right. I think changing recordDecl() to mean<br>
>> >> >> >>> >> >> RecordDecl<br>
>> >> >> >>> >> >> will<br>
>> >> >> >>> >> >> fix more code than it breaks, so long as we take a<br>
>> >> >> >>> >> >> holistic<br>
>> >> >> >>> >> >> approach<br>
>> >> >> >>> >> >> to the change and see which narrowing and traversal<br>
>> >> >> >>> >> >> matchers<br>
>> >> >> >>> >> >> we<br>
>> >> >> >>> >> >> need<br>
>> >> >> >>> >> >> to fix up at the same time. When I tried to think of AST<br>
>> >> >> >>> >> >> matchers<br>
>> >> >> >>> >> >> that<br>
>> >> >> >>> >> >> mean CXXRecordDecl but *not* RecordDecl, they were<br>
>> >> >> >>> >> >> horribly<br>
>> >> >> >>> >> >> contrived<br>
>> >> >> >>> >> >> because you usually are matching on additional selection<br>
>> >> >> >>> >> >> criteria<br>
>> >> >> >>> >> >> that<br>
>> >> >> >>> >> >> is specific to C++ (such as hasMethod() or<br>
>> >> >> >>> >> >> isDerivedFrom())<br>
>> >> >> >>> >> >> which<br>
>> >> >> >>> >> >> would cause the match to continue to fail, as expected.<br>
>> >> >> >>> >> >> Code<br>
>> >> >> >>> >> >> that<br>
>> >> >> >>> >> >> uses<br>
>> >> >> >>> >> >> recordDecl() to mean RecordDecl will suddenly start to<br>
>> >> >> >>> >> >> match<br>
>> >> >> >>> >> >> in<br>
>> >> >> >>> >> >> more<br>
>> >> >> >>> >> >> cases, but that's likely to be a bug fix more than a<br>
>> >> >> >>> >> >> breaking<br>
>> >> >> >>> >> >> change.<br>
>> >> >> >>> >> >> To the best of my understanding, the only breaking cases<br>
>> >> >> >>> >> >> would<br>
>> >> >> >>> >> >> be<br>
>> >> >> >>> >> >> where you wrote recordDecl(), meant CXXRecordDecl, had no<br>
>> >> >> >>> >> >> further<br>
>> >> >> >>> >> >> narrowing or traversal matchers, and were compiling in C<br>
>> >> >> >>> >> >> mode;<br>
>> >> >> >>> >> >> with<br>
>> >> >> >>> >> >> the result being additional unexpected matches.<br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> > Ah, there's one thing that can break: the compile can<br>
>> >> >> >>> >> > break:<br>
>> >> >> >>> >> > recordDecl(hasMethod(...)) will *not* compile (it'll work<br>
>> >> >> >>> >> > in<br>
>> >> >> >>> >> > the<br>
>> >> >> >>> >> > dynamic<br>
>> >> >> >>> >> > matchers and fail as you suggest, but the in-C++ DSL does<br>
>> >> >> >>> >> > more<br>
>> >> >> >>> >> > static<br>
>> >> >> >>> >> > type<br>
>> >> >> >>> >> > checking).<br>
>> >> >> >>> >> > I don't think that's super bad though.<br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> So perhaps it would make sense to:<br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> 1) Make recordDecl() mean RecordDecl<br>
>> >> >> >>> >> >> 2) Do a comprehensive review of matchers that take a<br>
>> >> >> >>> >> >> CXXRecordDecl<br>
>> >> >> >>> >> >> and<br>
>> >> >> >>> >> >> see if they should instead take a RecordDecl<br>
>> >> >> >>> >> >> 3) Add unionDecl() as a node matcher (or should we add<br>
>> >> >> >>> >> >> isUnion()<br>
>> >> >> >>> >> >> and<br>
>> >> >> >>> >> >> isStruct() as narrowing matchers?)<br>
>> >> >> >>> >> >> 4) Add tagDecl() as a node matcher, but not add<br>
>> >> >> >>> >> >> cxxRecordDecl()<br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> > Why not add cxxRecordDecl()? I think we need it if we want<br>
>> >> >> >>> >> > narrowing<br>
>> >> >> >>> >> > matchers on CXXRecordDecl?<br>
>> >> >> >>> >><br>
>> >> >> >>> >> If Richard thinks CXXRecordDecl is an anachronism, I figured<br>
>> >> >> >>> >> we<br>
>> >> >> >>> >> didn't<br>
>> >> >> >>> >> want to expose it. Instead, we could make hasMethod (et al)<br>
>> >> >> >>> >> accept<br>
>> >> >> >>> >> a<br>
>> >> >> >>> >> RecordDecl and do the type checking for the caller. Then<br>
>> >> >> >>> >> recordDecl(hasMethod(...)) continues to compile and work, and<br>
>> >> >> >>> >> when<br>
>> >> >> >>> >> hasMethod is given a RecordDecl instead of a CXXRecordDecl,<br>
>> >> >> >>> >> it<br>
>> >> >> >>> >> simply<br>
>> >> >> >>> >> matches nothing. But you bring up a good point about the C++<br>
>> >> >> >>> >> DSL<br>
>> >> >> >>> >> being<br>
>> >> >> >>> >> a problem still, I hadn't considered that.<br>
>> >> >> >>> ><br>
>> >> >> >>> ><br>
>> >> >> >>> > First I want Richard to confirm that. I have a very bad<br>
>> >> >> >>> > memory,<br>
>> >> >> >>> > so I<br>
>> >> >> >>> > might<br>
>> >> >> >>> > as well misremember :)<br>
>> >> >> >>> ><br>
>> >> >> >>> >><br>
>> >> >> >>> >><br>
>> >> >> >>> >> ~Aaron<br>
>> >> >> >>> >><br>
>> >> >> >>> >> ><br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> ~Aaron<br>
>> >> >> >>> >> >><br>
>> >> >> >>> >> >> ><br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> Thanks!<br>
>> >> >> >>> >> >> >><br>
>> >> >> >>> >> >> >> ~Aaron<br>
>> >> >> ><br>
>> >> >> ><br>
</div></div></blockquote></div><br></div></div></blockquote></div></div></div>
</blockquote></div><br></div></div>