[llvm] [clang-tools-extra] [clang] Fix #75686: add iter_swap and iter_move to the matched name (PR #76117)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 14 00:42:31 PST 2024


https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/76117

>From 97eeda4684804229d9faad19ea7b7888dcd91786 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Thu, 21 Dec 2023 02:30:34 +0000
Subject: [PATCH 1/6] Fix #75686: add iter_swap and iter_move to the matched
 name

---
 .../bugprone/ExceptionEscapeCheck.cpp           | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
index 90bf523ffb00b6..18cd7150185a20 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -58,14 +58,15 @@ void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 
 void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      functionDecl(isDefinition(),
-                   anyOf(isNoThrow(),
-                         allOf(anyOf(cxxDestructorDecl(),
-                                     cxxConstructorDecl(isMoveConstructor()),
-                                     cxxMethodDecl(isMoveAssignmentOperator()),
-                                     isMain(), hasName("swap")),
-                               unless(isExplicitThrow())),
-                         isEnabled(FunctionsThatShouldNotThrow)))
+      functionDecl(
+          isDefinition(),
+          anyOf(isNoThrow(),
+                allOf(anyOf(cxxDestructorDecl(),
+                            cxxConstructorDecl(isMoveConstructor()),
+                            cxxMethodDecl(isMoveAssignmentOperator()), isMain(),
+                            hasAnyName("swap", "iter_swap", "iter_move")),
+                      unless(isExplicitThrow())),
+                isEnabled(FunctionsThatShouldNotThrow)))
           .bind("thrower"),
       this);
 }

>From c7cbf4c9bebbf450410c2679af188672257895aa Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Thu, 28 Dec 2023 01:30:35 +0000
Subject: [PATCH 2/6] [clang-tidy] Update documentation, release notes and
 tests for bugprone-exception-escape

---
 clang-tools-extra/docs/ReleaseNotes.rst                |  4 ++++
 .../clang-tidy/checks/bugprone/exception-escape.rst    |  2 ++
 .../clang-tidy/checkers/bugprone/exception-escape.cpp  | 10 ++++++++++
 3 files changed, 16 insertions(+)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..967597cbba11b1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -242,6 +242,10 @@ Changes in existing checks
   casting during type conversions at variable initialization, now with improved
   compatibility for C++17 and later versions.
 
+- Improved :doc:`bugprone-exception-escape
+  <clang-tidy/checks/bugprone/exception-escape>` check by extending the default
+  check function names to include ``iter_swap`` and ``iter_move``.
+
 - Improved :doc:`bugprone-lambda-function-name
   <clang-tidy/checks/bugprone/lambda-function-name>` check by adding option
   `IgnoreMacros` to ignore warnings in macros.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
index e6aa8e001492a6..182fade7f47a03 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
@@ -11,6 +11,8 @@ should not. The functions which should not throw exceptions are the following:
 * Move assignment operators
 * The ``main()`` functions
 * ``swap()`` functions
+* ``iter_swap()`` functions
+* ``iter_move()`` functions
 * Functions marked with ``throw()`` or ``noexcept``
 * Other functions given as option
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
index 4a7149e81ce7e5..e20aa267392f55 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -586,6 +586,16 @@ void swap(int&, int&) {
   throw 1;
 }
 
+void iter_swap(int&, int&) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'iter_swap' which should not throw exceptions
+  throw 1;
+}
+
+void iter_move(int&, int&) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'iter_move' which should not throw exceptions
+  throw 1;
+}
+
 namespace std {
 class bad_alloc {};
 }

>From 0accdc7d74eda5719a1415588e704e2f8dff58c4 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 7 Jan 2024 02:25:47 +0000
Subject: [PATCH 3/6] [clang-tidy] exception-escape test: iter_move function
 should have only one parameter

---
 .../test/clang-tidy/checkers/bugprone/exception-escape.cpp      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
index e20aa267392f55..f5e74df1621cee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -591,7 +591,7 @@ void iter_swap(int&, int&) {
   throw 1;
 }
 
-void iter_move(int&, int&) {
+void iter_move(int&) {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'iter_move' which should not throw exceptions
   throw 1;
 }

>From 4b4733e2554aebecb12c5a2f67a2ea546ae234e1 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 7 Jan 2024 03:02:23 +0000
Subject: [PATCH 4/6] [clang-tidy] exception escape: the matched named
 functions should have at least one parameter

---
 .../clang-tidy/bugprone/ExceptionEscapeCheck.cpp           | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
index 18cd7150185a20..620a57194acb8e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -28,6 +28,10 @@ AST_MATCHER(FunctionDecl, isExplicitThrow) {
          Node.getExceptionSpecSourceRange().isValid();
 }
 
+AST_MATCHER(FunctionDecl, hasAtLeastOneParameter) {
+  return Node.getNumParams() > 0;
+}
+
 } // namespace
 
 ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
@@ -64,7 +68,8 @@ void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
                 allOf(anyOf(cxxDestructorDecl(),
                             cxxConstructorDecl(isMoveConstructor()),
                             cxxMethodDecl(isMoveAssignmentOperator()), isMain(),
-                            hasAnyName("swap", "iter_swap", "iter_move")),
+                            allOf(hasAnyName("swap", "iter_swap", "iter_move"),
+                                  hasAtLeastOneParameter())),
                       unless(isExplicitThrow())),
                 isEnabled(FunctionsThatShouldNotThrow)))
           .bind("thrower"),

>From 9178a2a7444a1f6cd5024fee1bc54c7d6af5e784 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 7 Jan 2024 03:05:28 +0000
Subject: [PATCH 5/6] [clang-tidy] performance-noexcept-swap add `iter_swap` as
 matched function name

---
 clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
index 25a58af74f7ee8..e7cba6e54e86a9 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
@@ -39,7 +39,8 @@ void NoexceptSwapCheck::registerMatchers(MatchFinder *Finder) {
                      .bind("type"))))),
       hasParameter(1, hasType(qualType(hasCanonicalType(
                           qualType(equalsBoundNode("type")))))));
-  Finder->addMatcher(functionDecl(unless(isDeleted()), hasName("swap"),
+  Finder->addMatcher(functionDecl(unless(isDeleted()),
+                                  hasAnyName("swap", "iter_swap"),
                                   anyOf(MethodMatcher, FunctionMatcher))
                          .bind(BindFuncDeclName),
                      this);

>From 6a844af2d72cda9276b0d1a332655b0cc1722bee Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 7 Jan 2024 03:18:51 +0000
Subject: [PATCH 6/6] [clang-tidy] performance-noexcept-swap: update the docs,
 release note and tests.

---
 clang-tools-extra/docs/ReleaseNotes.rst                       | 3 ++-
 .../docs/clang-tidy/checks/performance/noexcept-swap.rst      | 4 ++--
 .../test/clang-tidy/checkers/performance/noexcept-swap.cpp    | 4 ++++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 967597cbba11b1..63f01ca93c6d25 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -413,7 +413,8 @@ Changes in existing checks
 - Improved :doc:`performance-noexcept-swap
   <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter
   match with the swap function signature and better handling of condition
-  noexcept expressions, eliminating false-positives.
+  noexcept expressions, eliminating false-positives. ``iter_swap`` function name
+  is checked by default.
 
 - Improved :doc:`readability-braces-around-statements
   <clang-tidy/checks/readability/braces-around-statements>` check to
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/noexcept-swap.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/noexcept-swap.rst
index fa4007618ba0f2..2901d721706c8d 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/noexcept-swap.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/noexcept-swap.rst
@@ -3,11 +3,11 @@
 performance-noexcept-swap
 =========================
 
-The check flags user-defined swap functions not marked with ``noexcept`` or
+The check flags user-defined swap and iter_swap functions not marked with ``noexcept`` or
 marked with ``noexcept(expr)`` where ``expr`` evaluates to ``false``
 (but is not a ``false`` literal itself).
 
-When a swap function is marked as ``noexcept``, it assures the compiler that
+When a swap or iter_swap function is marked as ``noexcept``, it assures the compiler that
 no exceptions will be thrown during the swapping of two objects, which allows
 the compiler to perform certain optimizations such as omitting exception
 handling code.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp
index dfc71a2bb9ab37..6b266707d77f85 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp
@@ -32,6 +32,10 @@ void swap(A &, A &);
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: swap functions should be marked noexcept [performance-noexcept-swap]
 // CHECK-FIXES: void swap(A &, A &) noexcept ;
 
+void iter_swap(A &, A &);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: swap functions should be marked noexcept [performance-noexcept-swap]
+// CHECK-FIXES: void iter_swap(A &, A &) noexcept ;
+
 struct B {
   static constexpr bool kFalse = false;
   void swap(B &) noexcept(kFalse);



More information about the cfe-commits mailing list