[clang-tidy] new check: bugprone-unhandled-self-assignment

Summary:
This check searches for copy assignment operators which might not handle self-assignment properly. There are three patterns of
handling a self assignment situation: self check, copy-and-swap or the less common copy-and-move. The new check warns if none of
these patterns is found in a user defined implementation.

See also:
OOP54-CPP. Gracefully handle self-copy assignment
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment

Reviewers: JonasToth, alexfh, hokein, aaron.ballman

Subscribers: riccibruno, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D60507

llvm-svn: 360540

1 files changed, 99 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
new file mode 100644
index 00000000000..b529f72ddae
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -0,0 +1,99 @@
+//===--- UnhandledSelfAssignmentCheck.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 "UnhandledSelfAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  // We don't care about deleted, default or implicit operator implementations.
+  const auto IsUserDefined = cxxMethodDecl(
+      isDefinition(), unless(anyOf(isDeleted(), isImplicit(), isDefaulted())));
+
+  // We don't need to worry when a copy assignment operator gets the other
+  // object by value.
+  const auto HasReferenceParam =
+      cxxMethodDecl(hasParameter(0, parmVarDecl(hasType(referenceType()))));
+
+  // Self-check: Code compares something with 'this' pointer. We don't check
+  // whether it is actually the parameter what we compare.
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+      binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+                     has(ignoringParenCasts(cxxThisExpr()))))));
+
+  // Both copy-and-swap and copy-and-move method creates a copy first and
+  // assign it to 'this' with swap or move.
+  // In the non-template case, we can search for the copy constructor call.
+  const auto HasNonTemplateSelfCopy = cxxMethodDecl(
+      ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl())))),
+      hasDescendant(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+          isCopyConstructor(), ofClass(equalsBoundNode("class")))))));
+
+  // In the template case, we need to handle two separate cases: 1) a local
+  // variable is created with the copy, 2) copy is created only as a temporary
+  // object.
+  const auto HasTemplateSelfCopy = cxxMethodDecl(
+      ofClass(cxxRecordDecl(hasAncestor(classTemplateDecl()))),
+      anyOf(hasDescendant(
+                varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"))),
+                        hasDescendant(parenListExpr()))),
+            hasDescendant(cxxUnresolvedConstructExpr(hasDescendant(declRefExpr(
+                hasType(cxxRecordDecl(equalsBoundNode("class")))))))));
+
+  // If inside the copy assignment operator another assignment operator is
+  // called on 'this' we assume that self-check might be handled inside
+  // this nested operator.
+  const auto HasNoNestedSelfAssign =
+      cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
+          hasName("operator="), ofClass(equalsBoundNode("class"))))))));
+
+  // Matcher for standard smart pointers.
+  const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType(
+      recordType(hasDeclaration(classTemplateSpecializationDecl(
+          hasAnyName("::std::shared_ptr", "::std::unique_ptr",
+                     "::std::weak_ptr", "::std::auto_ptr"),
+          templateArgumentCountIs(1))))));
+
+  // We will warn only if the class has a pointer or a C array field which
+  // probably causes a problem during self-assignment (e.g. first resetting the
+  // pointer member, then trying to access the object pointed by the pointer, or
+  // memcpy overlapping arrays).
+  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
+      has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+                          hasType(arrayType())))))));
+
+  Finder->addMatcher(
+      cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")),
+                    isCopyAssignmentOperator(), IsUserDefined,
+                    HasReferenceParam, HasNoSelfCheck,
+                    unless(HasNonTemplateSelfCopy), unless(HasTemplateSelfCopy),
+                    HasNoNestedSelfAssign, ThisHasSuspiciousField)
+          .bind("copyAssignmentOperator"),
+      this);
+}
+
+void UnhandledSelfAssignmentCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+      Result.Nodes.getNodeAs<CXXMethodDecl>("copyAssignmentOperator");
+  diag(MatchedDecl->getLocation(),
+       "operator=() does not handle self-assignment properly");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang