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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:424
@@ +423,3 @@
+  expect("=");
+  LocationNode &Node = Script->addLocation(Expr, {});
+  while (!Error) {
----------------
ruiu wrote:
> 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.
Removed.

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

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

================
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);
----------------
ruiu wrote:
> Arg -> Args
There is no such method anymore.


http://reviews.llvm.org/D18499