<div dir="ltr">Sure, this should be fixed in <a href="http://reviews.llvm.org/rL264859">http://reviews.llvm.org/rL264859</a><br></div><div class="gmail_extra"><br><div class="gmail_quote">On 30 March 2016 at 14:24, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Looks like this broke clang-tidy/misc-dangling-handle.cpp (<a href="http://lab.llvm.org:8011/builders/clang-bpf-build/builds/9704" target="_blank">http://lab.llvm.org:8011/builders/clang-bpf-build/builds/9704</a>). Can you take a look?</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 30, 2016 at 4:22 AM, Gabor Horvath via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: xazax<br>
Date: Wed Mar 30 06:22:14 2016<br>
New Revision: 264855<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=264855&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=264855&view=rev</a><br>
Log:<br>
[ASTMatchers] Existing matcher hasAnyArgument fixed<br>
<br>
Summary: A checker (will be uploaded after this patch) needs to check implicit casts. The checker needs matcher hasAnyArgument but it ignores implicit casts and parenthesized expressions which disables checking of implicit casts for arguments in the checker. However the documentation of the matcher contains a FIXME that this should be removed once separate matchers for ignoring implicit casts and parenthesized expressions are ready. Since these matchers were already there the fix could be executed. Only one Clang checker was affected which was also fixed (ignoreParenImpCasts added) and is separately uploaded. Third party checkers (not in the Clang repository) may be affected by this fix so the fix must be emphasized in the release notes.<br>
<br>
Reviewers: klimek, sbenza, alexfh<br>
<br>
Subscribers: alexfh, klimek, xazax.hun, cfe-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D18243" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18243</a><br>
<br>
Modified:<br>
cfe/trunk/docs/LibASTMatchersReference.html<br>
cfe/trunk/docs/ReleaseNotes.rst<br>
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h<br>
cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp<br>
<br>
Modified: cfe/trunk/docs/LibASTMatchersReference.html<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=264855&r1=264854&r2=264855&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=264855&r1=264854&r2=264855&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/docs/LibASTMatchersReference.html (original)<br>
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 30 06:22:14 2016<br>
@@ -3636,11 +3636,6 @@ callExpr(hasAnyArgument(declRefExpr()))<br>
matches x(1, y, 42)<br>
with hasAnyArgument(...)<br>
matching y<br>
-<br>
-FIXME: Currently this will ignore parentheses and implicit casts on<br>
-the argument before applying the inner matcher. We'll want to remove<br>
-this to allow for greater control by the user once ignoreImplicit()<br>
-has been implemented.<br>
</pre></td></tr><br>
<br>
<br>
@@ -3907,11 +3902,6 @@ callExpr(hasAnyArgument(declRefExpr()))<br>
matches x(1, y, 42)<br>
with hasAnyArgument(...)<br>
matching y<br>
-<br>
-FIXME: Currently this will ignore parentheses and implicit casts on<br>
-the argument before applying the inner matcher. We'll want to remove<br>
-this to allow for greater control by the user once ignoreImplicit()<br>
-has been implemented.<br>
</pre></td></tr><br>
<br>
<br>
<br>
Modified: cfe/trunk/docs/ReleaseNotes.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=264855&r1=264854&r2=264855&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=264855&r1=264854&r2=264855&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/docs/ReleaseNotes.rst (original)<br>
+++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 30 06:22:14 2016<br>
@@ -120,6 +120,12 @@ this section should help get you past th<br>
AST Matchers<br>
------------<br>
<br>
+- hasAnyArgument: Matcher no longer ignores parentheses and implicit casts on<br>
+ the argument before applying the inner matcher. The fix was done to allow for<br>
+ greater control by the user. In all existing checkers that use this matcher<br>
+ all instances of code ``hasAnyArgument(<inner matcher>)`` must be changed to<br>
+ ``hasAnyArgument(ignoringParenImpCasts(<inner matcher>))``.<br>
+<br>
...<br>
<br>
libclang<br>
<br>
Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=264855&r1=264854&r2=264855&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=264855&r1=264854&r2=264855&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)<br>
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Mar 30 06:22:14 2016<br>
@@ -2989,18 +2989,13 @@ AST_MATCHER(CXXCtorInitializer, isMember<br>
/// matches x(1, y, 42)<br>
/// with hasAnyArgument(...)<br>
/// matching y<br>
-///<br>
-/// FIXME: Currently this will ignore parentheses and implicit casts on<br>
-/// the argument before applying the inner matcher. We'll want to remove<br>
-/// this to allow for greater control by the user once \c ignoreImplicit()<br>
-/// has been implemented.<br>
AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,<br>
AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,<br>
CXXConstructExpr),<br>
internal::Matcher<Expr>, InnerMatcher) {<br>
for (const Expr *Arg : Node.arguments()) {<br>
BoundNodesTreeBuilder Result(*Builder);<br>
- if (InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, &Result)) {<br>
+ if (InnerMatcher.matches(*Arg, Finder, &Result)) {<br>
*Builder = std::move(Result);<br>
return true;<br>
}<br>
<br>
Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=264855&r1=264854&r2=264855&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=264855&r1=264854&r2=264855&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)<br>
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Wed Mar 30 06:22:14 2016<br>
@@ -1633,10 +1633,15 @@ TEST(Matcher, Argument) {<br>
<br>
TEST(Matcher, AnyArgument) {<br>
StatementMatcher CallArgumentY = callExpr(<br>
- hasAnyArgument(declRefExpr(to(varDecl(hasName("y"))))));<br>
+ hasAnyArgument(<br>
+ ignoringParenImpCasts(declRefExpr(to(varDecl(hasName("y")))))));<br>
EXPECT_TRUE(matches("void x(int, int) { int y; x(1, y); }", CallArgumentY));<br>
EXPECT_TRUE(matches("void x(int, int) { int y; x(y, 42); }", CallArgumentY));<br>
EXPECT_TRUE(notMatches("void x(int, int) { x(1, 2); }", CallArgumentY));<br>
+<br>
+ StatementMatcher ImplicitCastedArgument = callExpr(<br>
+ hasAnyArgument(implicitCastExpr()));<br>
+ EXPECT_TRUE(matches("void x(long) { int y; x(y); }", ImplicitCastedArgument));<br>
}<br>
<br>
TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</blockquote></div><br></div>