diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp index 6a6e620a4387b..f615976c7edb6 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp @@ -13,79 +13,88 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { -namespace { -AST_MATCHER(FieldDecl, isMemberOfLambda) { - return Node.getParent()->isLambda(); +static bool isCopyConstructible(CXXRecordDecl const &Node) { + if (Node.needsOverloadResolutionForCopyConstructor() && + Node.needsImplicitCopyConstructor()) { + // unresolved + for (CXXBaseSpecifier const &BS : Node.bases()) { + CXXRecordDecl const *BRD = BS.getType()->getAsCXXRecordDecl(); + if (BRD != nullptr && !isCopyConstructible(*BRD)) + return false; + } + } + if (Node.hasSimpleCopyConstructor()) + return true; + for (CXXConstructorDecl const *Ctor : Node.ctors()) + if (Ctor->isCopyConstructor()) + return !Ctor->isDeleted(); + return false; } -struct MemberFunctionInfo { - bool Declared{}; - bool Deleted{}; -}; - -struct MemberFunctionPairInfo { - MemberFunctionInfo Copy{}; - MemberFunctionInfo Move{}; -}; - -MemberFunctionPairInfo getConstructorsInfo(CXXRecordDecl const &Node) { - MemberFunctionPairInfo Constructors{}; - - for (CXXConstructorDecl const *Ctor : Node.ctors()) { - if (Ctor->isCopyConstructor()) { - Constructors.Copy.Declared = true; - if (Ctor->isDeleted()) - Constructors.Copy.Deleted = true; - } - if (Ctor->isMoveConstructor()) { - Constructors.Move.Declared = true; - if (Ctor->isDeleted()) - Constructors.Move.Deleted = true; +static bool isMoveConstructible(CXXRecordDecl const &Node) { + if (Node.needsOverloadResolutionForMoveConstructor() && + Node.needsImplicitMoveConstructor()) { + // unresolved + for (CXXBaseSpecifier const &BS : Node.bases()) { + CXXRecordDecl const *BRD = BS.getType()->getAsCXXRecordDecl(); + if (BRD != nullptr && !isMoveConstructible(*BRD)) + return false; } } - - return Constructors; + if (Node.hasSimpleMoveConstructor()) + return true; + for (CXXConstructorDecl const *Ctor : Node.ctors()) + if (Ctor->isMoveConstructor()) + return !Ctor->isDeleted(); + return false; } -MemberFunctionPairInfo getAssignmentsInfo(CXXRecordDecl const &Node) { - MemberFunctionPairInfo Assignments{}; - - for (CXXMethodDecl const *Method : Node.methods()) { - if (Method->isCopyAssignmentOperator()) { - Assignments.Copy.Declared = true; - if (Method->isDeleted()) - Assignments.Copy.Deleted = true; +static bool isCopyAssignable(CXXRecordDecl const &Node) { + if (Node.needsOverloadResolutionForCopyAssignment() && + Node.needsImplicitCopyAssignment()) { + // unresolved + for (CXXBaseSpecifier const &BS : Node.bases()) { + CXXRecordDecl const *BRD = BS.getType()->getAsCXXRecordDecl(); + if (BRD != nullptr && !isCopyAssignable(*BRD)) + return false; } + } + if (Node.hasSimpleCopyAssignment()) + return true; + for (CXXMethodDecl const *Method : Node.methods()) + if (Method->isCopyAssignmentOperator()) + return !Method->isDeleted(); + return false; +} - if (Method->isMoveAssignmentOperator()) { - Assignments.Move.Declared = true; - if (Method->isDeleted()) - Assignments.Move.Deleted = true; +static bool isMoveAssignable(CXXRecordDecl const &Node) { + if (Node.needsOverloadResolutionForMoveAssignment() && + Node.needsImplicitMoveAssignment()) { + // unresolved + for (CXXBaseSpecifier const &BS : Node.bases()) { + CXXRecordDecl const *BRD = BS.getType()->getAsCXXRecordDecl(); + if (BRD != nullptr && !isMoveAssignable(*BRD)) + return false; } } - - return Assignments; + if (Node.hasSimpleMoveAssignment()) + return true; + for (CXXMethodDecl const *Method : Node.methods()) + if (Method->isMoveAssignmentOperator()) + return !Method->isDeleted(); + return false; } -AST_MATCHER(CXXRecordDecl, isCopyableOrMovable) { - MemberFunctionPairInfo Constructors = getConstructorsInfo(Node); - MemberFunctionPairInfo Assignments = getAssignmentsInfo(Node); +namespace { - if (Node.hasSimpleCopyConstructor() || - (Constructors.Copy.Declared && !Constructors.Copy.Deleted)) - return true; - if (Node.hasSimpleMoveConstructor() || - (Constructors.Move.Declared && !Constructors.Move.Deleted)) - return true; - if (Node.hasSimpleCopyAssignment() || - (Assignments.Copy.Declared && !Assignments.Copy.Deleted)) - return true; - if (Node.hasSimpleMoveAssignment() || - (Assignments.Move.Declared && !Assignments.Move.Deleted)) - return true; +AST_MATCHER(FieldDecl, isMemberOfLambda) { + return Node.getParent()->isLambda(); +} - return false; +AST_MATCHER(CXXRecordDecl, isCopyableOrMovable) { + return isCopyConstructible(Node) || isMoveConstructible(Node) || + isCopyAssignable(Node) || isMoveAssignable(Node); } } // namespace diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index db971f08ca3db..039ed658aada7 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -195,6 +195,10 @@ Changes in existing checks fix false positive that floating point variable is only used in increment expression. +- Improved :doc:`cppcoreguidelines-avoid-const-or-ref-data-members + ` check to + avoid false positives when detecting a templated class with inheritance. + - Improved :doc:`cppcoreguidelines-init-variables ` check by fixing the insertion location for function pointers. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp index e3864be134da3..19da88300aec4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp @@ -285,6 +285,28 @@ struct InheritBothFromNonCopyableAndNonMovable : NonCopyable, NonMovable int& x; // OK, non copyable nor movable }; +template struct TemplateInheritFromNonCopyable : NonCopyable +{ + int& x; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'x' of type 'int &' is a reference +}; + +template struct TemplateInheritFromNonMovable : NonMovable +{ + int& x; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'x' of type 'int &' is a reference +}; + +template struct TemplateInheritFromNonCopyableNonMovable : NonCopyableNonMovable +{ + int& x; // OK, non copyable nor movable +}; + +template struct TemplateInheritBothFromNonCopyableAndNonMovable : NonCopyable, NonMovable +{ + int& x; // OK, non copyable nor movable +}; + // Test composition struct ContainsNonCopyable {