[PATCH] D16771: [ELF] PHDRS linker script command implemented.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:195-197
@@ +194,5 @@
+StringRef ScriptParser::peek() {
+  if (atEOF())
+    error("unexpected EOF");
+  return Tokens[Pos];
+}
----------------
error() no longer call exit(), so you have to handle errors gracefully. In this case, if atEOF(), Token[Pos] is invalid.

================
Comment at: ELF/LinkerScript.cpp:358
@@ +357,3 @@
+    error("Integer can't be parsed: " + Tok);
+  return Val;
+}
----------------
Do not return an undefined value.

================
Comment at: ELF/LinkerScript.cpp:361
@@ +360,3 @@
+
+void ScriptParser::readPhdrs() {
+  expect("{");
----------------
Needs a comment.

================
Comment at: ELF/LinkerScript.cpp:380
@@ +379,3 @@
+      else
+        error("Unknown header attribute " + Tok);
+    }
----------------
Handle errors gracefully.

================
Comment at: ELF/Writer.cpp:1209
@@ +1208,3 @@
+template <class ELFT> void Writer<ELFT>::createPhdrsFromScript() {
+  // Reserve for total PHDRS amount + 1 for "NONE" synthetic PHDR.
+  Phdrs.reserve(Script->Phdrs.size() + 1);
----------------
grimar wrote:
> Btw, is it supposed that createPhdrsFromScript() stuff should go to LinkerScript.cpp ?
> Problem is that here the field of Writer is changed (Phdrs) and also methods and <ELFT> is used which I am not sure would look good in linkerscript files.
I think we probably want to move this to LinkerScript.cpp. Also this function is too long. Is there any way to reduce the amount of code and split into smaller pieces?


http://reviews.llvm.org/D16771