Latest version attached!<div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 17, 2012 at 2:07 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">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 class="im">On Tue, Jul 17, 2012 at 12:47 AM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
> I also noticed that a hasDeclaration matcher which serves a different<br>
> purpose. I think the new hasDecl matcher needs a new name...<br>
<br>
</div>Good point. Any ideas for how to differentiate "hasDeclaration" in<br>
terms of "something that declares what's used here" vs.<br>
"hasDeclaration" in terms of "aggregates a declaration that matches"?<br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div><br></div><div>How about "containsDeclaration" for the latter case? Since it's intended for use in a DeclStmt, it makes sense to talk about the declarations contained within the statement - and I think it would be difficult to find a better name for the original "hasDeclaration."</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
><br>
><br>
> On Mon, Jul 16, 2012 at 3:45 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
>><br>
>> Here's a new version of the DeclStmt patch. Changes include:<br>
>> - Fixed comments by declCountIs and hasSingleDecl<br>
>> - Added hasDecl in the spirit of hasArgument<br>
>> - Changed the loop to std::distance (std::advance in hasDecl)<br>
>> - Added a few more test cases.<br>
>><br>
>> And to explain the for loop in the test case for hasSingleDecl, I<br>
>> discovered that Clang explodes some DeclStmts with multiple declarations<br>
>> such as these:<br>
>> int a, b; // toplevel declarations<br>
>> According to the AST dump, Clang treats this line as two separate<br>
>> DeclStmts, rather than one DeclStmt with two Decls. This also happens to<br>
>> declarations inside namespaces, and I'm not really sure where else. Maybe<br>
>> someone else has a better idea how to describe when the AST doesn't reflect<br>
>> the source the same way?<br>
>><br>
>> The other patch will be sent on a fork of the previous discussion.<br>
>> Any new comments?<br>
>><br>
>> On Mon, Jul 16, 2012 at 9:39 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>>><br>
>>><br>
>>> On Mon, Jul 16, 2012 at 6:22 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>><br>
>>> wrote:<br>
>>> > On Mon, Jul 16, 2012 at 12:03 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>> > wrote:<br>
>>> >> + // We could use Node.decl_begin() - Node.decl_end(), but that<br>
>>> >> relies on<br>
>>> >> + // decl_iterator just being a Decl**.<br>
>>> >> + unsigned DeclCount = 0;<br>
>>> >> + for (DeclStmt::const_decl_iterator I = Node.decl_begin(),<br>
>>> >> + E = Node.decl_end(); I != E; ++I)<br>
>>> >> + ++DeclCount;<br>
>>> >><br>
>>> >> (after chatting with Chandler about this on irc):<br>
>>> >> I'd use Node.decl_end() - Node.decl_begin(). If it ever becomes a<br>
>>> >> non-const-time operation, the iterator will not implement the<br>
>>> >> interface and break compilation, so we'll notice.<br>
>>> ><br>
>>> > But do we need to notice? If the original algorithm written here is<br>
>>> > linear it seems like constant time size is not a requirement.<br>
>>> ><br>
>>> > If that's the case, then std::distance just DTRT - constant time for<br>
>>><br>
>>> I personally am fine with arguing for std::distance. My point is not<br>
>>> to write the loop :)<br>
>>><br>
>>> > random access iterators, linear for others. (alternatively, provide<br>
>>> > Node::decl_size that does the same thing - but I can understand the<br>
>>> > concern of providing a (possibly in the future) non-constant-time<br>
>>> > size, though at that point you could remove size & go back & examine<br>
>>> > each client to see which ones care about that)<br>
>>> ><br>
>>> >><br>
>>> >> Regardless of that, I think your comment is wrong in 2 ways: first,<br>
>>> >> there's a typo :) Second, that the iterator happens to come down do<br>
>>> >> being a pointer has nothing to do with its contract. It either<br>
>>> >> provides random access or not.<br>
>>> >><br>
>>> >> +/// \brief Matches expressions that match InnerMatcher after implicit<br>
>>> >> casts are<br>
>>> >> +/// stripped off.<br>
>>> >> +AST_MATCHER_P(Expr, ignoreImplicitCasts,<br>
>>> >> + internal::Matcher<Expr>, InnerMatcher) {<br>
>>> >> + return InnerMatcher.matches(*Node.IgnoreImpCasts(), Finder,<br>
>>> >> Builder);<br>
>>> >> +}<br>
>>> >><br>
>>> >> I think we should implement the equivalent based on ignoreParenImpCast<br>
>>> >> first, as that's what I've seen us needing much more often (we can<br>
>>> >> implement this one, too, of course ;)<br>
>>> >><br>
>>> >> Cheers,<br>
>>> >> /Manuel<br>
>>> >><br>
>>> >> On Fri, Jul 13, 2012 at 8:34 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
>>> >>> <div>Attached are three more small matcher patches. One fixes another<br>
>>> >>> rename typo (AnyOf --> anyOf) that was similar to the previous<br>
>>> >>> allOf patch. The second patch adds more inspection for<br>
>>> >>> declarationStatement matchers, making it easier to look at single<br>
>>> >>> declarations directly. The third patch adds expression matchers which<br>
>>> >>> call IgnoreXXXCasts() before applying their<br>
>>> >>> sub-matchers.</div><div><br></div>For future reference, should I<br>
>>> >>> continue splitting up these patches for<br>
>>> >>> review?<div><br></div><div>-Sam</div><br>
>>> >>><br>
>>> >>> _______________________________________________<br>
>>> >>> cfe-commits mailing list<br>
>>> >>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>>> >>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>>> >>><br>
>>> >> _______________________________________________<br>
>>> >> cfe-commits mailing list<br>
>>> >> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>>> >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>><br>
>><br>
><br>
</div></div></blockquote></div><br></div>