[PATCH] D23716: [ELF] Linkerscript: allow adding start/end symbols to arbitrary section

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:314
@@ +313,3 @@
+// Linker script may define start and end symbols for special section types, 
+// like .got, .eh_frame_hdr, .eh_frame and others. Those special cases are 
+// handled by this function.
----------------
I'd also mention that special sections are not a list of regular output sections and therefore the regular mechanism to define symbols doesn't work. It's also good to mention that this approach is not perfect -- it only handles start and end symbols.

================
Comment at: ELF/LinkerScript.cpp:321
@@ +320,3 @@
+  for (std::unique_ptr<BaseCommand> &Base : Cmd->Commands) {
+    if (dyn_cast<InputSectionDescription>(Base.get())) {
+      Start = false;
----------------
Use `isa` instead of `dyn_cast`.

================
Comment at: ELF/LinkerScript.cpp:321-324
@@ +320,6 @@
+  for (std::unique_ptr<BaseCommand> &Base : Cmd->Commands) {
+    if (dyn_cast<InputSectionDescription>(Base.get())) {
+      Start = false;
+    } else if (auto *AssignCmd = dyn_cast<SymbolAssignment>(Base.get())) {
+      if (AssignCmd->Sym) {
+        auto *Sym = cast<DefinedSynthetic<ELFT>>(AssignCmd->Sym);
----------------
ruiu wrote:
> Use `isa` instead of `dyn_cast`.
Let's reduce the indentation level by using early continues.

  if (isa<InputSectionDescription>(...)) {
    Start = false;
    continue;
  }
  auto *AssignCmd = dyn_cast...;
  if (!AssignCmd)
    continue;


https://reviews.llvm.org/D23716