Fix #14520: false positive: passedByValue on declaration with const#8460
Fix #14520: false positive: passedByValue on declaration with const#8460ludviggunne wants to merge 4 commits intodanmar:mainfrom
Conversation
| check("struct X { int a[5]; }; void f(const X v);"); | ||
| check("struct X { int a[5]; }; void f(const X v) { (void) v; }"); | ||
| ASSERT_EQUALS("[test.cpp:1:40]: (performance) Function parameter 'v' should be passed by const reference. [passedByValue]\n", errout_str()); |
There was a problem hiding this comment.
Shouldn't we keep the existing test (in addition to the changed one) as that is the false positive fixed here.
There was a problem hiding this comment.
I believe the intention of this test is to ensure we warn for struct parameters, so I wanted to keep that meaning. Although, now that you point it out there is a false positive here, but I think it's unrelated to the ticket (there is an implementation).
There was a problem hiding this comment.
There is a misunderstanding.
The test in its original form without the implementation showed the false positive. That is the fix that we no longer warn for it.
Instead of just adding the implementation so the result stays the same we should have tests with and without implementation.
There was a problem hiding this comment.
I think I'm still misunderstanding... after this patch this test would have to be updated either way (result/input code). 0357bb6 introduces a test without implementation, whereas this now has an implementation. Are you suggesting two new tests that are identical except for present/missing implementation?
There was a problem hiding this comment.
The new test uses the exact case from the ticket which involves a class and the constructor.
The test in question here is about free-standing functions which would be missing.
There was a problem hiding this comment.
Ah, thanks I see
Co-authored-by: Oliver Stöneberg <firewave@users.noreply.github.com>
|
3 participants
No description provided.