[PATCH] D36558: [llvm-objcopy] Add support for nested and overlapping segments

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 30 15:00:03 PDT 2017


jakehehrlich added a comment.

In https://reviews.llvm.org/D36558#856377, @jhenderson wrote:

> Lots of small nits, and one or two slightly larger issues to address here. I like the overall approach though.
>
> I'm not all that happy with the level of test coverage available, but at the same time, I'm not sure that there's much that can be done without unit-testing a lot of this, or creating large numbers of virtually identical Lit tests, which seems a little excessive.


I'm not that happy with test coverage either. You caught a mistake that wasn't just a little mess sup on my part (Segment->OriginalOffset - PSeg->OriginalOffset vs Segment->Offset - PSeg->Offset). I think it's pretty clear that the tests are not enough here. There are lots of branches in this code that likely aren't being checked as well. I don't know of a way to add unit tests in LLVM.

Are you aware of some way of adding unit tests in llvm?


Repository:
  rL LLVM

https://reviews.llvm.org/D36558





More information about the llvm-commits mailing list