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

[Sam Panzer]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120716/e3ae385a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: decls.patch
Type: application/octet-stream
Size: 4523 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120716/e3ae385a/attachment.obj>