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

2024-01-09 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL approved this pull request.


https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-01-06 Thread via cfe-commits 

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

>From 97eeda4684804229d9faad19ea7b7888dcd91786 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Thu, 21 Dec 2023 02:30:34 +
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 ) {
 
 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 
Date: Thu, 28 Dec 2023 01:30:35 +
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
+  ` check by extending the default
+  check function names to include ``iter_swap`` and ``iter_move``.
+
 - Improved :doc:`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 
Date: Sun, 7 Jan 2024 02:25:47 +
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

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

2024-01-04 Thread Piotr Zegar via cfe-commits 


@@ -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&) {

PiotrZSL wrote:

One argument would be better. Just to keep it in sync with standard.
But thats not a blocker.

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-01-04 Thread via cfe-commits 


@@ -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&) {

Da-Viper wrote:

Yeah the std:: common_iterator::iter_move

But the check checks only if the function name is iter_move then performs the 
check.

If you do prefer I could change it to only one arg

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-01-04 Thread Piotr Zegar via cfe-commits 


@@ -58,14 +58,15 @@ void 
ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap ) {
 
 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")),

PiotrZSL wrote:

NOTE: Here is old legacy bug. For example user can have method swap(), and it 
will still be counted in even if not needed. Would be good to check if those 
methods/functions got at least 1 parameter.
Also you can consider updating performance-noexcept-swap by adding there 
iter_swap.

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-01-04 Thread Piotr Zegar via cfe-commits 


@@ -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&) {

PiotrZSL wrote:

NOTE: Isn't iter_move taking single argument (iterator) ?

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-01-04 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL edited 
https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-01-04 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL approved this pull request.

LGTM. 
Looks fine even in current stage.

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-12-27 Thread via cfe-commits 

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

>From 97eeda4684804229d9faad19ea7b7888dcd91786 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Thu, 21 Dec 2023 02:30:34 +
Subject: [PATCH 1/2] 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 ) {
 
 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 
Date: Thu, 28 Dec 2023 01:30:35 +
Subject: [PATCH 2/2] [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
+  ` check by extending the default
+  check function names to include ``iter_swap`` and ``iter_move``.
+
 - Improved :doc:`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 {};
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-12-25 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL requested changes to this pull request.

Missing tests, documentation, release notes.

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-12-21 Thread via cfe-commits 

EugeneZelenko wrote:

Should Release Notes be updated?

https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-12-20 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-tidy

Author: None (Da-Viper)


Changes

Fixes  #75686

---
Full diff: https://github.com/llvm/llvm-project/pull/76117.diff


1 Files Affected:

- (modified) clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp 
(+9-8) 


``diff
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 ) {
 
 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);
 }

``




https://github.com/llvm/llvm-project/pull/76117
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-12-20 Thread via cfe-commits 

https://github.com/Da-Viper created 
https://github.com/llvm/llvm-project/pull/76117

Fixes  #75686

>From 97eeda4684804229d9faad19ea7b7888dcd91786 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Thu, 21 Dec 2023 02:30:34 +
Subject: [PATCH] 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 ) {
 
 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);
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits