[PATCH] D35373: Use delegation instead of inheritance

[Rafael Avila de Espindola via llvm-commits]

David Blaikie via Phabricator <reviews at reviews.llvm.org> writes:

> dblaikie added a comment.
>
> Might be helpful to have a bit of a summary about what's changing here, separate from looking at all the source manually - what operations stayed in DWARFContext{,InMemory} versus those that moved out into DWARFObject?

Every new virtual method was moved into DWARFObject.

> Do all the operations need wrappers in DWARFContext, or could many of those be removed because they're probably only used internally within DWARFContext & could be replaced with "Obj->thing" instead? (though that's more refactoring/churn, I realize)

Good point. I will give it a try.

> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:56
> +  virtual ~DWARFObj() {}
> +  virtual StringRef getFileName() const { llvm_unreachable("unimplemented"); }
> +  virtual bool isLittleEndian() const { llvm_unreachable("unimplemented"); }
> ----------------
> Why do these all need default implementations? Guessing there's a reason they're not pure virtual? Though it seems like it'd make using this class more difficult if there are implicit contracts about which subset of operations are implemented and which ones are not?

On the new patch version I added a comment.

The issue is that DWARFContext is used to parse many different debug
info sections. Some clients don't need it all, and it is cumbersome for
them to need to implement every method.

I will also upload the lld patch.

Cheers,
Rafael