I gave Zequan some guidance suggesting that he shouldn't revert. My possibly-incomplete understanding at the time was that reverting that change will create a regression for Clang code coverage, even though it fixes Rust coverage. I felt that it was better to fix clang first, rather than reintroducing the issue. I think I was worried that reintroducing the coverage bug would jeopardize our efforts to continuously release clang in Chromium, and it was likely to be the kind of issue that wouldn't show up in release testing and that users would instead discover it weeks later. It's possible that I accidentally I put my priorities ahead of the community priorities, but I was hoping we'd get a clang fix in a matter of weeks and get this all wrapped up quickly. However, I can't claim to have a deep understanding of the coverage mapping format. If there is a good reason to expedite the llvm-cov change revert, we can do that first too.

On the swift side, it's not urgent that D85036 be reverted. After it landed, we discovered and addressed a deeper issue in the swift implementation (swiftlang/swift@cfd38c4).

Thanks for the explanations. Given that the bugs fixed by D85036 were present in the release/11.x branch, I think it's best to revert this in release/12.x to avoid introducing a regression.

Once https://reviews.llvm.org/D97101 lands in main, we can look into cherry-picking it to release/12.x for the 12.0.1 release.

Merged: a92ceea

Reverting this patch breaks some of the llvm-cov tests in the branch:

https://github.com/llvm/llvm-project/runs/1957551005

Can someone take a look at this?

Reverting this patch breaks some of the llvm-cov tests in the branch:

For the three branch tests, it was because branch coverage landed after that change. The statistics it currently reports may not be correct due to the bug. I think, you can just update the statistics. I don't know about the symbolizer test failure.

Here's a diff for updating the statistics in the tests: https://gist.github.com/nikic/d8fb6c12b20b60843d87b77dbefa2b48

Reverting this patch breaks some of the llvm-cov tests in the branch:

For the three branch tests, it was because branch coverage landed after that
change. The statistics it currently reports may not be correct due to the
bug. I think, you can just update the statistics. I don't know about the
symbolizer test failure.

The symbolizer failure is unrelated and has been already fixed.