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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thanks for the useful check! I have a few comments, see inline.



================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-35
+      binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+                     anyOf(hasLHS(ignoringParenCasts(cxxThisExpr())),
+                           hasRHS(ignoringParenCasts(cxxThisExpr())))))));
+
----------------
I guess, `has(ignoringParenCasts(cxxThisExpr())` will have the same effect as `anyOf(hasLHS(...), hasRHS(...))`.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:53
+      recordType(hasDeclaration(classTemplateSpecializationDecl(
+          anyOf(hasName("std::shared_ptr"), hasName("std::unique_ptr"),
+                hasName("std::weak_ptr")),
----------------
Please use hasAnyName. It's more efficient and readable.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10-12
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment>`_.
----------------
JonasToth wrote:
> It is worth an alias into the cert module. Please add this with this revision already.
Please add a corresponding alias into the cert module.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60507/new/

https://reviews.llvm.org/D60507