Skip to content

mitigated most clang-tidy warnings in headers#4175

Merged
danmar merged 4 commits intocppcheck-opensource:mainfrom
firewave:tidy-header-2
Jul 26, 2022
Merged

mitigated most clang-tidy warnings in headers#4175
danmar merged 4 commits intocppcheck-opensource:mainfrom
firewave:tidy-header-2

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Jun 6, 2022

I did not enable the headers in .clang-tidy yet since there are some warnings I am not sure how to suppress. This will be done within #4100.

Comment thread gui/resultstree.h Outdated
*
* @param hidden true if there are some hidden results, or false if there are not
*/
// NOLINTNEXTLINE(readability-inconsistent-declaration-parameter-name) - caused by generated MOC code
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

there is a cppcheck warning that does not generate this noise. so I suggest we drop this clang-tidy warning.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I dropped these changes for now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@danmar This should be fine to merge now.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Jul 5, 2022

This should be okay to merge.

Comment thread lib/templatesimplifier.h Outdated
// Make sure a family flag is set and matches.
// This works because at most only one flag will be set.
// NOLINTNEXTLINE(misc-redundant-expression)
return ((mFlags & fFamilyMask) & (decl.mFlags & fFamilyMask)) != 0;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it's ok to fix this warning

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's a false positive. I still need to file a ticket about this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I assume it complains that it's redundant to repeat fFamilyMask right? So the fix would be either something like:

return (mFlags & decl.mFlags & fFamilyMask) != 0;

or:

return (mFlags & fFamilyMask) && (decl.mFlags & fFamilyMask);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I did not look closely at the warning before. It is indeed about the redundant fFamilyMask.

I chose the send suggestion for readability.

@danmar danmar merged commit 8874638 into cppcheck-opensource:main Jul 26, 2022
@firewave firewave deleted the tidy-header-2 branch July 26, 2022 09:12
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.

2 participants