Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance-unnecessary-value-param warning reported by clang-tidy #1106

Conversation

tanruixiang
Copy link
Member

This close #1105

@tanruixiang
Copy link
Member Author

By the way, I'm curious if there are any automated tools to fix relatively simple check like this one. @PragmaTwice

@torwig
Copy link
Contributor

torwig commented Nov 12, 2022

@tanruixiang I think such code fixes aren't similar to formatting and are able to change program behavior, so probably it's better to fix them manually, double-checking every case. Even if, in 99.99% of cases, the 'auto-fix' will do the right thing.

torwig
torwig previously approved these changes Nov 12, 2022
Copy link
Contributor

@torwig torwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

.clang-tidy Outdated Show resolved Hide resolved
@PragmaTwice
Copy link
Member

PragmaTwice commented Nov 12, 2022

By the way, I'm curious if there are any automated tools to fix relatively simple check like this one. @PragmaTwice

Sure, some clang-tidy checks provide auto-fix, but it cannot guarantee 100% right for fixed code, so I recommand at least manual review these reports one by one.

BTW, if you configure your editor/IDE correctly (vim/vscode/emacs with clangd or clion), you should see a fix button in the pop-up window in every code segment where a report exists here (if the check provides auto-fix). Some of them may also provide a button to fix all reports in the current source file.

@tanruixiang
Copy link
Member Author

BTW, if you configure your editor/IDE correctly (vim/vscode/emacs with clangd or clion), you should see a fix button in the pop-up window in every code segment where a report exists here. Some of them may also provide a button to fix all reports in the current source file.

Yes, vscode just need to download Clang-Tidy and take effect without any configuration.

@PragmaTwice
Copy link
Member

Thanks all. Merging...

@PragmaTwice PragmaTwice merged commit 2fb9a22 into apache:unstable Nov 14, 2022
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.

Fix performance-unnecessary-value-param warning reported by clang-tidy
4 participants