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

[George Rimar via Phabricator via llvm-commits]

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