Skip to content

Commit

Permalink
[clang-tidy] Improve ExceptionSpecAnalyzers handling of conditional…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
AMS21 authored Oct 9, 2023
1 parent 1c3fdb3 commit f9bd62f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 14 deletions.
21 changes: 15 additions & 6 deletions clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
{};

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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

0 comments on commit f9bd62f

Please sign in to comment.