[PATCH] D16376: clang-tidy check: rule-of-five

[Jonathan Coe via cfe-commits]

On 3 February 2016 at 18:44, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Wed, Feb 3, 2016 at 10:23 AM, Jonathan Coe <jon at jbcoe.net> wrote:
>
>> 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.
>>
>> The cpp core guide lines recommend defining all or none of the special
>> functions
>> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all
>>
>> 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.
>>
>
> It can't stop them being generated, because that's required behavior -
> warnings don't change the program behavior.
>
>
That's true but promoting them to errors will stop compilation and prevent
the sort of bug I'm trying to stop.


> Wording like this is in the C++11 standard:
>
> "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."
>
>
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.

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).


> (similar wording for the copy assignment operator, the dtor has a
> different/special case if I recall correctly)
>
>
>>
>> Jon
>>
>> On 3 February 2016 at 17:40, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> 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)
>>>
>>> On Wed, Feb 3, 2016 at 5:25 AM, Jonathan B Coe via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>>
>>>> jbcoe retitled this revision from "clang-tidy check: Assignment and
>>>> Construction" to "clang-tidy check: rule-of-five".
>>>> jbcoe removed rL LLVM as the repository for this revision.
>>>> jbcoe updated this revision to Diff 46775.
>>>> jbcoe added a comment.
>>>>
>>>> I've responded to review comments (thanks for those) and renamed the
>>>> check to 'rule-of-five'.
>>>>
>>>> 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.
>>>>
>>>>
>>>> http://reviews.llvm.org/D16376
>>>>
>>>> Files:
>>>>   clang-tidy/misc/CMakeLists.txt
>>>>   clang-tidy/misc/MiscTidyModule.cpp
>>>>   clang-tidy/misc/RuleOfFiveCheck.cpp
>>>>   clang-tidy/misc/RuleOfFiveCheck.h
>>>>   docs/clang-tidy/checks/list.rst
>>>>   docs/clang-tidy/checks/misc-rule-of-five.rst
>>>>   test/clang-tidy/misc-rule-of-five.cpp
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160203/cbac1c14/attachment.html>