[PATCH] D120650: [ELF] Don't use multiple inheritance for OutputSection. NFC

Mon Feb 28 11:44:12 PST 2022

MaskRay marked an inline comment as done.
MaskRay added a comment.

In D120650#3349298 <https://reviews.llvm.org/D120650#3349298>, @peter.smith wrote:

> No objections for the change. I think it makes sense to logically separate OutputSection from SectionCommand. I'd like to see if anyone else has a strong opinion as there are quite a few changes and some more dynamic memory allocations. I'm wondering if there may be a way of avoiding those somehow, for example could an implicit conversion operator to OutputSection* be added?

I have switched to

  struct OutputDesc final : SectionCommand {
    OutputSection osec;
  ...
  };

`make<OutputDesc>` is used in replace of previous `make<OutputDescription>`. There is no extra allocation.

> I'm wondering if there may be a way of avoiding those somehow, for example could an implicit conversion operator to OutputSection* be added?

The majority differences are due to `dyn_cast<OutputSection>` => `dyn_cast<OutputDesc>` which need changes.

An conversion operator may help a few `osd->osec` places but they are typically coupled with `dyn_cast<OutputDesc>`, so the total number of changes may not decrease a lot.
An explicit `osd->osec` seems clearer to me.

================
Comment at: lld/ELF/OutputSections.h:30

+struct OutputDesc final : SectionCommand {
+  OutputSection *osec;
----------------
peter.smith wrote:
> Given that this doesn't use anything about the representation of OutputSection, would it be better to move it to the `LinkerScript.h` header.
OutputDesc now uses OutputSection as a non-pointer data member, so it needs to stay in this file.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120650/new/

https://reviews.llvm.org/D120650