r264855 - [ASTMatchers] Existing matcher hasAnyArgument fixed

Gábor Horváth via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 30 05:25:12 PDT 2016


Sure, this should be fixed in http://reviews.llvm.org/rL264859

On 30 March 2016 at 14:24, Nico Weber <thakis at chromium.org> wrote:

> Looks like this broke clang-tidy/misc-dangling-handle.cpp (
> http://lab.llvm.org:8011/builders/clang-bpf-build/builds/9704). Can you
> take a look?
>
> On Wed, Mar 30, 2016 at 4:22 AM, Gabor Horvath via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Wed Mar 30 06:22:14 2016
>> New Revision: 264855
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=264855&view=rev
>> Log:
>> [ASTMatchers] Existing matcher hasAnyArgument fixed
>>
>> 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.
>>
>> Reviewers: klimek, sbenza, alexfh
>>
>> Subscribers: alexfh, klimek, xazax.hun, cfe-commits
>>
>> Differential Revision: http://reviews.llvm.org/D18243
>>
>> Modified:
>>     cfe/trunk/docs/LibASTMatchersReference.html
>>     cfe/trunk/docs/ReleaseNotes.rst
>>     cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
>>     cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
>>
>> Modified: cfe/trunk/docs/LibASTMatchersReference.html
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/docs/LibASTMatchersReference.html (original)
>> +++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 30 06:22:14 2016
>> @@ -3636,11 +3636,6 @@ callExpr(hasAnyArgument(declRefExpr()))
>>    matches x(1, y, 42)
>>  with hasAnyArgument(...)
>>    matching y
>> -
>> -FIXME: Currently this will ignore parentheses and implicit casts on
>> -the argument before applying the inner matcher. We'll want to remove
>> -this to allow for greater control by the user once ignoreImplicit()
>> -has been implemented.
>>  </pre></td></tr>
>>
>>
>> @@ -3907,11 +3902,6 @@ callExpr(hasAnyArgument(declRefExpr()))
>>    matches x(1, y, 42)
>>  with hasAnyArgument(...)
>>    matching y
>> -
>> -FIXME: Currently this will ignore parentheses and implicit casts on
>> -the argument before applying the inner matcher. We'll want to remove
>> -this to allow for greater control by the user once ignoreImplicit()
>> -has been implemented.
>>  </pre></td></tr>
>>
>>
>>
>> Modified: cfe/trunk/docs/ReleaseNotes.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/docs/ReleaseNotes.rst (original)
>> +++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 30 06:22:14 2016
>> @@ -120,6 +120,12 @@ this section should help get you past th
>>  AST Matchers
>>  ------------
>>
>> +- hasAnyArgument: Matcher no longer ignores parentheses and implicit
>> casts on
>> +  the argument before applying the inner matcher. The fix was done to
>> allow for
>> +  greater control by the user. In all existing checkers that use this
>> matcher
>> +  all instances of code ``hasAnyArgument(<inner matcher>)`` must be
>> changed to
>> +  ``hasAnyArgument(ignoringParenImpCasts(<inner matcher>))``.
>> +
>>  ...
>>
>>  libclang
>>
>> Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
>> +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Mar 30 06:22:14
>> 2016
>> @@ -2989,18 +2989,13 @@ AST_MATCHER(CXXCtorInitializer, isMember
>>  ///   matches x(1, y, 42)
>>  /// with hasAnyArgument(...)
>>  ///   matching y
>> -///
>> -/// FIXME: Currently this will ignore parentheses and implicit casts on
>> -/// the argument before applying the inner matcher. We'll want to remove
>> -/// this to allow for greater control by the user once \c
>> ignoreImplicit()
>> -/// has been implemented.
>>  AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
>>                            AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
>>
>>  CXXConstructExpr),
>>                            internal::Matcher<Expr>, InnerMatcher) {
>>    for (const Expr *Arg : Node.arguments()) {
>>      BoundNodesTreeBuilder Result(*Builder);
>> -    if (InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder,
>> &Result)) {
>> +    if (InnerMatcher.matches(*Arg, Finder, &Result)) {
>>        *Builder = std::move(Result);
>>        return true;
>>      }
>>
>> Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
>> +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Wed Mar 30
>> 06:22:14 2016
>> @@ -1633,10 +1633,15 @@ TEST(Matcher, Argument) {
>>
>>  TEST(Matcher, AnyArgument) {
>>    StatementMatcher CallArgumentY = callExpr(
>> -      hasAnyArgument(declRefExpr(to(varDecl(hasName("y"))))));
>> +      hasAnyArgument(
>> +
>> ignoringParenImpCasts(declRefExpr(to(varDecl(hasName("y")))))));
>>    EXPECT_TRUE(matches("void x(int, int) { int y; x(1, y); }",
>> CallArgumentY));
>>    EXPECT_TRUE(matches("void x(int, int) { int y; x(y, 42); }",
>> CallArgumentY));
>>    EXPECT_TRUE(notMatches("void x(int, int) { x(1, 2); }",
>> CallArgumentY));
>> +
>> +  StatementMatcher ImplicitCastedArgument = callExpr(
>> +      hasAnyArgument(implicitCastExpr()));
>> +  EXPECT_TRUE(matches("void x(long) { int y; x(y); }",
>> ImplicitCastedArgument));
>>  }
>>
>>  TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160330/4974f18b/attachment-0001.html>


More information about the cfe-commits mailing list