[PATCH] D46271: [CodeView] Improve debbuging of virtual base class member variables

[Brock Wyma via Phabricator via llvm-commits]

bwyma added inline comments.


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:793-794
 
+  /// \brief In inheritance, the pointer offset to virtual bases.
+  uint32_t VBPtrOffset;
+
----------------
aprantl wrote:
> aprantl wrote:
> > rnk wrote:
> > > `vbptroffset` is pretty esoteric. It seems really unfortunate to have to widen the very popular `DIDerivedType` constructor to take this obscure parameter. Pointer and reference types are some of the most common kinds of DIDerivedType nodes, so taking an address space is less objectionable, even though it is specialized towards one kind of derived type node.
> > > 
> > > I'd recommend encoding this information in the `extraData` field, similar to how we encode bit offsets for bitfields, or making a new `DIInheritanceType` node. @aprantl, @dblaikie WDYT?
> > please drop the \brief.
> Seems reasonable; avoiding making DIDerivedType larger for all languages and platforms would be a good thing.
Although I believe DIInheritedType is a better solution, changing DW_TAG_inheritance from DIDerivedType to DIInheritedType is a substantial change which I would recommend making independently from this fix.  Furthermore, your valid argument regarding the esoteric nature of the field will be just as valid for DIInheritedType as it is for DIDerivedType, so I think in either case the field should be moved to extraData.  As a result, for the purposes of this patch I propose moving this field to the extraData field of DIDerivedType.  Is this sufficient?


https://reviews.llvm.org/D46271