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

[Pavel Labath 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);
----------------
labath wrote:

> Aside: GCC doesn't use this style for all type unit references, it actually uses 3 different styles depending on context

Okay, not so nice then, I guess :)


> 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();
}
```

> 
> > 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)

> -    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

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