Bug 443343 - clazy-rule-of-two-soft ignores "= default"
Summary: clazy-rule-of-two-soft ignores "= default"
Status: RESOLVED FIXED
Alias: None
Product: clazy
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Sergio Martins
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-05 09:37 UTC by David Faure
Modified: 2021-10-26 19:47 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
sol-static-members.cpp (2.26 KB, text/x-c++src)
2021-10-05 09:37 UTC, David Faure
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Faure 2021-10-05 09:37:31 UTC
Created attachment 142164 [details]
sol-static-members.cpp

SUMMARY
Clazy warns about line 70 (c3 = c2) in the attached file, saying "no assign operator". But there is an assign operator, it's just implemented by "= default".

STEPS TO REPRODUCE
1. Open file in QtCreator

OBSERVED RESULT
Warning on line 70

EXPECTED RESULT
No warning

SOFTWARE/OS VERSIONS
On the command line, clazy --version
clazy version: 1.10
clang version: 12.0.1
but I don't know if QtCreator uses that or has its own.

ADDITIONAL INFORMATION
Comment 1 Sergio Martins 2021-10-17 19:44:10 UTC
Counting instances is a very special edge case.

The more common cases are your ctor allocating a resource, and the dtor freeing it. The default assign-op would be very bad in such case, example:

struct A {};

struct Test {

    Test() { m_a = new A(); }

    ~Test() { delete m_a; }

    Test& operator=(const Test&) = default;

    A *m_a;
};


Would lead to double-delete.

I understand your point, but I think it's better to have a false-positive than having more false-negatives which cause crashes
Comment 2 David Faure 2021-10-26 19:08:19 UTC
(sorry for the delay in my reply)

But that's an explicit operator=, the developer thought about it and decided =default would do the job.

If operator= is actually implemented, clazy will be silent, even if the implementation is nonsense or empty or insufficient.... E.g. Test& operator=(const Test& o) { return o; } :-)

The current logic is "If there's an implementation, trust that it's OK. If there's =default, assume it's wrong". This seems very odd to me.

=default *is* an implementation, so it should be treated the same.
Your example shows a case where it's a wrong implementation, indeed, just like the above is a wrong implementation.

clazy can't detect if the implementation is right or wrong, all it can do is catch the case where the developer didn't even think about operator=.
Comment 3 Sergio Martins 2021-10-26 19:47:53 UTC
Git commit 854f3408dfc9d2d96d2a9bda46660e2074315d6c by Sergio Martins.
Committed on 26/10/2021 at 19:44.
Pushed by smartins into branch 'master'.

rule-of-two-soft: Honour explicitly defaulted copy-ctor/operator

The user signaled he knows what he's doing and there's even valid
use cases

M  +5    -2    src/checks/level1/rule-of-two-soft.cpp
A  +30   -0    tests/rule-of-two-soft/bug443343.cpp  *
A  +1    -0    tests/rule-of-two-soft/bug443343.cpp.expected
M  +3    -0    tests/rule-of-two-soft/config.json

The files marked with a * at the end have a non valid license. Please read: https://community.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page.


https://invent.kde.org/sdk/clazy/commit/854f3408dfc9d2d96d2a9bda46660e2074315d6c