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

[Manuel Klimek]

On Tue, Jul 17, 2012 at 12:47 AM, Sam Panzer <panzer at google.com> wrote:
> I also noticed that a hasDeclaration matcher which serves a different
> purpose. I think the new hasDecl matcher needs a new name...

Good point. Any ideas for how to differentiate "hasDeclaration" in
terms of "something that declares what's used here" vs.
"hasDeclaration" in terms of "aggregates a declaration that matches"?

>
>
> On Mon, Jul 16, 2012 at 3:45 PM, Sam Panzer <panzer at google.com> wrote:
>>
>> Here's a new version of the DeclStmt patch. Changes include:
>>  - Fixed comments by declCountIs and hasSingleDecl
>>  - Added hasDecl in the spirit of hasArgument
>>  - Changed the loop to std::distance (std::advance in hasDecl)
>>  - Added a few more test cases.
>>
>> 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:
>>   int a, b;  // toplevel declarations
>> 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?
>>
>> The other patch will be sent on a fork of the previous discussion.
>> Any new comments?
>>
>> On Mon, Jul 16, 2012 at 9:39 AM, Manuel Klimek <klimek at google.com> wrote:
>>>
>>>
>>> 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
>>
>>
>