[cfe-commits] [PATCH] x3 More matcher patches

Manuel Klimek klimek at google.com
Mon Jul 16 09:39:42 PDT 2012


On Mon, Jul 16, 2012 at 6:22 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Jul 16, 2012 at 12:03 AM, Manuel Klimek <klimek at google.com> wrote:
>> +  // We could use Node.decl_begin() - Node.decl_end(), but that relies on
>> +  // decl_iterator just being a Decl**.
>> +  unsigned DeclCount = 0;
>> +  for (DeclStmt::const_decl_iterator I = Node.decl_begin(),
>> +       E = Node.decl_end(); I != E; ++I)
>> +    ++DeclCount;
>>
>> (after chatting with Chandler about this on irc):
>> I'd use Node.decl_end() - Node.decl_begin(). If it ever becomes a
>> non-const-time operation, the iterator will not implement the
>> interface and break compilation, so we'll notice.
>
> But do we need to notice? If the original algorithm written here is
> linear it seems like constant time size is not a requirement.
>
> If that's the case, then std::distance just DTRT - constant time for

I personally am fine with arguing for std::distance. My point is not
to write the loop :)

> random access iterators, linear for others. (alternatively, provide
> Node::decl_size that does the same thing - but I can understand the
> concern of providing a (possibly in the future) non-constant-time
> size, though at that point you could remove size & go back & examine
> each client to see which ones care about that)
>
>>
>> Regardless of that, I think your comment is wrong in 2 ways: first,
>> there's a typo :) Second, that the iterator happens to come down do
>> being a pointer has nothing to do with its contract. It either
>> provides random access or not.
>>
>> +/// \brief Matches expressions that match InnerMatcher after implicit casts are
>> +/// stripped off.
>> +AST_MATCHER_P(Expr, ignoreImplicitCasts,
>> +              internal::Matcher<Expr>, InnerMatcher) {
>> +  return InnerMatcher.matches(*Node.IgnoreImpCasts(), Finder, Builder);
>> +}
>>
>> I think we should implement the equivalent based on ignoreParenImpCast
>> first, as that's what I've seen us needing much more often (we can
>> implement this one, too, of course ;)
>>
>> Cheers,
>> /Manuel
>>
>> On Fri, Jul 13, 2012 at 8:34 PM, Sam Panzer <panzer at google.com> wrote:
>>> <div>Attached are three more small matcher patches. One fixes another
>>> rename typo (AnyOf --> anyOf) that was similar to the previous
>>> allOf patch. The second patch adds more inspection for
>>> declarationStatement matchers, making it easier to look at single
>>> declarations directly. The third patch adds expression matchers which
>>> call IgnoreXXXCasts() before  applying their
>>> sub-matchers.</div><div><br></div>For future reference, should I
>>> continue splitting up these patches for
>>> review?<div><br></div><div>-Sam</div>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list