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

feat(coverage): Demangling C++ & Rust function names in coverage results #398

Merged

Conversation

vogelsgesang
Copy link
Collaborator

Demangle C++ and Rust symbols using c++filt and rustfilt. We rely on those tools to be installed on the $PATH.

I was also considering whether I should compile c++filt and / or rustfilt to WebAssembly, but decided against it, given that this would make the build much more complicated and it is currently unclear to me whether we will stick with the "traditional" tool chains (npm, webpack, cargo, ...) or if we will pivot to using Bazel.

Demangle C++ and Rust symbols using `c++filt` and `rustfilt`. We rely on
those tools to be installed on the $PATH.

I was also considering whether I should compile `c++filt` and / or
`rustfilt` to WebAssembly, but decided against it, given that this would
make the build much more complicated and it is currently unclear to me
whether we will stick with the "traditional" tool chains (npm, webpack,
cargo, ...) or if we will pivot to using Bazel.
// we prefer rustfilt over c++filt. For C++ mangled names, rustfilt
// will fail and we will fallback to c++filt.
// See https://internals.rust-lang.org/t/symbol-mangling-of-rust-vs-c/7222
const demangled =
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a shame that we don't know what language the lcov was generated from here. Is there no reasonable way of getting this info out of Bazel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The LCOV file from Bazel might even be merged across multiple different languages. E.g., if I run ./bazel coverage //..., Bazel will merge coverage between Java, C++ and Go test cases.

We could use some heuristic by looking at the file extension, but then again: What would we gain by doing so?

What is your underlying concern with trying multiple decoders and using the first one that can interpret the name?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was just concerned that a decoder could incorrectly demangle a symbol from another language. However, I'm not sure how likely this is and if this isn't possible to easily determine the language then I'm happy with this approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we should be good here in almost all cases. As soon as we run into it, we will find a better solution 🙂

@vogelsgesang vogelsgesang merged commit d28c294 into bazel-contrib:master Jun 11, 2024
2 checks passed
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