AMS21 updated this revision to Diff 509013.
AMS21 added a comment.
Implemented suggested fixes from reviews
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/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
@@ -4,7 +4,7 @@
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,95 @@
// 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]
+};
+
class OK {};
void f() {
@@ -52,3 +141,86 @@
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;
+};
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
@@ -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/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
@@ -16,6 +16,116 @@
namespace clang::tidy::performance {
+namespace {
+
+enum CanMoveConstrcutorThrowResult {
+ CMCT_CanThrow, ///< We are sure the move constructor can throw
+ CMCT_CannotThrow, ///< We are sure the move constrcutor cannot throw
+ CMCT_Unknown, ///< We don't know if the move constrcutor can throw or not
+};
+
+CanMoveConstrcutorThrowResult evaluateFunctionEST(const FunctionDecl *Func) {
+ const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
+ if (!FunProto)
+ return CMCT_Unknown;
+
+ switch (FunProto->canThrow()) {
+ case CT_Cannot:
+ return CMCT_CannotThrow;
+ case CT_Dependent: {
+ const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
+ bool Result;
+ return (NoexceptExpr && !NoexceptExpr->isValueDependent() &&
+ NoexceptExpr->EvaluateAsBooleanCondition(
+ Result, Func->getASTContext(), true) &&
+ Result)
+ ? CMCT_CannotThrow
+ : CMCT_CanThrow;
+ }
+ default:
+ return CMCT_CanThrow;
+ };
+}
+
+CanMoveConstrcutorThrowResult
+canMoveConstructorThrow(const FunctionDecl *FuncDecl);
+
+CanMoveConstrcutorThrowResult
+canFieldMoveConstructorThrow(const FieldDecl *FDecl, bool TestConstructor) {
+ if (!FDecl)
+ return CMCT_Unknown;
+
+ const ASTContext &Context = FDecl->getASTContext();
+
+ if (auto *FieldRecordType =
+ Context.getBaseElementType(FDecl->getType())->getAs<RecordType>()) {
+ if (const auto *FieldRecordDecl =
+ cast<CXXRecordDecl>(FieldRecordType->getDecl())) {
+
+ // Trivial implies noexcept
+ if ((FieldRecordDecl->hasTrivialMoveConstructor() && TestConstructor) ||
+ (FieldRecordDecl->hasTrivialMoveAssignment() && !TestConstructor))
+ return CMCT_CannotThrow;
+
+ for (const auto *Method : FieldRecordDecl->methods()) {
+ const auto *Ctor = dyn_cast<CXXConstructorDecl>(Method);
+
+ if ((TestConstructor && Ctor && Ctor->isMoveConstructor()) ||
+ (Method->isMoveAssignmentOperator() && !TestConstructor))
+ return canMoveConstructorThrow(Method);
+ }
+ }
+ }
+
+ // We assume non record types cannot throw
+ return CMCT_CannotThrow;
+}
+
+CanMoveConstrcutorThrowResult
+resolveUnevaluatedOrDefaulted(const FunctionDecl *FuncDecl) {
+ const auto *FunProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+ if (!FunProto)
+ return CMCT_Unknown;
+
+ const CXXMethodDecl *MethodDecl = cast<CXXMethodDecl>(FuncDecl);
+ if (!MethodDecl || MethodDecl->isInvalidDecl())
+ return CMCT_Unknown;
+
+ const CXXRecordDecl *RecordDecl = MethodDecl->getParent();
+ if (!RecordDecl || RecordDecl->isInvalidDecl())
+ return CMCT_Unknown;
+
+ const ASTContext &Context = FuncDecl->getASTContext();
+ if (Context.getRecordType(RecordDecl).isNull())
+ return CMCT_Unknown;
+
+ const bool IsConstructor = !MethodDecl->isMoveAssignmentOperator();
+
+ // FIXME: We currectly ignore virtual and non virtual bases
+
+ for (const auto *FDecl : RecordDecl->fields())
+ if (!FDecl->isInvalidDecl() && !FDecl->isUnnamedBitfield())
+ if (canFieldMoveConstructorThrow(FDecl, IsConstructor) == CMCT_CanThrow)
+ return CMCT_CanThrow;
+
+ return CMCT_CannotThrow;
+}
+
+CanMoveConstrcutorThrowResult
+canMoveConstructorThrow(const FunctionDecl *FuncDecl) {
+ const auto *FunProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+ if (!FunProto)
+ return CMCT_Unknown;
+
+ const ExceptionSpecificationType EST = FunProto->getExceptionSpecType();
+
+ if (EST == EST_Unevaluated || (EST == EST_None && FuncDecl->isDefaulted()))
+ return resolveUnevaluatedOrDefaulted(FuncDecl);
+
+ return evaluateFunctionEST(FuncDecl);
+}
+} // namespace
+
void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
cxxMethodDecl(anyOf(cxxConstructorDecl(), hasOverloadedOperatorName("=")),
@@ -36,42 +146,41 @@
return;
}
- const auto *ProtoType = Decl->getType()->castAs<FunctionProtoType>();
+ const CanMoveConstrcutorThrowResult result = canMoveConstructorThrow(Decl);
- if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType()))
+ if (result != CMCT_CanThrow)
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 ");
- 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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
