[PATCH] D74375: [ELF] Support INSERT [AFTER|BEFORE] for orphan sections

[Peter Smith via Phabricator via llvm-commits]

psmith added inline comments.


================
Comment at: lld/ELF/LinkerScript.h:316
+  // to be reordered.
+  std::vector<std::tuple<OutputSection *, bool, StringRef>> insertCommands;
 };
----------------
grimar wrote:
> What do you think about adding a named struct instead of using the `tuple` (see `PhdrsCommand`/`AddressState` above).
FWIW I'd tend towards using a named struct here. We have to look down to the bit of code to find out what the fields are.
```
    OutputSection *os;
    bool isAfter;
    StringRef where;
    std::tie(os, isAfter, where) = insert;
```



================
Comment at: lld/ELF/ScriptParser.cpp:548
+  if (atEOF() || !consume("INSERT")) {
+    // -no-rosegment is used to avoid placing read only non-executable sections
+    // in their own segment. We do the same if SECTIONS command is present in
----------------
MaskRay wrote:
> In the future, we should stop enforcing --no-rosegment for hasSectionsCommand so that features can be more orthogonal. (I've mentioned this many times to Android/FreeBSD folks...)
Perhaps the intention can be added to the release notes for 10.0. For example in LLD 11.0 we intend to make the following changes:
- a linker script no longer implies --no-rosegment
This might be some way towards informing users of interface changes so that they don't get surprised. Given that few read the release notes, we might also consider a warning, maybe an optional one if someone uses a feature that will have its default changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74375