[PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:22
@@ +21,3 @@
+void UserDefinedCopyWithoutAssignmentCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(
----------------
Before registering the matchers, can you early return if not in CPlusPlus mode? (No need to register these matchers for C code.)

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:63
@@ +62,3 @@
+void UserDefinedCopyWithoutAssignmentCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl =
----------------
This seems to have a lot of duplicate code that could be consolidated.

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:21
@@ +20,3 @@
+///
+/// MSVC 2015 will generate an assignment operator even if the user defines a
+/// copy-constructor. This check finds classes with user-defined
----------------
Should update these comments to reflect the check's behavior better (and remove mention of MSVC since this isn't specific to that compiler).

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:28
@@ +27,3 @@
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-user-defined-copy-without-assignment.html
+class UserDefinedCopyWithoutAssignmentCheck : public ClangTidyCheck { public:
+  UserDefinedCopyWithoutAssignmentCheck(StringRef Name, ClangTidyContext
----------------
This no longer is about just copy, so you may want to rename the class (and files and check) to something different.

================
Comment at: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst:8
@@ +7,3 @@
+constructor.  This behaviour is deprecated by the standard (C++ 14 draft
+standard 12.8.18)
+
----------------
Best to use the tags ([class.copy] paragraph 18) when referring to the standard, since the numbers can change.


http://reviews.llvm.org/D16376