Skip to content

llvm-cov: Insufficient merging strategy for template instantiations #119299

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

Open
chapuni opened this issue Dec 10, 2024 · 2 comments · May be fixed by #121194 or #121196
Open

llvm-cov: Insufficient merging strategy for template instantiations #119299

chapuni opened this issue Dec 10, 2024 · 2 comments · May be fixed by #121194 or #121196

Comments

@chapuni
Copy link
Contributor

chapuni commented Dec 10, 2024

llvm-cov report

llvm-cov report merges metrics for instantiations with the strategy "Take max(covered)/max(num) for each instantance".

  • Line (and region): Each instance may have different number of records, due to skipped regions by instantiation-time constants. This sometimes confuses the strategy max/max.
  • Branch and MC/DC: Due to folded conditions, the result might show false negative with the strategy "max/max".

llvm-cov show Source view

The merged counters can be seen in Line (and region) counters. They show max counters in each record. I think this will be less informative if any instances are not fully covered on their own.

Branches and MC/DC records are not merged in the source view. It is noisy. Actual uncovered branches should be found in instantiation subviews.

Anyways, the source view doesn't reflect the summary. (see also #119282)

Possible merge strategies

Planning. It should be better if merged result can be seen in the source view. llvm-cov will accept one of strategies below by command line options.

They below are listed in order of tolerance.

Count up after merging records

  • Line (and region): Skipped lines will be filled.
  • Branch: Folded conditions will be dissolved.
  • MC/DC: Discovers more independence pairs by merging test vectors. (This will require moving findIndependencePairs deferred)

I think:

  • For the coverage of the complex codebase (e.g. LLVM), merging records will be more practical.
  • For local scope non-template functions (static inline functions in header files), merging records should be the best.

Max(any)

This assumes harderning the current strategy. Show "covered" if one of instantiations is fully covered.

This might be confused in skipped/folded stuff.

Min(all)

This actually means, "Don't merge, just count up each instance". The template will be "not covered at all" if at least one instantiation is not executed.

@chapuni
Copy link
Contributor Author

chapuni commented Mar 13, 2025

@evodius96 Do you have any opinions re. this direction?

@bartlettroscoe
Copy link

Related to this is inconsistent coverage metrics due to ifdefs and building with different defines in different translation units. This happens for unit tests, for example, where you don't need to link all of the different inconsistent translation units together. This also can happen with anonymous namespaced instantiations that get linked into the same libraries and exectuables. (And we have all of that with Trilinos.)

My current plan is to just take maxes over all of the (unit test) executables using llvm-cov report. I will use a similar strategy for merging coverage results for the different template instantiations for the same template function. That should be a fairly easier implementation. The more accurate implementation would be to deal with the raw region and branch coverage data in the JSON dicts output from llvm-cov export which you can match up using line numbers in those dicts. That is a more complex implementation, but it should yield and higher and more accurate view of the coverage of templated and/or ifdefed templated code.

NOTE: I need to merge all source data with clang-tidy metrics for function size and complexity as part of a larger effort to get better static and dynamic metrics for C++ code. See:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants