I also noticed that a hasDeclaration matcher which serves a different purpose. I think the new hasDecl matcher needs a new name...
On Mon, Jul 16, 2012 at 3:45 PM, Sam Panzer <[email protected]> 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 <[email protected]> wrote: > >> >> On Mon, Jul 16, 2012 at 6:22 PM, David Blaikie <[email protected]> >> wrote: >> > On Mon, Jul 16, 2012 at 12:03 AM, Manuel Klimek <[email protected]> >> 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 <[email protected]> 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 >> >>> [email protected] >> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >>> >> >> _______________________________________________ >> >> cfe-commits mailing list >> >> [email protected] >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
