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

[Toby Pohlen via Phabricator via llvm-commits]

pohlen added a comment.

Thanks for implementing this so quickly! Looks good to my untrained eyes :)



================
Comment at: lld/ELF/LinkerScript.cpp:264-274
+    bool found = false;
+    for (auto it = sectionCommands.begin(), e = sectionCommands.end(); it != e;
+         ++it)
+      if (auto *to = dyn_cast<OutputSection>(*it))
+        if (to->name == where) {
+          if (isAfter)
+            ++it;
----------------
I find this a bit difficult to read because of the nested statements. Maybe we can flatten it using llvm::find?


```
auto insertPos = llvm::find_if(sectionCommands, [where](BaseCommand *command) {
  auto *to = dyn_cast<OutputSection>(*it);
  return to != nullptr && to->name == where;
});

if (insertPos == sectionCommands.end())
  error("unable to insert " + os->name +
        Twine(isAfter ? " after " : " before ") + where);

if (isAfter)
  ++insertPos;

sectionCommands.insert(insertPos, os);
```

Up to you. Feel free to leave as is.


================
Comment at: lld/ELF/ScriptParser.cpp:558
+  bool isAfter = false;
+  StringRef where;
+  if (consume("AFTER"))
----------------
[nit]: Move declaration to initialization?

```
StringRef where = next();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74375