[clang-tools-extra] f9bd62f - [clang-tidy] Improve `ExceptionSpecAnalyzer`s handling of conditional noexcept expressions (#68359)

[via cfe-commits]

Author: AMS21
Date: 2023-10-09T16:40:18+02:00
New Revision: f9bd62fb8f666e818a4bf0f0dbcb77a0a223c97a

URL: https://github.com/llvm/llvm-project/commit/f9bd62fb8f666e818a4bf0f0dbcb77a0a223c97a
DIFF: https://github.com/llvm/llvm-project/commit/f9bd62fb8f666e818a4bf0f0dbcb77a0a223c97a.diff

LOG: [clang-tidy] Improve `ExceptionSpecAnalyzer`s handling of conditional noexcept expressions (#68359)

The previous code was pretty messy and treated value dependant
expressions which could not be evaluated the same as if they evaluted to
`false`. Which was obviously not correct.

We now check if we can evaluate the dependant expressions and if not we
truthfully return that we don't know if the function is declared as
`noexcept` or not.

This fixes #68101

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
    clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-swap.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
index 4dd4a95f880aca0..1dde0490517852d 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
@@ -142,13 +142,22 @@ ExceptionSpecAnalyzer::analyzeFunctionEST(const FunctionDecl *FuncDecl,
     return State::NotThrowing;
   case CT_Dependent: {
     const Expr *NoexceptExpr = FuncProto->getNoexceptExpr();
+    if (!NoexceptExpr)
+      return State::NotThrowing;
+
+    // We can't resolve value dependence so just return unknown
+    if (NoexceptExpr->isValueDependent())
+      return State::Unknown;
+
+    // Try to evaluate the expression to a boolean value
     bool Result = false;
-    return (NoexceptExpr && !NoexceptExpr->isValueDependent() &&
-            NoexceptExpr->EvaluateAsBooleanCondition(
-                Result, FuncDecl->getASTContext(), true) &&
-            Result)
-               ? State::NotThrowing
-               : State::Throwing;
+    if (NoexceptExpr->EvaluateAsBooleanCondition(
+            Result, FuncDecl->getASTContext(), true))
+      return Result ? State::NotThrowing : State::Throwing;
+
+    // The noexcept expression is not value dependent but we can't evaluate it
+    // as a boolean condition so we have no idea if its throwing or not
+    return State::Unknown;
   }
   default:
     return State::Throwing;

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 60d92ccf971490e..60b82f4c50dd715 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -294,9 +294,14 @@ Changes in existing checks
   <clang-tidy/checks/performance/faster-string-find>` check to properly escape
   single quotes.
 
+- Improved :doc:`performance-noexcept-move-constructor
+  <clang-tidy/checks/performance/noexcept-move-constructor>` to better handle
+  conditional noexcept expressions, eliminating false-positives.
+
 - Improved :doc:`performance-noexcept-swap
   <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter
-  match with the swap function signature, eliminating false-positives.
+  match with the swap function signature and better handling of condition
+  noexcept expressions, eliminating false-positives.
 
 - Improved :doc:`readability-braces-around-statements
   <clang-tidy/checks/readability/braces-around-statements>` check to

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
index 60596c2876c0a34..347a1e3220061af 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
@@ -1,5 +1,14 @@
 // RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t -- -- -fexceptions
 
+namespace std
+{
+  template <typename T>
+  struct is_nothrow_move_constructible
+  {
+    static constexpr bool value = __is_nothrow_constructible(T, __add_rvalue_reference(T));
+  };
+} // namespace std
+
 struct Empty
 {};
 
@@ -379,3 +388,12 @@ struct OK31 {
   OK31(OK31 &&) noexcept(TrueT<int>::value) = default;
   OK31& operator=(OK31 &&) noexcept(TrueT<int>::value) = default;
 };
+
+namespace gh68101
+{
+  template <typename T>
+  class Container {
+     public:
+      Container(Container&&) noexcept(std::is_nothrow_move_constructible<T>::value);
+  };
+} // namespace gh68101

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 392b5d17d456e71..dfc71a2bb9ab370 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
@@ -1,5 +1,14 @@
 // RUN: %check_clang_tidy %s performance-noexcept-swap %t -- -- -fexceptions
 
+namespace std
+{
+  template <typename T>
+  struct is_nothrow_move_constructible
+  {
+    static constexpr bool value = __is_nothrow_constructible(T, __add_rvalue_reference(T));
+  };
+} // namespace std
+
 void throwing_function() noexcept(false);
 void noexcept_function() noexcept;
 
@@ -54,9 +63,6 @@ struct D {
   // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: noexcept specifier on swap function evaluates to 'false' [performance-noexcept-swap]
 };
 
-template <typename T>
-void swap(D<T> &, D<T> &) noexcept(D<T>::kFalse);
-// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: noexcept specifier on swap function evaluates to 'false' [performance-noexcept-swap]
 void swap(D<int> &, D<int> &) noexcept(D<int>::kFalse);
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: noexcept specifier on swap function evaluates to 'false' [performance-noexcept-swap]
 
@@ -151,9 +157,8 @@ struct OK16 {
   void swap(OK16 &) noexcept(kTrue);
 };
 
-// FIXME: This gives a warning, but it should be OK.
-//template <typename T>
-//void swap(OK16<T> &, OK16<T> &) noexcept(OK16<T>::kTrue);
+template <typename T>
+void swap(OK16<T> &, OK16<T> &) noexcept(OK16<T>::kTrue);
 template <typename T>
 void swap(OK16<int> &, OK16<int> &) noexcept(OK16<int>::kTrue);
 
@@ -217,4 +222,13 @@ namespace PR64303 {
     friend void swap(Test&, Test&);
     // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: swap functions should be marked noexcept [performance-noexcept-swap]
   };
-}
+} // namespace PR64303
+
+namespace gh68101
+{
+  template <typename T>
+  class Container {
+     public:
+      void swap(Container&) noexcept(std::is_nothrow_move_constructible<T>::value);
+  };
+} // namespace gh68101