-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs: Exclude modules' bench_impl.h
headers from coverage report
#1722
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
docs: Exclude modules' bench_impl.h
headers from coverage report
#1722
Conversation
FWIW, on Ubuntu 24.04, when using GCC 13.3.0 and gcovr 7.0, I need to pass an extra |
Let's add it, and I also need |
Otherwise, commands fail with the error: ``` Stderr of gcov was >><< End of stderr Exception was >>Got function secp256k1_scalar_split_lambda on multiple lines: 67, 142. You can run gcovr with --merge-mode-functions=MERGE_MODE. The available values for MERGE_MODE are described in the documentation.<< End of stderr ```
b1b8869
to
1aecce5
Compare
Thanks! Done.
Yes, it outputs:
|
Then I guess the best thing for now is to add a note/comment that this should be omitted on gcovr before 8.3. |
What GCC version are you using? Any other specific steps to reproduce the issue? EDIT: nm, I can reproduce it on Fedora 42 with GCC 15.1.1. |
I think an alternative for <8.3 is Fwiw:
|
It was introduced in gcovr/gcovr#898 and has been available since version 8.0. The |
Taken this:
|
lgtm but I suggest adding a note like this: On gcovr >=8.3, Then we have a built-in reminder that we can remove the old argument in the future. |
Otherwise, commands might fail due to bugs in the `gcov` tool.
9982c5c
to
0458def
Compare
Thanks! The note has been added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 0458def
@josibake Want to review this (with whatever tool versions you have on your system)? |
Nice! Thanks for adding this, I ran into this recently and added gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary
(INFO) Reading coverage data...
(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_accum_mul(secp256k1_uint128 *r, uint64_t a, uint64_t b) {'.
(WARNING) Ignoring suspicious hits in line ' *r += (uint128_t)a * b;'.
(WARNING) Ignoring suspicious hits in line '}'.
(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_rshift(secp256k1_uint128 *r, unsigned int n) {'.
(WARNING) Ignoring suspicious hits in line ' *r >>= n;'.
(WARNING) Ignoring suspicious hits in line '}'.
(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE uint64_t secp256k1_u128_to_u64(const secp256k1_uint128 *a) {'.
(WARNING) Ignoring suspicious hits in line ' return (uint64_t)(*a);'.
(INFO) Writing coverage report...
------------------------------------------------------------------------------
GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
<snip> The warning goes away when running with gcovr --gcov-suspicious-hits-threshold=140737488355330 --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary
(INFO) Reading coverage data...
(INFO) Writing coverage report...
------------------------------------------------------------------------------
GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
<snip> Also confirmed I got the same coverage report with both flags. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 0458def
Tested with gcovr@8.3
and confirmed I get a warning with the old flag (--gcovr-ignore-parse-errors=all
), and that those warnings go away when running with the new flag suggest for gcovr >= 8.3
Additionally, this comment has been addressed.