[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