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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:424
@@ +423,3 @@
+  expect("=");
+  LocationNode &Node = Script->addLocation(Expr, {});
+  while (!Error) {
----------------
I think you should remove addLocation function and directly add new elements to Locations vector since you intentionally merge two different control flow into one function and dispatch again using the first argument. That's just confusing.

================
Comment at: ELF/LinkerScript.h:47
@@ +46,3 @@
+// Section is a description of output section, like ".data :..."
+enum Command { Expr, Section };
+struct LocationNode {
----------------
Let's make this an enum class because it currently adds "Section" and "Expr" to lld::elf namespace.

================
Comment at: ELF/LinkerScript.h:48
@@ +47,3 @@
+enum Command { Expr, Section };
+struct LocationNode {
+  Command Type;
----------------
Add blank line before this struct definition.

================
Comment at: ELF/LinkerScript.h:74
@@ -58,1 +73,3 @@
 private:
+  LocationNode &addLocation(Command Type, std::vector<StringRef> Arg);
+  uint64_t evaluate(std::vector<StringRef> &Tokens, uint64_t LocCounter);
----------------
Arg -> Args


http://reviews.llvm.org/D18499