aaron.ballman added inline comments.

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:37
@@ +36,3 @@
+
+  DiagnosticBuilder Diag = diag(MatchedDecl->getLocation(), "class '%0' 
defines a copy-constructor but not an assignment operator")
+      << ClassName;
----------------
You should run clang-format over the patch to address 80-col limit issues like 
this.

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:49
@@ +48,3 @@
+  llvm::raw_string_ostream Insertion(S);
+  Insertion << "\n" << ClassName << "& operator = (const " << ClassName << "&) 
= delete;";
+
----------------
I think we usually prefer `operator=` to `operator =`, but am not certain.

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:51
@@ +50,3 @@
+
+  Diag << FixItHint::CreateInsertion(CCtorEnd, Insertion.str());
+}
----------------
We probably do not want to generate this fixit unless we are compiling for at 
least C++11. We should have a test for C++98.

================
Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:24
@@ +23,3 @@
+/// copy-constructors but no assignement operator and (defensively) defines the
+/// assignment operator to be `= delete`.
+///
----------------
Would it make sense to have an explicitly defaulted copy constructor generate a 
fixit for an explicitly defaulted copy assignment operator instead of a deleted 
one? It seems like the user is being explicit about wanting default copy 
operations more often in that case, and the defaulted copy constructor would 
not be actively harmful to that.

================
Comment at: 
docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst:24
@@ +23,3 @@
+The fix is defensive. Incorrect compiler-generated assignement can cause
+unexpected behaviour. An explicitly deleted assigneemnt operator will cause a
+compiler error if it is used.
----------------
s/assigneemnt/assignment


Repository:
  rL LLVM

http://reviews.llvm.org/D16376



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to