This thread forked from a previous group of patches, now with a new and improved version of additional cast-related matchers.
On Sun, Jul 15, 2012 at 1:44 PM, Daniel Jasper <[email protected]> wrote: > Hi Sam, > > > ignore-casts.patch: > First of, I think these methods are going to be really helpful! > > I am not sure about the naming. I am not a native speaker, but: > variable(hasInitializer(ignoreImplicitCasts(expression()))) > feels a bit "rough". How about: > variable(hasInitializer(ignoringImplicitCasts(expression()))) > Still does not feel ideal. Any thoughts? > > I went with "ignoringImplicitCasts" as suggested though "withoutImplicitCasts" could also work. > The implementation definitely needs more elaborate comments with examples > of what the matchers are supposed to match. > I added much more in-depth comments, complete with examples that differentiate the different kinds of casts involved here. It almost seems that I'm duplicating some AST documentation though, since these matchers just forward a method call to the inner matcher. In this case, ignoringParenCasts does exactly what Expr::IgnoreParenCasts() is supposed to do. This made me wonder if it would be reasonable to create a macro that makes writing this kind of trivial matcher easier. I imagine something like this: AST_MATCHER_METHOD(Expr, ignoringParenAndImplicitCasts, internal::Matcher<Expr>, IgnoreParenImpCasts); which expands to the same result as AST_MATCHER_P(Expr, ignoringParenAndImplicitCasts, internal::Matcher<Expr>, InnerMatcher) { return InnerMatcher.matches(*Node.IgnoreParenImpCasts(), Finder, Builder); } Though this would only be useful if there are a good number of accessors to add, though I found around 15 existing matchers that just forwarded one method call and about 15 more matchers test the result of a method call for NULL before passing it on to an inner matcher. I expect that including a full predicate would be overkill, though. > And the tests should also include these examples with both positive and > negative cases. Tests added! > Also, I think you misunderstand what the ignoreParenCasts is mainly > supposed to do. A ParenExpr is added if you put parenthesis around an > expression. Thus, the ignoreParenCasts-method mainly handles cases like > "int i = (0);". It also strips of casts and thus the CStyleCastExpr in > "const int y = (const int) x;". This needs documentation and tests. > > You're right: I didn't realize that ignoreParenCasts meant stripping parentheses. This has been fixed :) I also added a CastExpression matcher and changed the hasSourceExpression matcher to require any CastExpression, rather than an ImplicitCastExpression, as I wanted to use it to match an explicit cast to any integer type (since some for loop authors cast to int rather than using unsigned loop indices). > 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> >> > >
cast-matchers.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
