[PATCH] D60353: ELF: Add basic partition data structures and behaviours.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 16 01:31:04 PDT 2019
grimar added inline comments.
================
Comment at: lld/ELF/Driver.cpp:1387
+
+ StringRef PartName = reinterpret_cast<const char *>(S->data().data());
+ for (Partition &Part : getPartitions()) {
----------------
Seems you can use `toStringRef` (from llvm\ADT\StringExtras.h)?
================
Comment at: lld/ELF/Driver.cpp:1396
+ if (NumPartitions == 254)
+ fatal("may not have more than 254 partitions");
+ ++NumPartitions;
----------------
Should it be `error`?
================
Comment at: lld/ELF/Driver.cpp:1648
+ if (S->Type == SHT_LLVM_SYMPART) {
+ readSymbolPartitionSection<ELFT>(S);
+ return true;
----------------
Will it be better to report errors somewhere here, so you could
include the file name to error text?
================
Comment at: lld/ELF/SyntheticSections.cpp:3258
+Partition elf::Partitions[255];
+size_t elf::NumPartitions;
+
----------------
Why not `std::vector<Partition>` Partitions?
================
Comment at: lld/ELF/SyntheticSections.h:1063
+ StringRef Name;
+ unsigned getNumber() const;
+};
----------------
Perhaps just inline the body?
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