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

[Aaron Ballman via cfe-commits]

On Wed, Feb 3, 2016 at 4:13 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Wed, Feb 3, 2016 at 1:03 PM, Jonathan Coe <jon at jbcoe.net> wrote:
>>
>>
>>
>> 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.
>
>
> Sure - and the user can do that with -Werror=deprecated (but, yes, we should
> split out that specific deprecation so it can be turned on without turning
> on other deprecation)
>
>>
>>
>>>
>>> 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.
>
>
> For sure - because it's only deprecated, not an error. Clang-tidy won't
> change that - it still won't be a hard error in every codebase.
>
> I agree that having all the Core Guidelines checks in one place is a good
> idea, so I'm not making any real suggestion here, sorry - just that it seems
> unfortunate to relegate this check (& encourage explicit & mostly redundant
> defaulting/deleting) to a separate tool when it's essentially built into the
> language and compiler already. My disagreement is perhaps more with the Core
> Guideline than with its implementation here.
>
>>
>> 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.
>
>
> But once the language does the right thing directly, rather than providing a
> clang-tidy warning that encourages you to change the code to achieve the
> same result, why would we worry about being explicit?

It could be argued that implicit code generated by the compiler is
magic, and being explicit is a readability improvement. You no longer
have to think "under what circumstances is this generated?"

~Aaron