[PATCH] D36813: [Coverage] Build sorted and unique segments

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In https://reviews.llvm.org/D36813#857562, @ikudrin wrote:

> The patch doesn't apply to the current tip clearly. And after applying it manually and fix compilation errors, I have two failing tests for llvm-cov: `tools/llvm-cov/showLineExecutionCounts.cpp` and `tools/llvm-cov/threads.c`, both of which fail with "Coverage segments not unique or sorted" assertion.


Both of these tests load the same covmapping file. That file was generated by an older, buggy clang. That version of clang produced coverage mapping regions where the start of the region actually ended after the end of the region. I have separate patches locally which (1) teach the coverage mapping reader and writer to report a recoverable Error when they find bogus mapping regions produced by the frontend, and (2) regenerate the buggy covmapping file. The clang bug was fixed long ago.

I had planned on landing the changes in one go but I can commit those fixes right away if you'd prefer.

> I'd also prefer a set of patches where each corner case is fixed in a separate patch and which gradually lead to the final code. Of course, if it's possible. Right now it looks like just changing an old implementation to a new one, and it could be hard to investigate later and understand each particular decision in the code. What do you think?

I'm a bit hesitant to do that. I'm not sure that splitting up the logic would result in individual patches which are easier to understand. It would also mean that none of the patches could be committed, since the stage2 coverage bot would hit the 'sorted & unique' assertion.

If there's any new logic which is obscure or not well-commented, I'd be happy to explain it in more detail or improve the comments.


https://reviews.llvm.org/D36813