[PATCH] D27276: [ELF] Allow defined symbols to be assigned from linker script

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/LinkerScript.cpp:85
+  // If there are no sections and the symbol value needs to be assigned,
+  // then do it now. Otherwise, let the value be assigned later in
+  // `assignAddresses`.
----------------
What is "it" here?


================
Comment at: ELF/LinkerScript.cpp:103
+    return;
+
   SymbolBody *B = Symtab<ELFT>::X->find(Cmd->Name);
----------------
I think this function can be simplified. We do not create a new symbol only when

 - it is in a PROVIDE(), and
 - the existing symbol is not an Undefined symbol.

Otherwise, we always want to create symbol and then assign a value to it. So, we can return early for the PROVIDE() case.


================
Comment at: ELF/LinkerScript.cpp:106-107
+  auto CreateSymbol = [=](void) {
+    Cmd->Sym = Cmd->Expression.IsAbsolute() ? addRegular<ELFT>(Cmd)
+                                            : addSynthetic<ELFT>(Cmd);
+  };
----------------
nit: the ternary operator doesn't look nice if it need too much indentation. Probably this is better.

  if (Cmd_>Expression.IsAbsolute())
    Cmd->Sym = addRegular...;
  else
    Cmd->Sym = ...;


https://reviews.llvm.org/D27276