-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Enable C++ branch coverage when using gcov 8 or later. #18879
Conversation
33f23ec
to
ed9ebda
Compare
There's quite a bit of duplication of helper functions in the various coverage related shell tests; I'll have a go at reducing that in a follow up change. |
ed9ebda
to
4280071
Compare
# Extract gcov's version: the output of `gcov --version` contains the | ||
# version as a set of major-minor-patch numbers, of which we extract | ||
# the major version. | ||
gcov_major_version=$("${GCOV}" --version | sed -n -E -e 's/^.*\s([0-9]+)\.[0-9]+\.[0-9]+\s?.*$/\1/p') |
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.
Why are you using a different mechanism to extract the major version here?
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.
It's simply replacing what was removed in #18878.
This approach handles using llvm-cov gcov
- the tests don't need to worry about that.
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.
That said, when I come to consolidate things, I'll probably use the same approach everywhere (no reason not to).
But right now, I don't want to change things everywhere in the tests...
Branch coverage was disabled due to a gcov bug, but this was fixed for GCC 8: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84879. It doesn't make sense to disable all C++ branch coverage for everyone because of a bug in an old GCC version. RELNOTES: Enable C++ branch coverage if gcov version is 8 or newer.
4280071
to
0cfd753
Compare
Branch coverage was disabled due to a gcov bug, but this was fixed for
GCC 8: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84879.
It doesn't make sense to disable all C++ branch coverage for everyone
because of a bug in an old GCC version.