Bug 430672

Summary: Level 2 lint copyable-polymorphic is a false positive for pure virtual classes
Product: [Developer tools] clazy Reporter: bepzi
Component: generalAssignee: Unassigned bugs mailing-list <unassigned-bugs>
Status: RESOLVED FIXED    
Severity: minor CC: faure, smartins
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: unspecified   
OS: Microsoft Windows   
Latest Commit: https://invent.kde.org/sdk/clazy/commit/9ec85f78a18c031e49cdc62517175bfee7ee92ea Version Fixed In:
Sentry Crash Report:

Description bepzi 2020-12-21 17:36:24 UTC 
SUMMARY

The lint for classes that are both copyable and polymorphic triggers for pure virtual classes, which can't be constructed or copied at all.

The implementation checks if 1) it's polymorphic and 2) if there's a callable copy constructor or copy-assign constructor:

https://github.com/KDE/clazy/blob/4a4742c750a563180cbf65c2438540cc7e700f7a/src/checks/level2/copyable-polymorphic.cpp#L56-L63

But pure virtual classes can have "callable" (as determined by this code) copy constructors despite not actually being copyable or even constructable.

STEPS TO REPRODUCE
1. Create a class with a pure virtual method and a not-deleted copy or copy-assign constructor.
2. Run clazy with the level 2 lint 'copyable-polymorphic' enabled.
3. Observe that clazy flags the pure virtual class for being a copyable polymorphic.

OBSERVED RESULT

Pure virtual classes are flagged as needing their copy and copy-assign constructors deleted.

EXPECTED RESULT

The lint should not trigger, because pure virtual classes cannot be constructed or copied and are not vulnerable to vtable slicing.

SOFTWARE/OS VERSIONS
Windows: 10.0.19042 Build 19042
Qt Version: 5.12.9
Qt Creator: 4.14.0 (Dec 17 2020)
Clazy: (`clazy-standalone.exe --version` only reports that it's based on LLVM version 11.0.0)
Comment 1 Sergio Martins 2021-01-10 13:50:32 UTC 
If they can't be copied why do they have a copy-ctor in the first place ?
Comment 2 David Faure 2021-06-03 12:13:29 UTC 
So that subclasses can be copied. Of course the right way to do this then is to make the copy constructor protected (rather than defaulting to public) in the base class. See https://bugs.kde.org/show_bug.cgi?id=438027, which I now believe is the same bug, sorry about that.
Comment 3 Sergio Martins 2021-06-03 21:50:13 UTC 
Git commit 9ec85f78a18c031e49cdc62517175bfee7ee92ea by Sergio Martins.
Committed on 03/06/2021 at 21:45.
Pushed by smartins into branch 'master'.

copyable-polymorphic: Don't warn for some final classes

If the class is final and its base classes don't have public
copy-ctor/assign then it's ok
Related: bug 438027

M  +28   -4    src/checks/level2/copyable-polymorphic.cpp
M  +23   -0    tests/copyable-polymorphic/main.cpp
M  +1    -0    tests/copyable-polymorphic/main.cpp.expected

https://invent.kde.org/sdk/clazy/commit/9ec85f78a18c031e49cdc62517175bfee7ee92ea