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 some bugprone-macro-parentheses warnings #2732

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

ChristianTackeGSI
Copy link
Contributor

Description

When using the public headers of catch2 in another project that uses clang-tidy with some checks enabled, then some warnings in catch2's headers are also reported. Those can distract from the real problem in our project. So I would like to reduce the warnings from catch2.

This fixes a bunch of bugprone-macro-parentheses warnings.

See: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/macro-parentheses.html

GitHub Issues

None

@horenmar
Copy link
Member

This does not even compile.

@ChristianTackeGSI
Copy link
Contributor Author

Okay, in my first attempt this used to compile. Now it also fails.

Anyway: I have stripped this down to the part that does compile and also gives a successful make test. Hope this is better now?

@codecov
Copy link

codecov bot commented Aug 20, 2023

Codecov Report

Merging #2732 (1d03457) into devel (9c541ca) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            devel    #2732      +/-   ##
==========================================
+ Coverage   91.36%   91.37%   +0.01%     
==========================================
  Files         190      190              
  Lines        7855     7855              
==========================================
+ Hits         7176     7177       +1     
+ Misses        679      678       -1     

@horenmar
Copy link
Member

Thanks, I'll take a look.

Okay, in my first attempt this used to compile. Now it also fails.

I suspect that you didn't have test build enabled, and because the main library doesn't exercise those macros, breaking them won't stop it from compiling.

@ChristianTackeGSI
Copy link
Contributor Author

Is there anything left, that I forgot about?
What's the next step?

@horenmar
Copy link
Member

You haven't forgotten about anything, I did forget about the PR though.

Copy link
Member

@horenmar horenmar left a comment

Choose a reason for hiding this comment

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

I am willing to merge the other changes, as they should not break anything.

When using the public headers of catch2 in another project
that uses clang-tidy with some checks enabled, then some
warnings in catch2's headers are also reported.

This fixes a bunch of bugprone-macro-parentheses warnings.

See: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/macro-parentheses.html
@horenmar horenmar merged commit 0fb817e into catchorg:devel Sep 25, 2023
74 checks passed
@ChristianTackeGSI ChristianTackeGSI deleted the pr/fix_bugprone branch September 25, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants