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

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst:6
@@ +5,3 @@
+
+MSVC 2015 will generate an assignment operator even if the user defines a copy constructor.
+This check finds classes with a user-defined (including deleted)
----------------
aaron.ballman wrote:
> alexfh wrote:
> > Is this problem only relevant to MSVC 2015? Is MSVC's behavior standard-compliant in this case?
> MSVC's behavior is standards compliant, but is deprecated behavior. [class.copy]p18:
> 
> "If the class definition does not explicitly declare a copy assignment operator, one is declared implicitly. If
> the class definition declares a move constructor or move assignment operator, the implicitly declared copy
> assignment operator is defined as deleted; otherwise, it is defined as defaulted (8.4). The latter case is
> deprecated if the class has a user-declared copy constructor or a user-declared destructor."
> 
> Now that I think about it more, I thought @dblaikie or @rsmith made a check for this once under -Wdeprecated. It looks like Richard did in r183884.
Ah, now I start understanding ;)

The check is actually (incompletely) verifying [[ https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) | the rule of 2 ]], and it's not specific to MSVC in any way (and the documentation shouldn't mention MSVC then). It makes sense to:
1. make the check symmetric (a user-defined assignment and an implicit copy-construction;
2. extend the check to verify move operations as well (the rule of 4);
3. and maybe extend it to verify the rule of 3 and the rule of 5 (though detecting non-RAII resource owning may be really hard if even possible).

================
Comment at: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst:23
@@ +22,3 @@
+
+The fix is defensive. Incorrect compiler-generated assignement can cause
+unexpected behaviour. An explicitly deleted assigneemnt operator will cause a
----------------
The fix will break code that uses the implicit assignment operator. I'm not sure it's a nice thing to do in a check enabled by default.


Repository:
  rL LLVM

http://reviews.llvm.org/D16376