[PATCH] D60353: ELF: Add basic partition data structures and behaviours.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 13 01:39:18 PDT 2019


grimar added a comment.

This looks good to me, but I do not think I am a proper person to give a final accept.
Minor nits/suggestions are inline.



================
Comment at: lld/ELF/Driver.cpp:101
+  Partitions.clear();
+  Partitions.emplace_back();
+
----------------
nit: I am not sure how much `Partitions = {{}};` is readable,
so I would replace with ` Partitions = {Partition()};`
Though IMO any of these lines looks probably a bit better than these two lines.


================
Comment at: lld/ELF/Driver.cpp:1419
+  NewPart.Name = PartName;
+  Sym->Partition = NewPart.getNumber();
+}
----------------
It seems to me this is an overoptimization for such place.

What do you think about the following?
```
  Partition P = {PartName};
  Sym->Partition = P.getNumber();
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60353/new/

https://reviews.llvm.org/D60353





More information about the llvm-commits mailing list