[PATCH] D62359: [dwarfdump] Make recursion affect the parent chain

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

In D62359#1515107 <https://reviews.llvm.org/D62359#1515107>, @dblaikie wrote:

> Is this the desired behavior?
>
> Currently, I assume, searching for a particular DIE offset dumps that DIE, its parent, and all its children. (& the recursion limit is entirely ignored?)


Yup.

> This patch, if I understand it, dumps from the CU to the target DIE until the recursion limit is reached. So you get some parent DIE of the one you searched for.
> 
> I would've probably thought that the entire parent chain would be dumped, and then the recursion limit would limit how deep into the children of the target DIE would be dumped.
> 
> Thoughts?

This is what happens currently (without the patch). The recursion depth only applies to children, and passing `-p` shows the whole parent chain.

I think there's a case to be made for both. With the patch, if you pass `-p -c -r 2`, you'll get two parents and two children, which is a little weird.

@aprantl since you requested this, WDYT?



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:559
     return Indent;
-  Indent = dumpParentChain(Die.getParent(), OS, Indent, DumpOpts);
+  if (DumpOpts.RecurseDepth > 0 && Depth >= DumpOpts.RecurseDepth)
+    return Indent;
----------------
aprantl wrote:
> `if (DumpOpts.RecurseDepth && Depth >= DumpOpts.RecurseDepth)`
The recursion depth can be negative, so that wouldn't work.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:562
+  Depth += 1;
+  Indent = dumpParentChain(Die.getParent(), OS, Indent, DumpOpts, Depth);
   Die.dump(OS, Indent, DumpOpts);
----------------
aprantl wrote:
> `Depth + 1`?
Makes sense!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62359