<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 3 February 2016 at 18:44, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Wed, Feb 3, 2016 at 10:23 AM, Jonathan Coe <span dir="ltr"><<a href="mailto:jon@jbcoe.net" target="_blank">jon@jbcoe.net</a>></span> wrote:<br><blockquote class="gmail_quote"><div dir="ltr">All the C++ compilers I have tried using (GCC,Clang,MSVC) will generate assignment operators even if the user defines a copy-constructor. This is the behaviour I set out to write a check for.<div><br></div><div>The cpp core guide lines recommend defining all or none of the special functions <a href="https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all" target="_blank">https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all</a></div><div><br></div><div>If the deprecation warning you mention will turn off the deprecated generation of special member functions when others are defined (or warn when deprecated compiler-generated functions are used) then the rule-of-five check is of reduced value.</div></div></blockquote><div><br></div></span><div>It can't stop them being generated, because that's required behavior - warnings don't change the program behavior.<br><br></div></div></div></div></blockquote><div><br></div><div>That's true but promoting them to errors will stop compilation and prevent the sort of bug I'm trying to stop.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Wording like this is in the C++11 standard:<br><br>"If the class definition does not explicitly declare a copy constructor, one is declared implicitly. If the class
definition declares a move constructor or move assignment operator, the implicitly declared copy constructor
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 assignment operator or a user-declared destructor."<br><br></div></div></div></div></blockquote><div><br></div><div>The 'deprecated' part is my target. I've seen code with user-defined copy constructors behave badly when compiler-generated assignment operators are unwittingly used. The rule of five lets me locally reason about code without having to worry (needlessly or not) about whether some functions are potentially being compiler-generated.</div><div><br></div><div>I don't advocate putting this in 'misc'. It belongs in 'cppcoreguidelines', I'll move it if the approach taken thus far is sound (as discussed in the review).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>(similar wording for the copy assignment operator, the dtor has a different/special case if I recall correctly)</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Jon</div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On 3 February 2016 at 17:40, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Is this really that useful of a rule? The language does the right thing for the most part already (you don't need to explicitly delete them - they're implicitly deleted if you define any others - except for backcompat with C++98, but those cases are deprecated & we should probably split out the warning for that deprecation so it's easier to turn on separately)</div><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Feb 3, 2016 at 5:25 AM, Jonathan B Coe via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>jbcoe retitled this revision from "clang-tidy check: Assignment and Construction" to "clang-tidy check: rule-of-five".<br>
jbcoe removed rL LLVM as the repository for this revision.<br>
jbcoe updated this revision to Diff 46775.<br>
jbcoe added a comment.<br>
<br>
I've responded to review comments (thanks for those) and renamed the check to 'rule-of-five'.<br>
<br>
I think it needs moving to cppcoreguidelines and needs destructor handling adding to it. As suggested, I'll address that in a later patch if everything else looks ok.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D16376" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16376</a><br>
<br>
Files:<br>
  clang-tidy/misc/CMakeLists.txt<br>
  clang-tidy/misc/MiscTidyModule.cpp<br>
  clang-tidy/misc/RuleOfFiveCheck.cpp<br>
  clang-tidy/misc/RuleOfFiveCheck.h<br>
  docs/clang-tidy/checks/list.rst<br>
  docs/clang-tidy/checks/misc-rule-of-five.rst<br>
  test/clang-tidy/misc-rule-of-five.cpp<br>
<br>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div>
</div></div></blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>