[llvm] Reapply "[llvm/DWARF] Recursively resolve DW_AT_signature references"… (PR #99495)

[David Blaikie via llvm-commits]

================
@@ -173,7 +169,8 @@ DWARFTypePrinter::appendUnqualifiedNameBefore(DWARFDie D,
   case DW_TAG_base_type:
   */
   default: {
-    const char *NamePtr = dwarf::toString(D.find(DW_AT_name), nullptr);
+    const char *NamePtr =
+        dwarf::toString(D.findRecursively(DW_AT_name), nullptr);
----------------
dwblaikie wrote:

> > It seems to me that perhaps we could/should introduce a `getAttributeValueAsReferencedDieRecursively`? That recurses the referenced type, but doesn't recurse the attribute (since this is called on an already found attribute - with find or findRecursively)?
> 
> This seems a bit too magical to me. `getAttributeValueAsReferencedDieRecursively()` is not that much shorter than `getAttributeValueAsReferencedDie().resolveTypeUnitReference()`, but the latter is clearer in what's happening (and I think this will need to be evaluated on a case-by-case basis, as depending on what exactly you want to do (e.g. which children to look at) you might or might not want to do the "recursive" part).
> 
> I think it would make sense if it were named somewhat differently to so that it's clear this is used for types and that it will point you to some kind of a canonical type definition... oops, I think i've just reinvented the existing
> 
> ```
> static DWARFDie resolveReferencedType(DWARFDie D, DWARFFormValue F) {
>   return D.getAttributeValueAsReferencedDie(F).resolveTypeUnitReference();
> }
> ```

Fair enough - though if there's more places that want this functionality, I'd be fine with elevating it to some more general purpose helper on DWARFDie rather than hidden as a static helper in DWARFTypePrinter.

> > > getAttributeValueAsReferencedDie will only resolve the first reference, and not the second one. Unless we want to teach it to resolve references recursively (which sounds like a bad idea)
> > 
> > 
> > Hmm, you mentioned that this sounds like a bad idea, but then mention that we should do it a few lines later? Not sure which you're thinking of, or why it might be a bad idea? (I mean, it might be, not suggesting it isn't, but trying to understand better)
> > Or perhaps you were presenting these as alternatives...
> 
> I'm sorry, I was being imprecise here. I think that the _recursive_ reference resolution is a bad idea. The rest of the text omits the word "recursive", which was meant to mean that it should handle a single DW_FORM_ref_sig8 reference (and nothing more)

Ah, OK.

> > * getAttributeValueAsReferencedDie should resolve type unit references
> 
> It should resolve a single DW_FORM_ref_sig8 like it does in this patch
> 
> > * findRecursively should do that as well
> 
> It should recurse on DW_AT_signature (like it does in this patch), by calling getAttributeValueAsReferencedDie
> 
> > * we should leave type printer alone?
> 
> just that

Yeah, that sounds OK to me. Thanks for your patience/giving this a go! (& if you are touching the lldb side of this sort of change too - it'd be handy to have similar type resolution helpers (ideally /really/ similar, because Real Soon Now, we'd like the DWARFTypePrinter to be generalized across both DWARFDie and DWARFDIE), so thanks for keeping this in mind (: )

https://github.com/llvm/llvm-project/pull/99495