[PATCH] D18499: [ELF] - Implemented prototype of location counter support.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:36
@@ +35,3 @@
+  // Evaluates the expression given by list of tokens.
+  uint64_t evaluate(std::vector<StringRef> &Tokens) {
+    uint64_t Result = 0;
----------------
Let's make Tokens const. I don't usually add `const` to every place which we can add const, but adding const improves readability in this case (to convey message that you are passing it as a reference just to avoid creating a copy of the vector.)

================
Comment at: ELF/LinkerScript.cpp:113
@@ -75,1 +112,3 @@
 
+uint64_t LinkerScript::getSectionVA(StringRef Name, uint64_t VA) {
+  auto It = SecLoc.find(Name);
----------------
So, it looks like we need to call this function with section names in the same order as the section names appear in the linker script. Is this correct? If so, is that a good API?

It feel to me that this patch doesn't capture a correct abstraction of the writer and the linker script. Both LinkerScript and Writer have only partial knowledge of the section layout, and the interaction between them is not well defined. I'd like to see a different approach.

So, if a linker script has a SECTIONS command, the entire file layout is determined by the linker script, right? Can you pass the list of sections or the symbol table to LinkerScript and let it layout all the things?


http://reviews.llvm.org/D18499