Bug 379342 - clazy-function-args-by-value false positive when ref constructor arg is stored as ref in class
Summary: clazy-function-args-by-value false positive when ref constructor arg is store...
Status: RESOLVED FIXED
Alias: None
Product: clazy
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Sergio Martins
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-29 09:42 UTC by Markus Enzenberger
Modified: 2017-09-18 21:40 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 Markus Enzenberger 2017-04-29 09:42:52 UTC
The following snippet creates a clazy-function-args-by-value warning even if it is essential to use a reference since the class stores a copy of the reference. If the user follows the recommendation by clazy, a bug is intruduced in the code, which not every compiler will detect and warn about.

struct A
{
    const int& m_a;
    A(const int& a) : m_a(a) { }
};

I tested this with the current clazy from github (f5e6ccbc). Funnily enough, clazy does not complain if a non-const reference is used for the member and the argument.
Comment 1 Markus Enzenberger 2017-04-29 10:03:16 UTC
Forget the last sentence about the non-const-ref case, of course there is no optimization there.
Comment 2 Sergio Martins 2017-04-30 18:36:44 UTC
(In reply to markus.enzenberger from comment #0)
> The following snippet creates a clazy-function-args-by-value warning even if
> it is essential to use a reference since the class stores a copy of the
> reference. If the user follows the recommendation by clazy, a bug is
> intruduced in the code, which not every compiler will detect and warn about.

Do you know which compilers actually warn about this ?
Comment 3 Markus Enzenberger 2017-05-01 05:56:23 UTC
I currently only have access to gcc and clang and they both do not warn if the reference argument is replaced by a value argument even if it obviously makes no sense to store a reference to a temporary.
Comment 4 Sergio Martins 2017-09-18 20:59:26 UTC
Git commit 64541809a9a98c6f16f80587baacb1ab7eeef381 by Sergio Martins.
Committed on 18/09/2017 at 20:57.
Pushed by smartins into branch '1.2'.

function-args-by-value: Add unit-test for bug 379342

M  +2    -1    tests/function-args-by-value/config.json
M  +6    -0    tests/function-args-by-value/main.cpp

https://commits.kde.org/clazy/64541809a9a98c6f16f80587baacb1ab7eeef381
Comment 5 Sergio Martins 2017-09-18 21:40:36 UTC
Git commit 51ba8ddfaffbe0d15e5165b4d135d9a62749dd8f by Sergio Martins.
Committed on 18/09/2017 at 21:39.
Pushed by smartins into branch '1.2'.

function-args-by-value: Fix false positive

When passing to a ctor that inits members by ref we must preserve
the ref in the original param.

M  +23   -1    src/checks/level2/function-args-by-value.cpp
A  +12   -0    tests/function-args-by-value/bug379342.cpp     [License: UNKNOWN]  *
A  +1    -0    tests/function-args-by-value/bug379342.cpp.expected
M  +4    -2    tests/function-args-by-value/config.json
M  +0    -6    tests/function-args-by-value/main.cpp

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


https://commits.kde.org/clazy/51ba8ddfaffbe0d15e5165b4d135d9a62749dd8f