This is an archive of the discontinued LLVM Phabricator instance.

[Coroutines] Fix code coverage for coroutine
ClosedPublic

Authored by lxfind on Jun 30 2020, 8:34 PM.

Details

Summary

Previously, source-based coverage analysis does not work properly for coroutine.
This patch adds processing of coroutine body and co_return in the coverage analysis, so that we can handle them properly.
For coroutine body, we should only look at the actual function body and ignore the compiler-generated things; for co_return, we need to terminate the region similar to return statement.
Added a test, and confirms that it now works properly. (without this patch, the statement after the if statement will be treated wrongly)

Diff Detail

Event Timeline

lxfind created this revision.Jun 30 2020, 8:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2020, 8:34 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
modocache accepted this revision.Jul 1 2020, 8:35 AM

Excellent, thanks for this!

This revision is now accepted and ready to land.Jul 1 2020, 8:35 AM
This revision was automatically updated to reflect the committed changes.
fhahn added a subscriber: fhahn.Jul 1 2020, 10:40 AM

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Looks like it's creating a tmp file within the test dir. Let me take a look.

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Let me revert it for now while fixing it. If you could accept: https://reviews.llvm.org/D82984

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Fix patch in https://reviews.llvm.org/D82986

thakis added a subscriber: thakis.Jul 1 2020, 12:55 PM
thakis added inline comments.
clang/test/CoverageMapping/coroutine.cpp
1

Since this line is missing an -o /dev/null, it writes a .ll file into the test directory, which lit tries to run as test next time, causing check-clang to fail: http://45.33.8.238/linux/21841/step_7.txt

Please add -o /dev/null, and please also add a RUN: rm -f %S/coroutine.ll to clean up bots (with a fixme to remove that rm line once it's been in the tree for a week or so)