AMS21 updated this revision to Diff 509409.
AMS21 marked 8 inline comments as done.
AMS21 added a comment.
Sprinkle some more `const`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146922/new/
https://reviews.llvm.org/D146922
Files:
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
clang-tools-extra/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
@@ -1,10 +1,10 @@
-// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t -- -- -fexceptions
class A {
A(A &&);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
A &operator=(A &&);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
};
struct B {
@@ -13,6 +13,121 @@
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
};
+template <typename>
+struct C
+{
+ C(C &&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+ C& operator=(C &&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct D
+{
+ static constexpr bool kFalse = false;
+ D(D &&) noexcept(kFalse) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+ D& operator=(D &&) noexcept(kFalse) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename>
+struct E
+{
+ static constexpr bool kFalse = false;
+ E(E &&) noexcept(kFalse);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+ E& operator=(E &&) noexcept(kFalse);
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename>
+struct F
+{
+ static constexpr bool kFalse = false;
+ F(F &&) noexcept(kFalse) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+ F& operator=(F &&) noexcept(kFalse) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+struct Field
+{
+ Field() = default;
+ Field(Field&&) noexcept(false) {
+ }
+ Field& operator=(Field &&) noexcept(false) {
+ return *this;
+ }
+};
+
+struct G {
+ G(G &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+ G& operator=(G &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+
+ Field field;
+};
+
+void throwing_function() noexcept(false) {}
+
+struct H {
+ H(H &&) noexcept(noexcept(throwing_function()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+ H &operator=(H &&) noexcept(noexcept(throwing_function()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename>
+struct I {
+ I(I &&) noexcept(noexcept(throwing_function()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+ I &operator=(I &&) noexcept(noexcept(throwing_function()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename TemplateType> struct TemplatedType {
+ static void f() {}
+};
+
+template <> struct TemplatedType<int> {
+ static void f() noexcept {}
+};
+
+struct J {
+ J(J &&) noexcept(noexcept(TemplatedType<double>::f()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+ J &operator=(J &&) noexcept(noexcept(TemplatedType<double>::f()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false' [performance-noexcept-move-constructor]
+};
+
+struct K : public Field {
+ K(K &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+ K &operator=(K &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct InheritFromField : public Field
+{};
+
+struct L {
+ L(L &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+ L &operator=(L &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+
+ InheritFromField IFF;
+};
+
+struct M : public InheritFromField {
+ M(M &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+ M &operator=(M &&) = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
class OK {};
void f() {
@@ -52,3 +167,112 @@
OK5(OK5 &&) noexcept(true) = default;
OK5 &operator=(OK5 &&) noexcept(true) = default;
};
+
+struct OK6 {
+ OK6(OK6 &&) = default;
+ OK6& operator=(OK6 &&) = default;
+};
+
+template <typename>
+struct OK7 {
+ OK7(OK7 &&) = default;
+ OK7& operator=(OK7 &&) = default;
+};
+
+template <typename>
+struct OK8 {
+ OK8(OK8 &&) noexcept = default;
+ OK8& operator=(OK8 &&) noexcept = default;
+};
+
+template <typename>
+struct OK9 {
+ OK9(OK9 &&) noexcept(true) = default;
+ OK9& operator=(OK9 &&) noexcept(true) = default;
+};
+
+template <typename>
+struct OK10 {
+ OK10(OK10 &&) noexcept(false) = default;
+ OK10& operator=(OK10 &&) noexcept(false) = default;
+};
+
+template <typename>
+struct OK11 {
+ OK11(OK11 &&) = delete;
+ OK11& operator=(OK11 &&) = delete;
+};
+
+void nothrowing_function() noexcept {}
+
+struct OK12 {
+ OK12(OK12 &&) noexcept(noexcept(nothrowing_function()));
+ OK12 &operator=(OK12 &&) noexcept(noexcept(nothrowing_function));
+};
+
+struct OK13 {
+ OK13(OK13 &&) noexcept(noexcept(nothrowing_function)) = default;
+ OK13 &operator=(OK13 &&) noexcept(noexcept(nothrowing_function)) = default;
+};
+
+template <typename> struct OK14 {
+ OK14(OK14 &&) noexcept(noexcept(TemplatedType<int>::f()));
+ OK14 &operator=(OK14 &&) noexcept(noexcept(TemplatedType<int>::f()));
+};
+
+struct OK15 {
+ OK15(OK15 &&) = default;
+ OK15 &operator=(OK15 &&) = default;
+
+ int member;
+};
+
+template <typename>
+struct OK16 {
+ OK16(OK16 &&) = default;
+ OK16 &operator=(OK16 &&) = default;
+
+ int member;
+};
+
+struct OK17
+{
+ OK17(OK17 &&) = default;
+ OK17 &operator=(OK17 &&) = default;
+ OK empty_field;
+};
+
+template <typename>
+struct OK18
+{
+ OK18(OK18 &&) = default;
+ OK18 &operator=(OK18 &&) = default;
+
+ OK empty_field;
+};
+
+struct OK19 : public OK
+{
+ OK19(OK19 &&) = default;
+ OK19 &operator=(OK19 &&) = default;
+};
+
+struct OK20 : virtual OK
+{
+ OK20(OK20 &&) = default;
+ OK20 &operator=(OK20 &&) = default;
+};
+
+template <typename T>
+struct OK21 : public T
+{
+ OK21(OK21 &&) = default;
+ OK21 &operator=(OK21 &&) = default;
+};
+
+template <typename T>
+struct OK22 : virtual T
+{
+ OK22(OK22 &&) = default;
+ OK22 &operator=(OK22 &&) = default;
+};
Index: clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t -- -- -fexceptions
struct C_1 {
~C_1() {}
@@ -31,9 +31,7 @@
};
C_3::C_3(C_3&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
C_3& C_3::operator=(C_3&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
template <class T>
struct C_4 {
@@ -41,7 +39,6 @@
// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
~C_4() {}
C_4& operator=(C_4&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
};
template <class T>
@@ -50,7 +47,6 @@
// CHECK-FIXES:){{.*}}noexcept{{.*}} {}
~C_5() {}
auto operator=(C_5&& a)->C_5<T> = default;
-// CHECK-FIXES:){{.*}}noexcept{{.*}} = default;
};
template <class T>
@@ -64,4 +60,4 @@
template <class T>
auto C_6<T>::operator=(C_6<T>&& a) -> C_6<T> {}
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
\ No newline at end of file
+// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -257,6 +257,10 @@
<clang-tidy/checks/google/readability-avoid-underscore-in-googletest-name>` when using
``DISABLED_`` in the test suite name.
+- Fixed an issue in :doc:`performance-noexcept-move-constructor
+ <clang-tidy/checks/performance/noexcept-move-constructor>` which resulted in false
+ positives if the constructor was defaulted.
+
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h
@@ -0,0 +1,84 @@
+//===--- ExceptionSpecAnalyzer.h - clang-tidy -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_SPEC_ANALYZER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_SPEC_ANALYZER_H
+
+#include "clang/AST/DeclCXX.h"
+
+namespace clang::tidy::utils {
+
+/// This class analysis if a `FunctionDecl` has been declared implicitly through
+/// defaulting or explicitly as throwing or not and evaluates noexcept
+/// expressions if needed. Unlike the `ExceptionAnalyzer` however it can't tell
+/// you if the function will actually throw an exception or not.
+class ExceptionSpecAnalyzer {
+public:
+ enum class State : std::int8_t {
+ Throwing, ///< This function has been declared as possibly throwing.
+ NotThrowing, ///< This function has been declared as not throwing.
+ Unknown, ///< We're unable to tell if this function is declared as throwing
+ ///< or not.
+ };
+
+ ExceptionSpecAnalyzer() = default;
+
+ State analyze(const FunctionDecl *FuncDecl);
+
+private:
+ enum class DefaultableMemberKind : std::int8_t {
+ DefaultConstructor,
+ CopyConstructor,
+ MoveConstructor,
+ CopyAssignment,
+ MoveAssignment,
+ Destructor,
+
+ CompareEqual,
+ CompareNotEqual,
+ CompareThreeWay,
+ CompareRelational,
+
+ None,
+ };
+
+ State analyzeImpl(const FunctionDecl *FuncDecl);
+
+ State analyzeUnresolvedOrDefaulted(const FunctionDecl *FuncDecl);
+
+ State analyzeFieldDecl(const FieldDecl *FDecl, DefaultableMemberKind Kind);
+
+ State analyzeBase(const CXXBaseSpecifier &Base, DefaultableMemberKind Kind);
+
+ enum class SkipMethods : bool {
+ Yes = true,
+ No = false,
+ };
+
+ State analyzeRecord(const CXXRecordDecl *RecDecl, DefaultableMemberKind Kind,
+ SkipMethods SkipMethods = SkipMethods::No);
+
+ static State analyzeFunctionEST(const FunctionDecl *FuncDecl);
+
+ static bool hasTrivialMemberKind(const CXXRecordDecl *RecDecl,
+ DefaultableMemberKind Kind);
+
+ static bool isConstructor(DefaultableMemberKind Kind);
+
+ static bool isSpecialMember(DefaultableMemberKind Kind);
+
+ static bool isComparison(DefaultableMemberKind Kind);
+
+ DefaultableMemberKind getDefaultableMemberKind(const FunctionDecl *FuncDecl);
+
+ std::map<const FunctionDecl *, State> FunctionCache;
+};
+
+} // namespace clang::tidy::utils
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_SPEC_ANALYZER_H
Index: clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
@@ -0,0 +1,299 @@
+//===--- ExceptionSpecAnalyzer.cpp - clang-tidy ---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ExceptionSpecAnalyzer.h"
+
+#include "clang/AST/Expr.h"
+
+namespace clang::tidy::utils {
+
+enum class BasesToVisit : std::int8_t {
+ /// Visit all non-virtual (direct) bases.
+ VisitNonVirtualBases,
+ /// Visit all direct bases, virtual or not.
+ VisitDirectBases,
+ /// Visit all non-virtual bases, and all virtual bases if the class
+ /// is not abstract.
+ VisitPotentiallyConstructedBases,
+ /// Visit all direct or virtual bases.
+ VisitAllBases,
+};
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyze(const FunctionDecl *FuncDecl) {
+ ExceptionSpecAnalyzer::State State;
+
+ // Check if the function has already been analyzed and reuse that result.
+ if (FunctionCache.count(FuncDecl) == 0) {
+ State = analyzeImpl(FuncDecl);
+
+ // Cache the result of the analysis.
+ FunctionCache.insert(std::make_pair(FuncDecl, State));
+ } else
+ State = FunctionCache[FuncDecl];
+
+ return State;
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeUnresolvedOrDefaulted(
+ const FunctionDecl *FuncDecl) {
+ const auto *FunProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+ if (!FunProto)
+ return State::Unknown;
+
+ const auto *MethodDecl = cast<CXXMethodDecl>(FuncDecl);
+ if (!MethodDecl)
+ return State::Unknown;
+
+ const CXXRecordDecl *RecordDecl = MethodDecl->getParent();
+ if (!RecordDecl)
+ return State::Unknown;
+
+ const DefaultableMemberKind Kind = getDefaultableMemberKind(FuncDecl);
+
+ if (Kind == DefaultableMemberKind::None)
+ return State::Unknown;
+
+ return analyzeRecord(RecordDecl, Kind, SkipMethods::Yes);
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeFieldDecl(const FieldDecl *FDecl,
+ DefaultableMemberKind Kind) {
+ if (!FDecl)
+ return State::Unknown;
+
+ if (const CXXRecordDecl *RecDecl =
+ FDecl->getType()->getUnqualifiedDesugaredType()->getAsCXXRecordDecl())
+ return analyzeRecord(RecDecl, Kind);
+
+ // FIXME: There may still be a way to figure out if the field can throw or not
+ return State::Unknown;
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeBase(const CXXBaseSpecifier &Base,
+ DefaultableMemberKind Kind) {
+ const auto *RecType = Base.getType()->getAs<RecordType>();
+ if (!RecType)
+ return State::Unknown;
+
+ const auto *BaseClass = cast<CXXRecordDecl>(RecType->getDecl());
+
+ return analyzeRecord(BaseClass, Kind);
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeRecord(const CXXRecordDecl *RecordDecl,
+ DefaultableMemberKind Kind,
+ SkipMethods SkipMethods) {
+ if (!RecordDecl)
+ return State::Unknown;
+
+ // Trivial implies noexcept
+ if (hasTrivialMemberKind(RecordDecl, Kind))
+ return State::NotThrowing;
+
+ if (SkipMethods == SkipMethods::No)
+ for (const auto *MethodDecl : RecordDecl->methods())
+ if (getDefaultableMemberKind(MethodDecl) == Kind)
+ return analyze(MethodDecl);
+
+ BasesToVisit Bases = isConstructor(Kind)
+ ? BasesToVisit::VisitPotentiallyConstructedBases
+ : BasesToVisit::VisitAllBases;
+
+ if (Bases == BasesToVisit::VisitPotentiallyConstructedBases)
+ Bases = RecordDecl->isAbstract() ? BasesToVisit::VisitNonVirtualBases
+ : BasesToVisit::VisitAllBases;
+
+ for (const auto &BaseSpec : RecordDecl->bases())
+ if (Bases == BasesToVisit::VisitDirectBases || !BaseSpec.isVirtual()) {
+ State Result = analyzeBase(BaseSpec, Kind);
+ if (Result == State::Throwing || Result == State::Unknown)
+ return Result;
+ }
+
+ if (Bases == BasesToVisit::VisitAllBases)
+ for (const auto &BaseSpec : RecordDecl->vbases()) {
+ State Result = analyzeBase(BaseSpec, Kind);
+ if (Result == State::Throwing || Result == State::Unknown)
+ return Result;
+ }
+
+ for (const auto *FDecl : RecordDecl->fields())
+ if (!FDecl->isInvalidDecl() && !FDecl->isUnnamedBitfield()) {
+ State Result = analyzeFieldDecl(FDecl, Kind);
+ if (Result == State::Throwing || Result == State::Unknown)
+ return Result;
+ }
+
+ return State::NotThrowing;
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeImpl(const FunctionDecl *FuncDecl) {
+ const auto *FuncProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+ if (!FuncProto)
+ return State::Unknown;
+
+ const ExceptionSpecificationType EST = FuncProto->getExceptionSpecType();
+
+ if (EST == EST_Unevaluated || (EST == EST_None && FuncDecl->isDefaulted()))
+ return analyzeUnresolvedOrDefaulted(FuncDecl);
+
+ return analyzeFunctionEST(FuncDecl);
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeFunctionEST(const FunctionDecl *FuncDecl) {
+ const auto *FuncProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+ if (!FuncProto)
+ return State::Unknown;
+
+ if (isUnresolvedExceptionSpec(FuncProto->getExceptionSpecType()))
+ return State::Unknown;
+
+ switch (FuncProto->canThrow()) {
+ case CT_Cannot:
+ return State::NotThrowing;
+ case CT_Dependent: {
+ const Expr *NoexceptExpr = FuncProto->getNoexceptExpr();
+ bool Result;
+ return (NoexceptExpr && !NoexceptExpr->isValueDependent() &&
+ NoexceptExpr->EvaluateAsBooleanCondition(
+ Result, FuncDecl->getASTContext(), true) &&
+ Result)
+ ? State::NotThrowing
+ : State::Throwing;
+ }
+ default:
+ return State::Throwing;
+ };
+}
+
+bool ExceptionSpecAnalyzer::hasTrivialMemberKind(const CXXRecordDecl *RecDecl,
+ DefaultableMemberKind Kind) {
+ if (!RecDecl)
+ return false;
+
+ switch (Kind) {
+ case DefaultableMemberKind::DefaultConstructor:
+ return RecDecl->hasTrivialDefaultConstructor();
+ case DefaultableMemberKind::CopyConstructor:
+ return RecDecl->hasTrivialCopyConstructor();
+ case DefaultableMemberKind::MoveConstructor:
+ return RecDecl->hasTrivialMoveConstructor();
+ case DefaultableMemberKind::CopyAssignment:
+ return RecDecl->hasTrivialCopyAssignment();
+ case DefaultableMemberKind::MoveAssignment:
+ return RecDecl->hasTrivialMoveAssignment();
+ case DefaultableMemberKind::Destructor:
+ return RecDecl->hasTrivialDestructor();
+
+ default:
+ return false;
+ }
+}
+
+bool ExceptionSpecAnalyzer::isConstructor(DefaultableMemberKind Kind) {
+ switch (Kind) {
+ case DefaultableMemberKind::DefaultConstructor:
+ case DefaultableMemberKind::CopyConstructor:
+ case DefaultableMemberKind::MoveConstructor:
+ return true;
+
+ default:
+ return false;
+ }
+}
+
+bool ExceptionSpecAnalyzer::isSpecialMember(DefaultableMemberKind Kind) {
+ switch (Kind) {
+ case DefaultableMemberKind::DefaultConstructor:
+ case DefaultableMemberKind::CopyConstructor:
+ case DefaultableMemberKind::MoveConstructor:
+ case DefaultableMemberKind::CopyAssignment:
+ case DefaultableMemberKind::MoveAssignment:
+ case DefaultableMemberKind::Destructor:
+ return true;
+ default:
+ return false;
+ }
+}
+
+bool ExceptionSpecAnalyzer::isComparison(DefaultableMemberKind Kind) {
+ switch (Kind) {
+ case DefaultableMemberKind::CompareEqual:
+ case DefaultableMemberKind::CompareNotEqual:
+ case DefaultableMemberKind::CompareRelational:
+ case DefaultableMemberKind::CompareThreeWay:
+ return true;
+ default:
+ return false;
+ }
+}
+
+ExceptionSpecAnalyzer::DefaultableMemberKind
+ExceptionSpecAnalyzer::getDefaultableMemberKind(const FunctionDecl *FuncDecl) {
+ if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl)) {
+ if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
+ if (Ctor->isDefaultConstructor())
+ return DefaultableMemberKind::DefaultConstructor;
+
+ if (Ctor->isCopyConstructor())
+ return DefaultableMemberKind::CopyConstructor;
+
+ if (Ctor->isMoveConstructor())
+ return DefaultableMemberKind::MoveConstructor;
+ }
+
+ if (MethodDecl->isCopyAssignmentOperator())
+ return DefaultableMemberKind::CopyAssignment;
+
+ if (MethodDecl->isMoveAssignmentOperator())
+ return DefaultableMemberKind::MoveAssignment;
+
+ if (isa<CXXDestructorDecl>(FuncDecl))
+ return DefaultableMemberKind::Destructor;
+ }
+
+ const LangOptions &LangOpts = FuncDecl->getLangOpts();
+
+ switch (FuncDecl->getDeclName().getCXXOverloadedOperator()) {
+ case OO_EqualEqual:
+ return DefaultableMemberKind::CompareEqual;
+
+ case OO_ExclaimEqual:
+ return DefaultableMemberKind::CompareNotEqual;
+
+ case OO_Spaceship:
+ // No point allowing this if <=> doesn't exist in the current language mode.
+ if (!LangOpts.CPlusPlus20)
+ break;
+ return DefaultableMemberKind::CompareThreeWay;
+
+ case OO_Less:
+ case OO_LessEqual:
+ case OO_Greater:
+ case OO_GreaterEqual:
+ // No point allowing this if <=> doesn't exist in the current language mode.
+ if (!LangOpts.CPlusPlus20)
+ break;
+ return DefaultableMemberKind::CompareRelational;
+
+ default:
+ break;
+ }
+
+ // Not a defaultable member kind
+ return DefaultableMemberKind::None;
+}
+
+} // namespace clang::tidy::utils
Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/utils/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt
@@ -8,6 +8,7 @@
ASTUtils.cpp
DeclRefExprUtils.cpp
ExceptionAnalyzer.cpp
+ ExceptionSpecAnalyzer.cpp
ExprSequence.cpp
FileExtensionsUtils.cpp
FixItHintUtils.cpp
Index: clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
+++ clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H
#include "../ClangTidyCheck.h"
+#include "../utils/ExceptionSpecAnalyzer.h"
namespace clang::tidy::performance {
@@ -25,10 +26,13 @@
NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
- return LangOpts.CPlusPlus11;
+ return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ utils::ExceptionSpecAnalyzer SpecAnalyzer;
};
} // namespace clang::tidy::performance
Index: clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
@@ -18,7 +18,8 @@
void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- cxxMethodDecl(anyOf(cxxConstructorDecl(), hasOverloadedOperatorName("=")),
+ cxxMethodDecl(anyOf(cxxConstructorDecl(isMoveConstructor()),
+ hasOverloadedOperatorName("=")),
unless(isImplicit()), unless(isDeleted()))
.bind("decl"),
this);
@@ -29,49 +30,46 @@
if (const auto *Decl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl")) {
bool IsConstructor = false;
if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Decl)) {
- if (!Ctor->isMoveConstructor())
- return;
IsConstructor = true;
} else if (!Decl->isMoveAssignmentOperator()) {
return;
}
- const auto *ProtoType = Decl->getType()->castAs<FunctionProtoType>();
-
- if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType()))
- return;
-
- if (!isNoexceptExceptionSpec(ProtoType->getExceptionSpecType())) {
- auto Diag = diag(Decl->getLocation(),
- "move %select{assignment operator|constructor}0s should "
- "be marked noexcept")
- << IsConstructor;
- // Add FixIt hints.
- SourceManager &SM = *Result.SourceManager;
- assert(Decl->getNumParams() > 0);
- SourceLocation NoexceptLoc = Decl->getParamDecl(Decl->getNumParams() - 1)
- ->getSourceRange()
- .getEnd();
- if (NoexceptLoc.isValid())
- NoexceptLoc = Lexer::findLocationAfterToken(
- NoexceptLoc, tok::r_paren, SM, Result.Context->getLangOpts(), true);
- if (NoexceptLoc.isValid())
- Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+ if (SpecAnalyzer.analyze(Decl) !=
+ utils::ExceptionSpecAnalyzer::State::Throwing) {
return;
}
// Don't complain about nothrow(false), but complain on nothrow(expr)
// where expr evaluates to false.
- if (ProtoType->canThrow() == CT_Can) {
- Expr *E = ProtoType->getNoexceptExpr();
- E = E->IgnoreImplicit();
- if (!isa<CXXBoolLiteralExpr>(E)) {
- diag(E->getExprLoc(),
+ const auto *ProtoType = Decl->getType()->castAs<FunctionProtoType>();
+ const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
+ if (NoexceptExpr) {
+ NoexceptExpr = NoexceptExpr->IgnoreImplicit();
+ if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
+ diag(NoexceptExpr->getExprLoc(),
"noexcept specifier on the move %select{assignment "
"operator|constructor}0 evaluates to 'false'")
<< IsConstructor;
}
+ return;
}
+
+ auto Diag = diag(Decl->getLocation(),
+ "move %select{assignment operator|constructor}0s should "
+ "be marked noexcept")
+ << IsConstructor;
+ // Add FixIt hints.
+ SourceManager &SM = *Result.SourceManager;
+ assert(Decl->getNumParams() > 0);
+ SourceLocation NoexceptLoc =
+ Decl->getParamDecl(Decl->getNumParams() - 1)->getSourceRange().getEnd();
+ if (NoexceptLoc.isValid())
+ NoexceptLoc = Lexer::findLocationAfterToken(
+ NoexceptLoc, tok::r_paren, SM, Result.Context->getLangOpts(), true);
+ if (NoexceptLoc.isValid())
+ Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+ return;
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
