[PATCH] D42297: [DebugInfo] Basic .debug_names dumping support

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D42297#981752, @labath wrote:

> I would really appreciate it if someone could compare the code with the dwarf
>  specification, to make sure we are reading it the same way.


I didn’t find time for this today, but I will do so on Monday.

> The other open question for me here is the difference in dwarf-dump -apple-names
>  and -debug-names format. I chose to use a ScopedPrinter for dumping because the
>  dwarf tables have more structure to them than the apple ones, and ScopedPrinter
>  made dumping that much easier. However, that makes the format quite different.
>  If you are OK with this format, and there is no requirement to preserve the
>  apple-names dump, I could switch the apple format to use a ScopedPrinter as a
>  follow-up.

I’m very much in favor for keeping both consistent. We might have some internal tests that would need to be updated which I’ll gladly take care of.

> The test case input was generated by my very-experimental very-wip .debug_names
>  generator (with some additional massaging of the assembly).



In https://reviews.llvm.org/D42297#981949, @aprantl wrote:

> May I ask you to split out the renaming and the adding of the new functionality into separate patches? Having smaller incremental steps will make it easier for us to adapt our yet-to-be upstreamed code.


Another +1 from my part. I think you can directly commit the rename.

> Seems fair. @JDevlieghere, do you agree with Pavel's assessment?

I totally agree. I was afraid this was going to be the outcome when I read the standard last time but I wanted to remain optimistic. We can always reevaluate this in the future.

So I had a quick look and generally this looks good. I’ll do a more in depth review on Monday as promised.


Repository:
  rL LLVM

https://reviews.llvm.org/D42297