Skip to content

enabled and fixed modernize-pass-by-value clang-tidy warnings#4169

Merged
danmar merged 3 commits intocppcheck-opensource:mainfrom
firewave:tidy-pass-by-value
Jul 28, 2022
Merged

enabled and fixed modernize-pass-by-value clang-tidy warnings#4169
danmar merged 3 commits intocppcheck-opensource:mainfrom
firewave:tidy-pass-by-value

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Jun 3, 2022

No description provided.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Jun 3, 2022

Do we have a check for this? That would be quite a few false negatives to look into...

@chrchr-github
Copy link
Copy Markdown
Collaborator

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jun 4, 2022

We don't have a checker that recommends to pass by value and use std::move.

We do have the checker that recommends to pass by reference that chr pointed out but I am not sure how valid that is after std::move was added to C++.

@chrchr-github
Copy link
Copy Markdown
Collaborator

Right, I misread the title, thinking it was about passing by reference.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Jun 4, 2022

So I will just file a basic "new check" ticket which will sit there for a while just for completeness sake. If there isn't any yet...

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Jul 5, 2022

This seems to trigger a bug in the naming add-on. I am not able to look into/work on that. Any takers are welcome and I can rebase it after that issue was fixed.

@firewave firewave force-pushed the tidy-pass-by-value branch from f22c2f1 to a658e6d Compare July 20, 2022 09:21
@firewave
Copy link
Copy Markdown
Collaborator Author

I suppressed the naming-varname warning for now until it works properly. I also filed https://trac.cppcheck.net/ticket/11210 about this.

@firewave firewave marked this pull request as draft July 20, 2022 11:14
@firewave firewave force-pushed the tidy-pass-by-value branch from a658e6d to 795f8ce Compare July 27, 2022 13:24
@firewave
Copy link
Copy Markdown
Collaborator Author

I suppressed the naming-varname warning for now until it works properly. I also filed https://trac.cppcheck.net/ticket/11210 about this.

The need for multiple suppressions seems a bit strange. I wonder if this is related to it being an addon.

@firewave firewave force-pushed the tidy-pass-by-value branch from 795f8ce to 93f5015 Compare July 27, 2022 13:48
@firewave firewave marked this pull request as ready for review July 27, 2022 14:42
@danmar danmar merged commit b65b47d into cppcheck-opensource:main Jul 28, 2022
@firewave firewave deleted the tidy-pass-by-value branch July 28, 2022 21:28
@firewave
Copy link
Copy Markdown
Collaborator Author

The need for multiple suppressions seems a bit strange. I wonder if this is related to it being an addon.

It appears this is already tracked as https://trac.cppcheck.net/ticket/2799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants