[PATCH] D35373: Use delegation instead of inheritance

[Rafael Avila de Espindola via llvm-commits]

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

> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:91-95
> +  virtual const DWARFSection &getRangeDWOSection() const { return Dummy; }
> +  virtual const DWARFSection &getAddrSection() const { return Dummy; }
> +  virtual const DWARFSection &getAppleNamesSection() const { return Dummy; }
> +  virtual const DWARFSection &getAppleTypesSection() const { return Dummy; }
> +  virtual const DWARFSection &getAppleNamespacesSection() const {
> ----------------
> Given that DWARFSection is now only StringRef - why not return by value & return "" like the rest? (& I sitll don't know whether DWARFSection needs to exist at all if it's only a wrapper around StringRef - it makes this whole class look a bit random/arbitrary between some functions returning StringRef for a section, and others returning const DWARFSection&)

DWARFSection is just StringRef, but derived class can return a class
derived from DWARFSection, so we need a pointer.

> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:156
> +protected:
> +  const DWARFObj &DObj;
> +
> ----------------
> Why is this non-owning and DWARFContextInMemory owning?

Mostly because it doesn't need to own. DWARFContextInMemory owns since
it constructs it.

> Are tehre any other implementations of DWARFContext (other than DWARFContextInMemory) or should we just do away with the abstraction if it's unused?

We could do away with it if DWARFContext would own the DWARFObj. That
way we would have just a convenience method to create a DWARFContext
with a "InMemory" DWARFObj. I will give that a try.

>
> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFSection.h:18-24
>  struct DWARFSection {
>    StringRef Data;
> +};
> +
> +struct DWARFSectionMap final : public DWARFSection {
>    RelocAddrMap Relocs;
>  };
> ----------------
> What's the reason for this split? Is DWARFSection much use if it's a straight up wrapper around StringRef?

In lld we will have one that handles relocations differently. See
D35386.

Cheers,
Rafael