[PATCH] D42082: Add DWARF for discriminated unions

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 23 09:13:34 PST 2018


On Fri, Jan 19, 2018 at 8:32 AM Tom Tromey <ttromey at mozilla.com> wrote:

> >>>>> "David" == David Blaikie <dblaikie at gmail.com> writes:
>
> David> Any difficulty with modelling this more generally - having a
> David> DW_TAG_enum_type (DICompositeType) with a member that's a
> David> DW_TAG_variant_part (probably another DICompositeType?) with the
> David> members - so it doesn't matter whether there's a discriminator on
> David> the enum_type, but there can be when needed.
>
> I think this would be fine.
>
> David> Fair point - if a debugger would benefit from/need this
> David> information/distinction (ie: encoding the invariant union/enum
> David> thing needs special handling in the debugger) rather than adding
> David> an extra attribute to a DW_TAG_struct_type (and encoding the
> David> invariant union/enum thing as just a straight struct_tag without
> David> any variant_part/etc) then I think having a variant_part without
> David> a discriminator, etc, is probably a good idea.
>
> I'm afraid I didn't understand this.  Would you mind spelling it out a
> bit more?  Maybe saying what you think the DIEs should look like or what
> the API should look like would help me understand.
>

Sure, here's an attempt to be more clear *fingers crossed*

If this union-of-one-thing needs special handling in the debugger, such
that (using C syntax for my convenience) "struct X { int i; };" and "union
X { int i; };" can't be encoded the same, then it seems reasonable to me to
encode the latter as:

  DW_TAG_struct
    DW_TAG_variant_part
      DW_TAG_variant

no DW_AT_discr, no DW_AT_discr_values, etc.

Though of course this is non-conforming, I suppose (specifically "If the
variant part does not have a discriminant (tag field), the variant part
entry has a DW_AT_type attribute to represent the tag type." - sounds like
the variant_part above would need a DW_AT_type, but there is no tag type,
arguably it could be a void tag type?)


> Here's a way that this could be made more flexible.
>
> * Add DIBuilder::createVariantPartMember, to make a variant part.  This
> could
>   be attached to whatever we think is reasonable -- an enum, a struct, etc.
>
> * Also add DIBuilder::createVariantMember, which makes a single variant
>   for a variant part.
>
> Then, when emitting the variant part, if there is a non-variant member,
> it is taken as the discriminant.  This ways Rust could emit a univariant
> enum by simply omitting the discriminant.
>
> This approach would let callers have more control over the construction
> of the variant part, variants, and the placement thereof.
>

*nod* Roughly what I had in mind. Open to other opinions/if the review has
already moved on from this area, etc.

I was thinking more, rather than distinguish the discriminant by being a
non-variant member, sounded like/I thought you were proposing having the
extra data for the variant_part store the pointer to that? But I've
probably misremembered.


>
>
> On the topic of where the variant part belongs -- it does not matter
> much to me.  One reason this doesn't matter is that neither gdb nor lldb
> deal with any of this at all; so whatever is done will require debugger
> work.
>
> Now, I've already written the gdb support based on the patches I
> submitted.  However, it's fine to change that.  lldb on the other hand
> will have to wait until I finish the lldb Rust plugin.
>
> Furthermore, I think it's not very worthwhile to speculate about how
> other tools might fail.  There isn't enough information.  But if this is
> truly a concern then I would argue again for new tags everywhere,
> because that's probably safest -- it's normal in DWARF to ignore what
> one does not understand.
>
>
> Let me know what you'd like.  What I would most like is a clear
> direction forward.  Thanks.
>
> Tom
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180123/d6d369fa/attachment.html>


More information about the llvm-commits mailing list