Here's a new version of the DeclStmt patch. Changes include:<div> - Fixed comments by declCountIs and hasSingleDecl</div><div> - Added hasDecl in the spirit of hasArgument</div><div> - Changed the loop to std::distance (std::advance in hasDecl)</div>
<div> - Added a few more test cases.<br><div class="gmail_extra"><br></div><div class="gmail_extra">And to explain the for loop in the test case for hasSingleDecl, I discovered that Clang explodes some DeclStmts with multiple declarations such as these:</div>
<div class="gmail_extra"> int a, b; // toplevel declarations</div><div class="gmail_extra">According to the AST dump, Clang treats this line as two separate DeclStmts, rather than one DeclStmt with two Decls. This also happens to declarations inside namespaces, and I'm not really sure where else. Maybe someone else has a better idea how to describe when the AST doesn't reflect the source the same way?</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">The other patch will be sent on a fork of the previous discussion.</div><div class="gmail_extra">Any new comments?</div><div class="gmail_extra"><br><div class="gmail_quote">
On Mon, Jul 16, 2012 at 9:39 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"><br>
On Mon, Jul 16, 2012 at 6:22 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> On Mon, Jul 16, 2012 at 12:03 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>> + // We could use Node.decl_begin() - Node.decl_end(), but that 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>
</div>I personally am fine with arguing for std::distance. My point is not<br>
to write the loop :)<br>
<div class=""><div class="h5"><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 casts are<br>
>> +/// stripped off.<br>
>> +AST_MATCHER_P(Expr, ignoreImplicitCasts,<br>
>> + internal::Matcher<Expr>, InnerMatcher) {<br>
>> + return InnerMatcher.matches(*Node.IgnoreImpCasts(), Finder, 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>
</div></div></blockquote></div><br></div></div>