<br><div class="gmail_extra">This thread forked from a previous group of patches, now with a new and improved version of additional cast-related matchers.<br><br><div class="gmail_quote">On Sun, Jul 15, 2012 at 1:44 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Sam,<div><br></div><div><br></div><div>ignore-casts.patch:</div>
<div>First of, I think these methods are going to be really helpful!</div><div><br></div><div>I am not sure about the naming. I am not a native speaker, but:</div><div> variable(hasInitializer(ignoreImplicitCasts(expression())))</div>
<div>feels a bit "rough". How about:</div><div> variable(hasInitializer(ignoringImplicitCasts(expression())))</div><div>Still does not feel ideal. Any thoughts?</div><div><br></div></blockquote><div> </div><div>
I went with "ignoringImplicitCasts" as suggested though "withoutImplicitCasts" could also work.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div></div><div>The implementation definitely needs more elaborate comments with examples of what the matchers are supposed to match.</div></blockquote><div><br></div><div>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.</div>
<div>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:</div><div><br></div><div> AST_MATCHER_METHOD(Expr, ignoringParenAndImplicitCasts, internal::Matcher<Expr>, IgnoreParenImpCasts);</div>
<div>which expands to the same result as</div><div> AST_MATCHER_P(Expr, ignoringParenAndImplicitCasts, internal::Matcher<Expr>, InnerMatcher) {</div><div> return InnerMatcher.matches(*Node.IgnoreParenImpCasts(), Finder, Builder);</div>
<div> }</div><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">And the tests should also include these examples with both positive and negative cases.</blockquote>
<div><br></div><div>Tests added!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>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.</div>
<div class=""><div class="h5"><br></div></div></blockquote><div><br></div><div>You're right: I didn't realize that ignoreParenCasts meant stripping parentheses. This has been fixed :)</div><div><br></div><div>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).</div>
<div><br></div></div><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">
On Fri, Jul 13, 2012 at 8:34 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>Attached are three more small matcher patches. One fixes another<br>
rename typo (AnyOf --> anyOf) that was similar to the previous<br>
allOf patch. The second patch adds more inspection for<br>
declarationStatement matchers, making it easier to look at single<br>
declarations directly. The third patch adds expression matchers which<br>
call IgnoreXXXCasts() before applying their<br>
sub-matchers.</div><div><br></div>For future reference, should I<br>
continue splitting up these patches for<br>
review?<div><br></div><div>-Sam</div><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>