PiotrZSL updated this revision to Diff 547502. PiotrZSL added a comment. Rebase, add comments, make more strict for method (must be non-const)
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157185/new/ https://reviews.llvm.org/D157185 Files: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp Index: clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp +++ clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp @@ -201,3 +201,17 @@ template <typename T> void swap(OK21<T> &, OK21<T> &) noexcept(noexcept(TemplateNoexceptWithInt<int>::f())); void swap(OK21<int> &, OK21<int> &) noexcept(noexcept(TemplateNoexceptWithInt<int>::f())); + +namespace PR64303 { + void swap(); + void swap(int&, bool&); + void swap(int&, int&, int&); + void swap(int&); + + struct Test { + void swap(); + void swap(Test&, Test&); + void swap(int&); + static void swap(int&, int&); + }; +} Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -172,6 +172,10 @@ <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for ``inline`` namespaces in the same format as :program:`clang-format`. +- Improved :doc:`performanc-noexcept-swap + <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter + match with the swap function signature, eliminating false-positives. + Removed checks ^^^^^^^^^^^^^^ Index: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp +++ clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp @@ -17,9 +17,30 @@ namespace clang::tidy::performance { void NoexceptSwapCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - functionDecl(unless(isDeleted()), hasName("swap")).bind(BindFuncDeclName), - this); + + // Match non-const method with single argument that is non-const reference to + // a class type that owns method and return void void Class::swap(Class&) + auto MethodMatcher = cxxMethodDecl( + parameterCountIs(1U), unless(isConst()), returns(voidType()), + hasParameter(0, hasType(qualType(hasCanonicalType( + qualType(unless(isConstQualified()), + references(namedDecl().bind("class"))))))), + ofClass(equalsBoundNode("class"))); + + // Match function with 2 arguments, both are non-const references to same type + // and return void void swap(Type&, Type&) + auto FunctionMatcher = allOf( + unless(cxxMethodDecl()), parameterCountIs(2U), returns(voidType()), + hasParameter( + 0, hasType(qualType(hasCanonicalType( + qualType(unless(isConstQualified()), references(qualType())) + .bind("type"))))), + hasParameter(1, hasType(qualType(hasCanonicalType( + qualType(equalsBoundNode("type"))))))); + Finder->addMatcher(functionDecl(unless(isDeleted()), hasName("swap"), + anyOf(MethodMatcher, FunctionMatcher)) + .bind(BindFuncDeclName), + this); } DiagnosticBuilder
Index: clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp +++ clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp @@ -201,3 +201,17 @@ template <typename T> void swap(OK21<T> &, OK21<T> &) noexcept(noexcept(TemplateNoexceptWithInt<int>::f())); void swap(OK21<int> &, OK21<int> &) noexcept(noexcept(TemplateNoexceptWithInt<int>::f())); + +namespace PR64303 { + void swap(); + void swap(int&, bool&); + void swap(int&, int&, int&); + void swap(int&); + + struct Test { + void swap(); + void swap(Test&, Test&); + void swap(int&); + static void swap(int&, int&); + }; +} Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -172,6 +172,10 @@ <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for ``inline`` namespaces in the same format as :program:`clang-format`. +- Improved :doc:`performanc-noexcept-swap + <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter + match with the swap function signature, eliminating false-positives. + Removed checks ^^^^^^^^^^^^^^ Index: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp +++ clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp @@ -17,9 +17,30 @@ namespace clang::tidy::performance { void NoexceptSwapCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - functionDecl(unless(isDeleted()), hasName("swap")).bind(BindFuncDeclName), - this); + + // Match non-const method with single argument that is non-const reference to + // a class type that owns method and return void void Class::swap(Class&) + auto MethodMatcher = cxxMethodDecl( + parameterCountIs(1U), unless(isConst()), returns(voidType()), + hasParameter(0, hasType(qualType(hasCanonicalType( + qualType(unless(isConstQualified()), + references(namedDecl().bind("class"))))))), + ofClass(equalsBoundNode("class"))); + + // Match function with 2 arguments, both are non-const references to same type + // and return void void swap(Type&, Type&) + auto FunctionMatcher = allOf( + unless(cxxMethodDecl()), parameterCountIs(2U), returns(voidType()), + hasParameter( + 0, hasType(qualType(hasCanonicalType( + qualType(unless(isConstQualified()), references(qualType())) + .bind("type"))))), + hasParameter(1, hasType(qualType(hasCanonicalType( + qualType(equalsBoundNode("type"))))))); + Finder->addMatcher(functionDecl(unless(isDeleted()), hasName("swap"), + anyOf(MethodMatcher, FunctionMatcher)) + .bind(BindFuncDeclName), + this); } DiagnosticBuilder
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits