[PATCH] D60364: [DWARF] Set discriminator to 0 for DW_LNS_copy

Wed Apr 10 14:24:09 PDT 2019

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D60364#1460672 <https://reviews.llvm.org/D60364#1460672>, @MaskRay wrote:

> In D60364#1460434 <https://reviews.llvm.org/D60364#1460434>, @dblaikie wrote:
>
> > Should there also be an LLVM test - given that LLVM does emit DW_LNS_copy by the looks of it, and if it's been being incorrectly interpreted/dumped, then that seems to imply there's some missing LLVM test coverage for that case, maybe?
>
>
> Do you mean a test of `lib/MC/MCDwarf.cpp`? As I understand it, `lib/DebugInfo/DWARF` is mostly the reader part and `MC` the writer part. With my casual reading, `MCDwarf.cpp` seems to do the correct thing by doing `Discriminator = 0;`. I haven't looked into that part yet but I can investigate that and add a test if needed in a separate change.

Right (& yes, lib/DebugInfo is for reading, lib/MC is for writing) - it seems likely lib/MC is doing the right thing, but it might be missing some test coverage (given the bug in the dumper, it should've been visible in some test case).

But this patch looks generally good.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60364