Bug 430672 - Level 2 lint copyable-polymorphic is a false positive for pure virtual classes
Summary: Level 2 lint copyable-polymorphic is a false positive for pure virtual classes
Status: RESOLVED FIXED
Alias: None
Product: clazy
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Microsoft Windows
: NOR minor
Target Milestone: ---
Assignee: Unassigned bugs mailing-list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-21 17:36 UTC by bepzi
Modified: 2021-06-03 21:50 UTC (History)
2 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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