[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 @@
+
+class elf::LocCounterEvaluator {
+public:
----------------
It seems that we don't need this class because this class has only one member, LocCounter and you always reset LocCounter before calling evaluate. This class can just be a function that takes the current VA and an expression and returns a new VA.

================
Comment at: ELF/LinkerScript.cpp:72
@@ -31,1 +71,3 @@
 LinkerScript *elf::Script;
+LocCounterEvaluator Eval;
+
----------------
It is better to move this variable inside LinkerScript.

================
Comment at: ELF/LinkerScript.cpp:109
@@ +108,3 @@
+  uintX_t VA = 0;
+  auto VisitLocations = [&](std::vector<LocationNode> &Loc) {
+    for (LocationNode &Node : Loc) {
----------------
Does this have to be a lambda?

================
Comment at: ELF/LinkerScript.cpp:125
@@ +124,3 @@
+      OutputSectionBase<ELFT> *Sec = *I;
+      S.erase(I);
+
----------------
Why do you have to remove the element here?

================
Comment at: ELF/LinkerScript.cpp:267-270
@@ -165,1 +266,6 @@
 void ScriptParser::run() {
+  // Dummies sections has empty name, but we still should add
+  // and theat them as normal ouput sections.
+  for (unsigned I = 0; I < dummySectionsNum(); ++I)
+    Script->addLocation(LocOutSec, {""});
+
----------------
The concept of dummy sections seems a bit confusing. Could you improve it?

================
Comment at: ELF/LinkerScript.h:52-54
@@ +51,5 @@
+
+  // For LocSet this keeps list of tokens to evaluate location.
+  // For LocOutSec first element is a output section name.
+  std::vector<StringRef> Args;
+};
----------------
I wouldn't use the same variable for completely different purposes. I'd define two fields, Expr and SectionName, instead.


http://reviews.llvm.org/D18499