[PATCH] D72337: [DebugInfo][Support] Replace DWARFDataExtractor size function

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 9 14:47:08 PST 2020


dblaikie added a comment.

In D72337#1811669 <https://reviews.llvm.org/D72337#1811669>, @jhenderson wrote:

> Added unit test case that would trigger the assertion mentioned in the description. I'm not entirely convinced that this test case is worthwhile, since with the change in how size() is calculated, it really is moot (e.g. I wouldn't have needed it if the size() function had always been the way I'm proposing).


Ah, I was more expecting an end-to-end test (with llvm-dwarfdump or similar) when you said in the patch description "The old behaviour could cause an assertion in the debug line parser"

What were the circumstances where this assertion would fail? Was the failure reachable with llvm-dwarfdump, or only via a unit test/non-production API usage?

> Also, adding the unit test makes this patch now depend on D72154 <https://reviews.llvm.org/D72154> (well, not strictly - I could implement without the dependency, but the two overlap in my branch).




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72337/new/

https://reviews.llvm.org/D72337





More information about the llvm-commits mailing list